Re: [Libevent-users] Possible bug in http.c

[Ronen Mizrahi <ronen@xxxxxxxxxxxx>]

It looks like I was mistaken and the patch was not the cause of the issue I mentioned. I have continued testing it and it seems to perform correctly so I am attaching it.

Note that I chose to enable read when write is performed, instead of just eliminating all cases where read is disabled, because we did not know what side-effects that might have. In contrast, enabling read while writing, suggests that the worst side-effects one may experience will be related to HTTP pipelining (the only case where the client is expected to write something while waiting for response from server), and so I briefly tested it as well (via telnet) and all seems to work as expected. It probably makes sense to test http pipelining some more though.

I did not get a chance to create another unit test, however.

Best,

Ronen

On Wed, Mar 5, 2014 at 11:01 AM, Nick Mathewson <nickm@xxxxxxxxxxxxx> wrote:

On Tue, Mar 4, 2014 at 9:43 PM, Ronen Mizrahi <ronen@xxxxxxxxxxxx> wrote:
> It looks like the patch does create issues with subsequent HTTP requests
> made on the same connection (for persistent connections) and therefore we
> won't submit it.
>
> The original issue reported however is still very much in effect. It is
> particularly problematic for long-polling type requests where the server
> uses evhttp_send_reply_start() and then every once in a while does
> evhttp_send_reply_chunk(). If the client closes the connection, the server
> won't detect it during the period between different chunks but rather only
> when attempting to send the next chunk. This period can be several minutes
> so this can be a real issue for servers that handle many connections.

What kind of issues did the original patch cause?  Maybe we can track
those issues down and fix it?

peace,

--
Nick
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.

--- http.c	2014-03-05 15:05:50.577168380 -0500
+++ http-patched.c	2014-03-05 15:11:13.629154795 -0500
@@ -373,7 +373,7 @@
 	    evhttp_error_cb,
 	    evcon);
 
-	bufferevent_enable(evcon->bufev, EV_WRITE);
+	bufferevent_enable(evcon->bufev, EV_READ | EV_WRITE);
 }
 
 static void
@@ -1150,7 +1150,10 @@
 	}
 
 	if (evcon->bufev != NULL)
+	{
+		bufferevent_disable(evcon->bufev, EV_READ|EV_WRITE);
 		bufferevent_free(evcon->bufev);
+	}
 
 	event_deferred_cb_cancel_(get_deferred_queue(evcon),
 	    &evcon->read_more_deferred_cb);