From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Mon, 21 Mar 2016 14:09:17 +0000 (+0300)
Subject: be_sock: cancel in-progress dns requests
X-Git-Tag: release-2.1.6-beta~52^2~4
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=86dfd2ced1985ba7b2a2d4f48f2dc5d7a07ce0ab;p=libevent

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 d9d9e666..9960aefa 100644
--- 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 643aa810..ab35f879 100644
--- 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);
 }