Merge pull request #83 from tellerhq/close_connections_in_single_requ…

…est_pool

Fix pool: false leaking connections

lib/mojito/conn.ex

@@ -21,6 +21,19 @@ defmodule Mojito.Conn do
     end
   end
 
+  @doc ~S"""
+  Closes a connection
+  """
+  @spec close(t) :: :ok
+  def close(conn) do
+    # mint returns an updated conn
+    # but i don't understand why anyone
+    # would care about it. i think they just
+    # close the socket and set state: closed
+    Mint.HTTP.close(conn.conn)
+    :ok
+  end
+
   @doc ~S"""
   Connects to the server specified in the given URL,
   returning a connection to the server.  No requests are made.

lib/mojito/request/single.ex

@@ -23,12 +23,12 @@ defmodule Mojito.Request.Single do
   @spec request(Mojito.request()) ::
           {:ok, Mojito.response()} | {:error, Mojito.error()}
   def request(%Request{} = req) do
-    with {:ok, req} <- Request.validate_request(req),
-         {:ok, conn} <- Conn.connect(req.url, req.opts),
-         {:ok, conn, _ref, response} <- Conn.request(conn, req) do
-      timeout = req.opts[:timeout] || Config.timeout()
-      receive_response(conn, response, timeout)
-    end
+    with_connection(req, fn conn ->
+      with {:ok, conn, _ref, response} <- Conn.request(conn, req) do
+        timeout = req.opts[:timeout] || Config.timeout()
+        receive_response(conn, response, timeout)
+      end
+    end)
   end
 
   defp time, do: System.monotonic_time(:millisecond)
@@ -87,4 +87,15 @@ defmodule Mojito.Request.Single do
         receive_response(conn, response, new_timeout.())
     end
   end
+
+  defp with_connection(req, fun) do
+    with {:ok, req} <- Request.validate_request(req),
+         {:ok, conn} <- Conn.connect(req.url, req.opts) do
+      try do
+        fun.(conn)
+      after
+        Conn.close(conn)
+      end
+    end
+  end
 end

test/mojito_sync_test.exs

@@ -0,0 +1,43 @@
+defmodule MojitoSyncTest do
+  use ExSpec, async: false
+  doctest Mojito
+
+  context "local server tests" do
+    @http_port Application.get_env(:mojito, :test_server_http_port)
+
+    defp get(path, opts) do
+      Mojito.get(
+        "http://localhost:#{@http_port}#{path}",
+        [],
+        opts
+      )
+    end
+
+    it "doesn't leak connections with pool: false" do
+      original_open_ports = length(open_tcp_ports(@http_port))
+      assert({:ok, response} = get("/", pool: false))
+      assert(200 == response.status_code)
+
+      final_open_ports = length(open_tcp_ports(@http_port))
+      assert original_open_ports == final_open_ports
+    end
+  end
+
+  defp open_tcp_ports(to_port) do
+    Enum.filter(tcp_sockets(), fn socket ->
+      case :inet.peername(socket) do
+        {:ok, {_ip, ^to_port}} -> true
+        _error -> false
+      end
+    end)
+  end
+
+  defp tcp_sockets() do
+    Enum.filter(:erlang.ports(), fn port ->
+      case :erlang.port_info(port, :name) do
+        {_, 'tcp_inet'} -> true
+        _ -> false
+      end
+    end)
+  end
+end