From d7142993ed372c7f51ba381d5166ec7707951670 Mon Sep 17 00:00:00 2001
From: Kirilll Zaborsky <qrilka@gmail.com>
Date: Wed, 8 Oct 2014 15:31:13 +0400
Subject: inets: stop httpc_handler on 'connection closed' send error

httpc_handler should be stopped when sending requests returns error
because of a closed connection and `tcp_closed` message could not be
catched (see http://erlang.org/pipermail/erlang-bugs/2007-May/000346.html )
otherwise it will lead to process leak.
---
 lib/inets/src/http_client/httpc_handler.erl | 19 ++++++++++---------
 lib/inets/test/httpc_SUITE.erl              | 29 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 9 deletions(-)

(limited to 'lib/inets')

diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl
index 0a42e7210c..7f7328f1d9 100644
--- a/lib/inets/src/http_client/httpc_handler.erl
+++ b/lib/inets/src/http_client/httpc_handler.erl
@@ -316,8 +316,9 @@ handle_call(#request{address = Addr} = Request, _,
                     {reply, ok, State}
             end;
         {error, Reason} ->
-            ?hcri("failed sending request", [{reason, Reason}]),
-            {reply, {pipeline_failed, Reason}, State0}
+		    ?hcri("failed sending request", [{reason, Reason}]),
+            NewPipeline = queue:in(Request, State0#state.pipeline),
+            {stop, shutdown, {pipeline_failed, Reason}, State0#state{pipeline = NewPipeline}}
     end;
 
 handle_call(#request{address = Addr} = Request, _, 
@@ -355,25 +356,25 @@ handle_call(#request{address = Addr} = Request, _,
 	    ?hcrd("no current request", []),
 	    cancel_timer(Timers#timers.queue_timer,
 			 timeout_queue),
+	    NewTimers = Timers#timers{queue_timer = undefined},
+	    State1 = State0#state{timers = NewTimers},
 	    Address = handle_proxy(Addr, Proxy),
 	    case httpc_request:send(Address, Session, Request) of
 		ok ->
 		    ?hcrd("request sent", []),
 
 		    %% Activate the request time out for the new request
-		    State1 =
-			activate_request_timeout(State0#state{request = Request}),
-		    NewTimers = State1#state.timers,
+		    State2 =
+			activate_request_timeout(State1#state{request = Request}),
 		    NewSession =
 			Session#session{queue_length = 1,
 					client_close = ClientClose},
 		    insert_session(NewSession, ProfileName),
-		    State = init_wait_for_response_state(Request, State1#state{session = NewSession,
-								      timers = NewTimers}),
+		    State = init_wait_for_response_state(Request, State2#state{session = NewSession}),
 		    {reply, ok, State};
 		{error, Reason} ->
 		    ?hcri("failed sending request", [{reason, Reason}]),
-		    {reply, {request_failed, Reason}, State0}
+		    {stop, shutdown, {keepalive_failed, Reason}, State1}
 	    end
     end;
 
@@ -1329,7 +1330,7 @@ handle_keep_alive_queue(#state{status       = keep_alive,
 					     Session, <<>>,
 					     State#state{keep_alive = KeepAlive});
 			{error, Reason} ->
-			    {reply, {keep_alive_failed, Reason}, State}
+			    {stop, shutdown, {keepalive_failed, Reason}, State}
 		    end
 	    end
     end.
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index c535d59b9f..78d7ecbf61 100644
--- a/lib/inets/test/httpc_SUITE.erl
+++ b/lib/inets/test/httpc_SUITE.erl
@@ -28,6 +28,7 @@
 -include_lib("common_test/include/ct.hrl").
 -include("inets_test_lib.hrl").
 -include("http_internal.hrl").
+-include("httpc_internal.hrl").
 %% Note: This directive should only be used in test suites.
 -compile(export_all).
 
@@ -105,6 +106,7 @@ only_simulated() ->
      empty_response_header,
      remote_socket_close,
      remote_socket_close_async,
+     process_leak_on_keepalive%,
      transfer_encoding,
      transfer_encoding_identity,
      redirect_loop,
@@ -900,6 +902,33 @@ remote_socket_close_async(Config) when is_list(Config) ->
 
 %%-------------------------------------------------------------------------
 
+process_leak_on_keepalive(Config) ->
+    {ok, ClosedSocket} = gen_tcp:listen(6666, [{active, false}]),
+    ok = gen_tcp:close(ClosedSocket),
+    Request  = {url(group_name(Config), "/dummy.html", Config), []},
+    HttpcHandlers0 = supervisor:which_children(httpc_handler_sup),
+    {ok, {{_, 200, _}, _, Body}} = httpc:request(get, Request, [], []),
+    HttpcHandlers1 = supervisor:which_children(httpc_handler_sup),
+    ChildrenCount = supervisor:count_children(httpc_handler_sup),
+    %% Assuming that the new handler will be selected for keep_alive
+    %% which could not be the case if other handlers existed
+    [{undefined, Pid, worker, [httpc_handler]}] =
+        ordsets:to_list(
+          ordsets:subtract(ordsets:from_list(HttpcHandlers1),
+                           ordsets:from_list(HttpcHandlers0))),
+    sys:replace_state(
+      Pid, fun (State) ->
+                   Session = element(3, State),
+                   setelement(3, State, Session#session{socket=ClosedSocket})
+           end),
+    {ok, {{_, 200, _}, _, Body}} = httpc:request(get, Request, [], []),
+    %% bad handler with the closed socket should get replaced by
+    %% the new one, so children count should stay the same
+    ChildrenCount = supervisor:count_children(httpc_handler_sup),
+    ok.
+
+%%-------------------------------------------------------------------------
+
 stream_to_pid(Config) when is_list(Config) ->
     ReceiverPid = create_receiver(pid),
     Receiver    = ReceiverPid,
-- 
cgit v1.2.3