[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [Libevent-users] Bug: evhttp_make_request(), introduced in git commit 0d6622e



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.ko@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.