Socket delivery: fix race condition that might crash the manager.

miniclip · Dec 4, 2014 · f166ab0 · f166ab0
1 parent 938a902
commit f166ab0
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 8 deletions.
diff --git a/src/lhttpc_manager.erl b/src/lhttpc_manager.erl
@@ -243,9 +243,15 @@ ensure_call(Pool, Pid, Host, Port, Ssl, Options) ->
 -spec client_done(pid(), host(), port_num(), boolean(), socket()) -> ok.
 client_done(Pool, Host, Port, Ssl, Socket) ->
     case lhttpc_sock:controlling_process(Socket, Pool, Ssl) of
-        ok ->
+        {ok, PoolPid} ->
             DoneMsg = {done, Host, Port, Ssl, Socket},
-            ok = gen_server:call(Pool, DoneMsg, infinity);
+            DeliveryStatus = (catch gen_server:call(PoolPid, DoneMsg, infinity)),
+            case DeliveryStatus of
+                {'EXIT', {noproc,_}} -> catch lhttpc_sock:close(Socket, Ssl);
+                {'EXIT', {killed,_}} -> catch lhttpc_sock:close(Socket, Ssl);
+                ok -> ok
+            end,
+            ok;
         _ ->
             ok
     end.
@@ -391,7 +397,7 @@ find_socket({_, _, Ssl} = Dest, Pid, State) ->
         {ok, [Socket | Sockets]} ->
             lhttpc_sock:setopts(Socket, [{active, false}], Ssl),
             case lhttpc_sock:controlling_process(Socket, Pid, Ssl) of
-                ok ->
+                {ok, Pid} ->
                     {_, Timer} = dict:fetch(Socket, State#httpc_man.sockets),
                     cancel_timer(Timer, Socket),
                     NewState = State#httpc_man{
@@ -515,7 +521,7 @@ deliver_socket(Socket, {_, _, Ssl} = Dest, State) ->
         {ok, {PidWaiter, _} = FromWaiter, Queues2} ->
             lhttpc_sock:setopts(Socket, [{active, false}], Ssl),
             case lhttpc_sock:controlling_process(Socket, PidWaiter, Ssl) of
-                ok ->
+                {ok, PidWaiter} ->
                     gen_server:reply(FromWaiter, {ok, Socket}),
                     monitor_client(Dest, FromWaiter, State#httpc_man{queues = Queues2});
                 {error, badarg} -> % Pid died, reuse for someone else

diff --git a/src/lhttpc_sock.erl b/src/lhttpc_sock.erl
@@ -129,7 +129,7 @@ send(Socket, Request, false) ->
     gen_tcp:send(Socket, Request).
 
 %%------------------------------------------------------------------------------
-%% @spec (Socket, Process, SslFlag) -> ok | {error, Reason}
+%% @spec (Socket, Process, SslFlag) -> {ok, pid()} | {error, Reason}
 %%   Socket = socket()
 %%   Process = pid() | atom()
 %%   SslFlag = boolean()
@@ -139,13 +139,19 @@ send(Socket, Request, false) ->
 %% @end
 %%------------------------------------------------------------------------------
 -spec controlling_process(socket(), pid() | atom(), boolean()) ->
-    ok | {error, atom()}.
+    {ok, pid()} | {error, atom()}.
 controlling_process(Socket, Controller, IsSsl) when is_atom(Controller) ->
     controlling_process(Socket, whereis(Controller), IsSsl);
 controlling_process(Socket, Pid, true) ->
-    ssl:controlling_process(Socket, Pid);
+    case ssl:controlling_process(Socket, Pid) of
+        ok  -> {ok, Pid};
+        Err -> Err
+    end;
 controlling_process(Socket, Pid, false) ->
-    gen_tcp:controlling_process(Socket, Pid).
+    case gen_tcp:controlling_process(Socket, Pid) of
+        ok  -> {ok, Pid};
+        Err -> Err
+    end.
 
 %%------------------------------------------------------------------------------
 %% @spec (Socket, Options, SslFlag) -> ok | {error, Reason}