Skip to content

Commit

Permalink
be_sock: cancel in-progress dns requests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
azat committed Mar 23, 2016
1 parent 8cbe65d commit 86dfd2c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
2 changes: 2 additions & 0 deletions bufferevent-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
18 changes: 16 additions & 2 deletions bufferevent_sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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);
}

Expand Down

0 comments on commit 86dfd2c

Please sign in to comment.