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 <[email protected]> wrote:
> On Tue, Mar 4, 2014 at 9:43 PM, Ronen Mizrahi <[email protected]> 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 [email protected] 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);
