Ensure files in sendfile calls are closed (#78)

lib/thousand_island/transports/ssl.ex

@@ -107,15 +107,22 @@ defmodule ThousandIsland.Transports.SSL do
   def sendfile(socket, filename, offset, length) do
     # We can't use :file.sendfile here since it works on clear sockets, not ssl
     # sockets. Build our own (much slower and not optimized for large files) version.
-    with {:ok, fd} <- :file.open(filename, [:raw]),
-         {:ok, data} <- :file.pread(fd, offset, length) do
-      case :ssl.send(socket, data) do
-        :ok -> {:ok, length}
-        {:error, error} -> {:error, error}
-      end
-    else
-      :eof -> {:error, :eof}
-      err -> err
+    case :file.open(filename, [:raw]) do
+      {:ok, fd} ->
+        try do
+          with {:ok, data} <- :file.pread(fd, offset, length),
+               :ok <- :ssl.send(socket, data) do
+            {:ok, length}
+          else
+            :eof -> {:error, :eof}
+            {:error, reason} -> {:error, reason}
+          end
+        after
+          :file.close(fd)
+        end
+
+      {:error, reason} ->
+        {:error, reason}
     end
   end
 

lib/thousand_island/transports/tcp.ex

@@ -93,8 +93,16 @@ defmodule ThousandIsland.Transports.TCP do
           length :: non_neg_integer()
         ) :: ThousandIsland.Transport.on_sendfile()
   def sendfile(socket, filename, offset, length) do
-    with {:ok, fd} <- :file.open(filename, [:raw]) do
-      :file.sendfile(fd, socket, offset, length, [])
+    case :file.open(filename, [:raw]) do
+      {:ok, fd} ->
+        try do
+          :file.sendfile(fd, socket, offset, length, [])
+        after
+          :file.close(fd)
+        end
+
+      {:error, reason} ->
+        {:error, reason}
     end
   end
 

test/thousand_island/socket_test.exs

@@ -46,6 +46,7 @@ defmodule ThousandIsland.SocketTest do
     def handle_connection(socket, state) do
       ThousandIsland.Socket.sendfile(socket, Path.join(__DIR__, "../support/sendfile"), 0, 6)
       ThousandIsland.Socket.sendfile(socket, Path.join(__DIR__, "../support/sendfile"), 1, 3)
+      send(state[:test_pid], Process.info(self(), :monitored_by))
       {:close, state}
     end
   end
@@ -90,13 +91,6 @@ defmodule ThousandIsland.SocketTest do
         assert context.client_mod.recv(client, 0) == {:ok, ~c"HELLO"}
       end
 
-      test "it should send files", context do
-        {:ok, port} = start_handler(Sendfile, context.server_opts)
-        {:ok, client} = context.client_mod.connect(~c"localhost", port, context.client_opts)
-
-        assert context.client_mod.recv(client, 9) == {:ok, ~c"ABCDEFBCD"}
-      end
-
       test "it should close connections", context do
         {:ok, port} = start_handler(Closer, context.server_opts)
         {:ok, client} = context.client_mod.connect(~c"localhost", port, context.client_opts)
@@ -154,6 +148,14 @@ defmodule ThousandIsland.SocketTest do
 
       context.client_mod.close(client)
     end
+
+    test "it should send files", context do
+      server_opts = Keyword.put(context.server_opts, :handler_options, test_pid: self())
+      {:ok, port} = start_handler(Sendfile, server_opts)
+      {:ok, client} = context.client_mod.connect(~c"localhost", port, context.client_opts)
+      assert context.client_mod.recv(client, 9) == {:ok, ~c"ABCDEFBCD"}
+      assert_receive {:monitored_by, []}
+    end
   end
 
   describe "behaviour specific to ssl" do
@@ -183,6 +185,14 @@ defmodule ThousandIsland.SocketTest do
 
       context.client_mod.close(client)
     end
+
+    test "it should send files", context do
+      server_opts = Keyword.put(context.server_opts, :handler_options, test_pid: self())
+      {:ok, port} = start_handler(Sendfile, server_opts)
+      {:ok, client} = context.client_mod.connect(~c"localhost", port, context.client_opts)
+      assert context.client_mod.recv(client, 9) == {:ok, ~c"ABCDEFBCD"}
+      assert_receive {:monitored_by, [_pid]}
+    end
   end
 
   defp start_handler(handler, server_args) do