Bugs in httpc timeout and keep-alive queue handling

View: New views
1 Messages — Rating Filter:   Alert me  

Bugs in httpc timeout and keep-alive queue handling

by Jean-Sébastien Pédron-4 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

Attached is a patch against R13B02-1 that fixes two bugs in
httpc_handler.erl.

The first part fixes a bug with request timeout. When a queued request
(ie. not the active one in #state.request) times out, the error is sent
to the process associated with the active request. If I understand the
problem correctly, this means that this process could receive the
timeout error and the HTTP response, but the process for which the
request timed out won't receive anything.

The second part fixes a bug with the keep-alive queue: in terminate/2,
the requests in this queue are "forgotten". With the patch, the
keep-alive queue is treated like the pipeline queue.

The third part adds a clause to the retry_pipline/2 function to not
retry timed out requests.

With this patch, I couldn't hang httpc with multiple concurrent requests
to the same URL (a slow webservice). The handle_info/2 clause printing a
warning about unexpected received data isn't triggered anymore (it was
added in R13B02).

- --
Jean-Sébastien Pédron
http://www.dumbbell.fr/

PGP Key: http://www.dumbbell.fr/pgp/pubkey.asc
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrcILIACgkQa+xGJsFYOlPy+ACeNJMIIkKBUaOH7+r7FYBKgSpG
C94AoIvGUW4NLh/FjjorTYJJxLVEFjiy
=D7JD
-----END PGP SIGNATURE-----

--- otp_src_R13B02-1.orig/lib/inets/src/http_client/httpc_handler.erl 2009-09-18 16:12:59.000000000 +0200
+++ otp_src_R13B02-1/lib/inets/src/http_client/httpc_handler.erl 2009-10-14 09:22:46.000000000 +0200
@@ -505,10 +505,25 @@
     {stop, normal,
      State#state{canceled = [RequestId | State#state.canceled],
  request = Request#request{from = answer_sent}}};
-handle_info({timeout, RequestId}, State = #state{request = Request}) ->
-    httpc_response:send(Request#request.from,
-       httpc_response:error(Request,timeout)),
-    {noreply, State#state{canceled = [RequestId | State#state.canceled]}};
+handle_info({timeout, RequestId}, State) ->
+    Fun = fun
+ (#request{id = Id} = Request) when Id == RequestId ->
+    httpc_response:send(Request#request.from,
+ httpc_response:error(Request,timeout)),
+    [Request#request{from = answer_sent}];
+ (_) ->
+    true
+    end,
+    case State#state.status of
+ pipeline ->
+    Pipeline = queue:filter(Fun, State#state.pipeline),
+    {noreply, State#state{canceled = [RequestId | State#state.canceled],
+  pipeline = Pipeline}};
+ keep_alive ->
+    KeepAlive = queue:filter(Fun, State#state.keep_alive),
+    {noreply, State#state{canceled = [RequestId | State#state.canceled],
+  keep_alive = KeepAlive}}
+    end;
 
 handle_info(timeout_queue, State = #state{request = undefined}) ->
     {stop, normal, State};
@@ -588,6 +603,12 @@
  true ->
     ok
     end,
+    case queue:is_empty(State#state.keep_alive) of
+ false ->
+    retry_pipline(queue:to_list(State#state.keep_alive), State);
+ true ->
+    ok
+    end,
     cancel_timer(Timers#timers.queue_timer, timeout_queue),
     Socket = Session#tcp_session.socket,
     http_transport:close(socket_type(Session#tcp_session.scheme), Socket);
@@ -1191,6 +1212,8 @@
 retry_pipline([], _) ->
     ok;
 
+retry_pipline([#request{from = answer_sent}|PipeLine], State) ->
+    retry_pipline(PipeLine, State);
 retry_pipline([Request |PipeLine],  State =
       #state{timers = Timers, profile_name = ProfileName}) ->
     NewState =


________________________________________________________________
erlang-patches mailing list. See http://www.erlang.org/faq.html
erlang-patches (at) erlang.org