]> granicus.if.org Git - libevent/commitdiff
http: reset connection before installing retry timer (fix http retries handling)
authorAzat Khuzhin <a3at.mail@gmail.com>
Sat, 27 Sep 2014 17:29:45 +0000 (21:29 +0400)
committerAzat Khuzhin <a3at.mail@gmail.com>
Tue, 30 Sep 2014 14:48:15 +0000 (18:48 +0400)
This will fix some invalid read/write:
==556== Invalid read of size 8
==556==    at 0x4E4EEC6: event_queue_remove_timeout (minheap-internal.h:178)
==556==    by 0x4E508AA: event_del_nolock_ (event.c:2764)
==556==    by 0x4E53535: event_base_loop (event.c:3088)
==556==    by 0x406FCFA: dispatch (libcrawl.c:271)
==556==    by 0x402863: main (crawler.c:49)
==556==  Address 0x68a3f18 is 152 bytes inside a block of size 400 free'd
==556==    at 0x4C29C97: free (in /usr/local/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==556==    by 0x406F140: renew (libcrawl.c:625)
==556==    by 0x4E6CDE9: evhttp_connection_cb_cleanup (http.c:1331)
==556==    by 0x4E6E2B2: evhttp_connection_cb (http.c:1424)
==556==    by 0x4E4DF2D: bufferevent_writecb (bufferevent_sock.c:310)
==556==    by 0x4E52D1D: event_process_active_single_queue (event.c:1584)
==556==    by 0x4E53676: event_base_loop (event.c:1676)
==556==    by 0x406FCFA: dispatch (libcrawl.c:271)
==556==    by 0x402863: main (crawler.c:49)
But this one because of some invalid write before (I guess).

It is 100% reproduced during massive crawling (because this process
has many different servers), but after spending some time for trying to
reproduce this using some simple tests/utils I gave up for a few days (I
have a lot of work to do), but I'm sending this patch as a reminder.

Just in case, I've tried next tests:
- mixing timeouts/retries
- shutdown http server and return it back
- slow dns server for first request
- sleep before accept
- hacking libevent sources to change the behaviour of http layer (so it
  will go into that function which I'm insterested in).

http.c

diff --git a/http.c b/http.c
index 3941d1790063328240e1cb4ceb65e07f41066093..2049e4d7b3d6fe17eaadfe33664a063430400b00 100644 (file)
--- a/http.c
+++ b/http.c
@@ -1285,6 +1285,7 @@ evhttp_connection_cb_cleanup(struct evhttp_connection *evcon)
 {
        struct evcon_requestq requests;
 
+       evhttp_connection_reset_(evcon);
        if (evcon->retry_max < 0 || evcon->retry_cnt < evcon->retry_max) {
                struct timeval tv_retry = evcon->initial_retry_timeout;
                int i;
@@ -1306,7 +1307,6 @@ evhttp_connection_cb_cleanup(struct evhttp_connection *evcon)
                evcon->retry_cnt++;
                return;
        }
-       evhttp_connection_reset_(evcon);
 
        /*
         * User callback can do evhttp_make_request() on the same