be_sock: cancel in-progress dns requests

Before this patch we didn't have such functionality and this can cause some
issues when we are cancelling HTTP request while after this we will get a
response/timeout from the NS and in this case error_cb will be called and can
damage newly started HTTP request.
We could also have problems with connect, but we don't have them since we
closes the fd in HTTP layer.

This is not so good that this is done in be_sock, but it is internal, so we can
change without pain. Plus I don't like that callback-via-evutil, but since we
have event_extra we need do like that.

And after this patch the following tests doesn't report leaks:
$ valgrind --leak-check=full --show-reachable=yes --track-fds=yes --error-exitcode=1 regress --no-fork http/cancel..
...
==10469== FILE DESCRIPTORS: 2309 open at exit.
...
==10469== HEAP SUMMARY:
==10469==     in use at exit: 0 bytes in 0 blocks
==10469==   total heap usage: 33,846 allocs, 33,846 frees, 4,617,651 bytes allocated
==10469==
==10469== All heap blocks were freed -- no leaks are possible

v2: do under lock
v3: reset dns request
v4: ignore EVUTIL_EAI_CANCEL in regular callback

diff --git a/bufferevent-internal.h b/bufferevent-internal.h
index d9d9e66640fd18d4a1e0128c8515f6a79e5904a1..9960aefa5d10c15619f802163da0d82f2a482c72 100644 (file)
--- a/bufferevent-internal.h
+++ b/bufferevent-internal.h
@@ -228,6 +228,8 @@ struct bufferevent_private {
                struct sockaddr_in6 in6;
                struct sockaddr_in in;
        } conn_address;
+
+       struct evdns_getaddrinfo_request *dns_request;
 };
 
 /** Possible operations for a control callback. */

diff --git a/bufferevent_sock.c b/bufferevent_sock.c
index 643aa810bec0a6d3c69b6a240b9ab62b75aeb67c..ab35f8799c66096d36981cfa2392dd2876ca2f49 100644 (file)
--- a/bufferevent_sock.c
+++ b/bufferevent_sock.c
@@ -468,6 +468,13 @@ bufferevent_connect_getaddrinfo_cb(int result, struct evutil_addrinfo *ai,
        bufferevent_unsuspend_write_(bev, BEV_SUSPEND_LOOKUP);
        bufferevent_unsuspend_read_(bev, BEV_SUSPEND_LOOKUP);
 
+       bev_p->dns_request = NULL;
+
+       if (result == EVUTIL_EAI_CANCEL) {
+               bev_p->dns_error = result;
+               bufferevent_decref_and_unlock_(bev);
+               return;
+       }
        if (result != 0) {
                bev_p->dns_error = result;
                bufferevent_run_eventcb_(bev, BEV_EVENT_ERROR, 0);
@@ -514,8 +521,8 @@ bufferevent_socket_connect_hostname(struct bufferevent *bev,
        bufferevent_suspend_read_(bev, BEV_SUSPEND_LOOKUP);
 
        bufferevent_incref_(bev);
-       evutil_getaddrinfo_async_(evdns_base, hostname, portbuf,
-           &hint, bufferevent_connect_getaddrinfo_cb, bev);
+       bev_p->dns_request = evutil_getaddrinfo_async_(evdns_base, hostname,
+           portbuf, &hint, bufferevent_connect_getaddrinfo_cb, bev);
        BEV_UNLOCK(bev);
 
        return 0;
@@ -603,6 +610,8 @@ be_socket_destruct(struct bufferevent *bufev)
 
        if ((bufev_p->options & BEV_OPT_CLOSE_ON_FREE) && fd >= 0)
                EVUTIL_CLOSESOCKET(fd);
+
+       evutil_getaddrinfo_cancel_async_(bufev_p->dns_request);
 }
 
 static int
@@ -616,6 +625,9 @@ be_socket_flush(struct bufferevent *bev, short iotype,
 static void
 be_socket_setfd(struct bufferevent *bufev, evutil_socket_t fd)
 {
+       struct bufferevent_private *bufev_p =
+           EVUTIL_UPCAST(bufev, struct bufferevent_private, bev);
+
        BEV_LOCK(bufev);
        EVUTIL_ASSERT(bufev->be_ops == &bufferevent_ops_socket);
 
@@ -633,6 +645,8 @@ be_socket_setfd(struct bufferevent *bufev, evutil_socket_t fd)
        if (fd >= 0)
                bufferevent_enable(bufev, bufev->enabled);
 
+       evutil_getaddrinfo_cancel_async_(bufev_p->dns_request);
+
        BEV_UNLOCK(bufev);
 }