This looks awesome. Is it possible to add a regression test that triggers the behavior?
On Mon, May 9, 2011 at 11:46 PM, Kevin Ko <kevin.s...@gmail.com> wrote: > Hi, > > Patch in question: > - > Fix the case when failed evhttp_make_request() leaved request in the queue. > - > http://levent.git.sourceforge.net/git/gitweb.cgi?p=levent/libevent;a=commit;h=0d6622e > > The above patch introduces a failing assertion in > evhttp_connection_fail(). This happens because the patch defers the > assignment of the outstanding request to the evcon->requests list, > while evhttp_connection_fail() assumes that the request lies in the > list. > > One scenario in which this can happen is when the request list is > empty and a connection is made to an unreachable host. The assertion > will then fail after bufferevent_socket_connect() errors out (with > ENETUNREACH in my case). > > At the end of this message, I've attached a gdb backtrace of an > instance with the failed assertion and a patch that should revise the > original solution to have an effect only when > evhttp_connection_connect() fails. > > Kevin > > ***** begin gdb backtrace ***** > > #0 0xb7fe2430 in __kernel_vsyscall () > #1 0xb7e8f651 in *__GI_raise (sig=6) > at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 > #2 0xb7e92a82 in *__GI_abort () at abort.c:92 > #3 0x08090a8f in event_exit (errcode=-559030611) at log.c:79 > #4 0x08090be9 in event_errx (eval=-559030611, > fmt=0x80d3e78 "%s:%d: Assertion %s failed in %s") at log.c:136 > #5 0x080894ec in evhttp_connection_fail (evcon=0x8572480, > error=EVCON_HTTP_EOF) at http.c:689 > #6 0x0808a766 in evhttp_error_cb (bufev=0x8572560, what=32, arg=0x8572480) > at http.c:1327 > #7 0x0808a7ce in evhttp_connection_cb (bufev=0x8572560, what=32, > arg=0x8572480) at http.c:1352 > #8 0x0807cde5 in _bufferevent_run_eventcb (bufev=0x8572560, what=32) > at bufferevent.c:264 > #9 0x0807fcce in bufferevent_socket_connect (bev=0x8572560, sa=0x8571bd0, > socklen=16) at bufferevent_sock.c:420 > #10 0x0807fd9a in bufferevent_connect_getaddrinfo_cb (result=0, > ai=0x8571bb0, > arg=0x8572560) at bufferevent_sock.c:453 > #11 0x080872ac in evutil_getaddrinfo_async (dns_base=0x0, > nodename=0x8572548 "72.14.204.141", servname=0xbfffe1c2 "80", > hints_in=0xbfffe198, cb=0x807fcfe <bufferevent_connect_getaddrinfo_cb>, > arg=0x8572560) at evutil.c:1396 > #12 0x0807fed4 in bufferevent_socket_connect_hostname (bev=0x8572560, > evdns_base=0x0, family=0, hostname=0x8572548 "72.14.204.141", port=80) > at bufferevent_sock.c:489 > #13 0x0808bee8 in evhttp_connection_connect (evcon=0x8572480) at > http.c:2148 > #14 0x0808c0a1 in evhttp_make_request (evcon=0x8572480, req=0x85715b0, > type=EVHTTP_REQ_POST, uri=0x855b640 "/verify") at http.c:2199 > > ***** end gdb backtrace ***** > > ***** begin patch ***** > > diff --git a/libevent2/http.c b/libevent2/http.c > index 06fe614..0797b5a 100644 > --- a/libevent2/http.c > +++ b/libevent2/http.c > @@ -2194,21 +2194,21 @@ evhttp_make_request(struct evhttp_connection > *evcon, > req->evcon = evcon; > EVUTIL_ASSERT(!(req->flags & EVHTTP_REQ_OWN_CONNECTION)); > > + TAILQ_INSERT_TAIL(&evcon->requests, req, next); > + > /* If the connection object is not connected; make it so */ > if (!evhttp_connected(evcon)) { > int res = evhttp_connection_connect(evcon); > - /* > - * Enqueue the request only if we aren't going to > - * return failure from evhttp_make_request(). > - */ > - if (res == 0) > - TAILQ_INSERT_TAIL(&evcon->requests, req, next); > + /* evhttp_connection_fail(), which is called through > + * evhttp_connection_connect(), assumes that req lies in > + * evcon->requests. Thus, enqueue the request in advance > and r > + * it in the error case. */ > + if (res != 0) > + TAILQ_REMOVE(&evcon->requests, req, next); > > return res; > } > > - TAILQ_INSERT_TAIL(&evcon->requests, req, next); > - > /* > * If it's connected already and we are the first in the queue, > * then we can dispatch this request immediately. Otherwise, it > > ***** end patch ***** > *********************************************************************** > To unsubscribe, send an e-mail to majord...@freehaven.net with > unsubscribe libevent-users in the body. >
