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); } diff --git a/evdns.c b/evdns.c index c4112330..152ba766 100644 --- a/evdns.c +++ b/evdns.c @@ -3908,6 +3908,7 @@ evdns_base_new(struct event_base *event_base, int flags) * functionality. We can't just call evdns_getaddrinfo directly or * else libevent-core will depend on libevent-extras. */ evutil_set_evdns_getaddrinfo_fn_(evdns_getaddrinfo); + evutil_set_evdns_getaddrinfo_cancel_fn_(evdns_getaddrinfo_cancel); base = mm_malloc(sizeof(struct evdns_base)); if (base == NULL) diff --git a/evutil.c b/evutil.c index 495bfcc0..06b4e948 100644 --- a/evutil.c +++ b/evutil.c @@ -1546,6 +1546,7 @@ evutil_freeaddrinfo(struct evutil_addrinfo *ai) } static evdns_getaddrinfo_fn evdns_getaddrinfo_impl = NULL; +static evdns_getaddrinfo_cancel_fn evdns_getaddrinfo_cancel_impl = NULL; void evutil_set_evdns_getaddrinfo_fn_(evdns_getaddrinfo_fn fn) @@ -1553,27 +1554,40 @@ evutil_set_evdns_getaddrinfo_fn_(evdns_getaddrinfo_fn fn) if (!evdns_getaddrinfo_impl) evdns_getaddrinfo_impl = fn; } +void +evutil_set_evdns_getaddrinfo_cancel_fn_(evdns_getaddrinfo_cancel_fn fn) +{ + if (!evdns_getaddrinfo_cancel_impl) + evdns_getaddrinfo_cancel_impl = fn; +} /* Internal helper function: act like evdns_getaddrinfo if dns_base is set; * otherwise do a blocking resolve and pass the result to the callback in the * way that evdns_getaddrinfo would. */ -int -evutil_getaddrinfo_async_(struct evdns_base *dns_base, +struct evdns_getaddrinfo_request *evutil_getaddrinfo_async_( + struct evdns_base *dns_base, const char *nodename, const char *servname, const struct evutil_addrinfo *hints_in, void (*cb)(int, struct evutil_addrinfo *, void *), void *arg) { if (dns_base && evdns_getaddrinfo_impl) { - evdns_getaddrinfo_impl( + return evdns_getaddrinfo_impl( dns_base, nodename, servname, hints_in, cb, arg); } else { struct evutil_addrinfo *ai=NULL; int err; err = evutil_getaddrinfo(nodename, servname, hints_in, &ai); cb(err, ai, arg); + return NULL; + } +} + +void evutil_getaddrinfo_cancel_async_(struct evdns_getaddrinfo_request *data) +{ + if (evdns_getaddrinfo_cancel_impl && data) { + evdns_getaddrinfo_cancel_impl(data); } - return 0; } const char * diff --git a/http.c b/http.c index cce68ce6..ac146075 100644 --- a/http.c +++ b/http.c @@ -1244,6 +1244,9 @@ evhttp_connection_free(struct evhttp_connection *evcon) event_deferred_cb_cancel_(get_deferred_queue(evcon), &evcon->read_more_deferred_cb); + if (evcon->fd == -1) + evcon->fd = bufferevent_getfd(evcon->bufev); + if (evcon->fd != -1) { bufferevent_disable(evcon->bufev, EV_READ|EV_WRITE); shutdown(evcon->fd, EVUTIL_SHUT_WR); @@ -1329,6 +1332,9 @@ evhttp_connection_reset_(struct evhttp_connection *evcon) */ bufferevent_disable_hard_(evcon->bufev, EV_READ|EV_WRITE); + if (evcon->fd == -1) + evcon->fd = bufferevent_getfd(evcon->bufev); + if (evcon->fd != -1) { /* inform interested parties about connection close */ if (evhttp_connected(evcon) && evcon->closecb != NULL) @@ -1336,9 +1342,9 @@ evhttp_connection_reset_(struct evhttp_connection *evcon) shutdown(evcon->fd, EVUTIL_SHUT_WR); evutil_closesocket(evcon->fd); - bufferevent_setfd(evcon->bufev, -1); evcon->fd = -1; } + bufferevent_setfd(evcon->bufev, -1); /* we need to clean up any buffered data */ tmp = bufferevent_get_output(evcon->bufev); diff --git a/test/regress_http.c b/test/regress_http.c index 22e71add..1906da80 100644 --- a/test/regress_http.c +++ b/test/regress_http.c @@ -1268,6 +1268,21 @@ http_request_never_call(struct evhttp_request *req, void *arg) fprintf(stdout, "FAILED\n"); exit(1); } +static void +http_failed_request_done(struct evhttp_request *req, void *arg) +{ + tt_assert(!req); +end: + event_base_loopexit(arg, NULL); +} +static void +http_timed_out_request_done(struct evhttp_request *req, void *arg) +{ + tt_assert(req); + tt_int_op(evhttp_request_get_response_code(req), !=, HTTP_OK); +end: + event_base_loopexit(arg, NULL); +} static void http_request_error_cb_with_cancel(enum evhttp_request_error error, void *arg) @@ -1293,7 +1308,59 @@ http_do_cancel(evutil_socket_t fd, short what, void *arg) evhttp_cancel_request(req); ++test_ok; } +static void +http_no_write(struct evbuffer *buffer, const struct evbuffer_cb_info *info, void *arg) +{ + fprintf(stdout, "FAILED\n"); + exit(1); +} +static void +http_free_evcons(struct evhttp_connection **evcons) +{ + if (!evcons) + return; + struct evhttp_connection *evcon, **orig = evcons; + while ((evcon = *evcons++)) { + evhttp_connection_free(evcon); + } + free(orig); +} +/** fill the backlog to force server drop packages for timeouts */ +static struct evhttp_connection ** +http_fill_backlog(struct event_base *base, int port) +{ +#define BACKLOG_SIZE 256 + struct evhttp_connection **evcon = malloc(sizeof(*evcon) * (BACKLOG_SIZE + 1)); + int i; + + for (i = 0; i < BACKLOG_SIZE; ++i) { + struct evhttp_request *req; + + evcon[i] = evhttp_connection_base_new(base, NULL, "127.0.0.1", port); + tt_assert(evcon[i]); + evhttp_connection_set_timeout(evcon[i], 5); + + req = evhttp_request_new(http_request_never_call, NULL); + tt_assert(req); + tt_int_op(evhttp_make_request(evcon[i], req, EVHTTP_REQ_GET, "/delay"), !=, -1); + } + evcon[i] = NULL; + + return evcon; + end: + fprintf(stderr, "Couldn't fill the backlog"); + return NULL; +} + +enum http_cancel_test_type { + BASIC = 1, + BY_HOST = 2, + NO_NS = 4, + INACTIVE_SERVER = 8, + SERVER_TIMEOUT = 16, + NS_TIMEOUT = 32, +}; static void http_cancel_test(void *arg) { @@ -1301,7 +1368,36 @@ http_cancel_test(void *arg) ev_uint16_t port = 0; struct evhttp_connection *evcon = NULL; struct evhttp_request *req = NULL; + struct bufferevent *bufev = NULL; struct timeval tv; + struct evdns_base *dns_base = NULL; + ev_uint16_t portnum = 0; + char address[64]; + struct evhttp *inactive_http = NULL; + struct event_base *inactive_base = NULL; + struct evhttp_connection **evcons = NULL; + + enum http_cancel_test_type type; + type = (enum http_cancel_test_type)data->setup_data; + + if (type & BY_HOST) { + tt_assert(regress_dnsserver(data->base, &portnum, search_table)); + + dns_base = evdns_base_new(data->base, 0/* init name servers */); + tt_assert(dns_base); + + /** XXX: Hack the port to make timeout after resolving */ + if (type & NO_NS) + ++portnum; + + evutil_snprintf(address, sizeof(address), "127.0.0.1:%d", portnum); + evdns_base_nameserver_ip_add(dns_base, address); + + char *timeout = (type & NS_TIMEOUT) ? "6" : "3"; + evdns_base_set_option(dns_base, "timeout:", timeout); + evdns_base_set_option(dns_base, "initial-probe-timeout:", timeout); + evdns_base_set_option(dns_base, "attempts:", "1"); + } exit_base = data->base; @@ -1309,9 +1405,29 @@ http_cancel_test(void *arg) http = http_setup(&port, data->base, 0); - evcon = evhttp_connection_base_new(data->base, NULL, "127.0.0.1", port); + if (type & INACTIVE_SERVER) { + port = 0; + inactive_base = event_base_new(); + inactive_http = http_setup(&port, inactive_base, 0); + } + + if (type & SERVER_TIMEOUT) + evcons = http_fill_backlog(inactive_base ?: data->base, port); + + evcon = evhttp_connection_base_new( + data->base, dns_base, + type & BY_HOST ? "localhost" : "127.0.0.1", + port); + if (type & INACTIVE_SERVER) + evhttp_connection_set_timeout(evcon, 5); tt_assert(evcon); + bufev = evhttp_connection_get_bufferevent(evcon); + /* Guarantee that we stack in connect() not after waiting EV_READ after + * write() */ + if (type & SERVER_TIMEOUT) + evbuffer_add_cb(bufferevent_get_output(bufev), http_no_write, NULL); + /* * At this point, we want to schedule a request to the HTTP * server using our make request method. @@ -1335,12 +1451,24 @@ http_cancel_test(void *arg) event_base_dispatch(data->base); - tt_int_op(test_ok, ==, 3); + if (type & NO_NS || type & INACTIVE_SERVER) + tt_int_op(test_ok, ==, 2); /** no servers responses */ + else + tt_int_op(test_ok, ==, 3); /* try to make another request over the same connection */ test_ok = 0; - req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY); + http_free_evcons(evcons); + if (type & SERVER_TIMEOUT) + evcons = http_fill_backlog(inactive_base ?: data->base, port); + + if (!(type & NO_NS) && (type & SERVER_TIMEOUT)) + req = evhttp_request_new(http_timed_out_request_done, data->base); + else if ((type & INACTIVE_SERVER) || (type & NO_NS)) + req = evhttp_request_new(http_failed_request_done, data->base); + else + req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY); /* Add the information that we care about */ evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost"); @@ -1354,7 +1482,16 @@ http_cancel_test(void *arg) /* make another request: request empty reply */ test_ok = 0; - req = evhttp_request_new(http_request_empty_done, data->base); + http_free_evcons(evcons); + if (type & SERVER_TIMEOUT) + evcons = http_fill_backlog(inactive_base ?: data->base, port); + + if (!(type & NO_NS) && (type & SERVER_TIMEOUT)) + req = evhttp_request_new(http_timed_out_request_done, data->base); + else if ((type & INACTIVE_SERVER) || (type & NO_NS)) + req = evhttp_request_new(http_failed_request_done, data->base); + else + req = evhttp_request_new(http_request_empty_done, data->base); /* Add the information that we care about */ evhttp_add_header(evhttp_request_get_output_headers(req), "Empty", "itis"); @@ -1366,10 +1503,20 @@ http_cancel_test(void *arg) event_base_dispatch(data->base); end: + http_free_evcons(evcons); + if (bufev) + evbuffer_remove_cb(bufferevent_get_output(bufev), http_no_write, NULL); if (evcon) evhttp_connection_free(evcon); if (http) evhttp_free(http); + if (dns_base) + evdns_base_free(dns_base, 0); + regress_clean_dnsserver(); + if (inactive_http) + evhttp_free(inactive_http); + if (inactive_base) + event_base_free(inactive_base); } static void @@ -3770,13 +3917,6 @@ http_large_entity_test_done(struct evhttp_request *req, void *arg) end: event_base_loopexit(arg, NULL); } -static void -http_failed_request_done(struct evhttp_request *req, void *arg) -{ - tt_assert(!req); -end: - event_base_loopexit(arg, NULL); -} #ifndef WIN32 static void http_expectation_failed_done(struct evhttp_request *req, void *arg) @@ -3910,13 +4050,6 @@ static void http_data_length_constraints_test(void *arg) static void http_read_on_write_error_test(void *arg) { http_data_length_constraints_test_impl(arg, 1); } -static void -http_large_entity_non_lingering_test_done(struct evhttp_request *req, void *arg) -{ - tt_assert(!req); -end: - event_base_loopexit(arg, NULL); -} static void http_lingering_close_test_impl(void *arg, int lingering) { @@ -3951,7 +4084,7 @@ http_lingering_close_test_impl(void *arg, int lingering) if (lingering) cb = http_large_entity_test_done; else - cb = http_large_entity_non_lingering_test_done; + cb = http_failed_request_done; req = evhttp_request_new(cb, data->base); tt_assert(req); evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost"); @@ -4342,8 +4475,10 @@ http_request_own_test(void *arg) { #name, run_legacy_test_fn, TT_ISOLATED|TT_LEGACY, &legacy_setup, \ http_##name##_test } -#define HTTP(name) \ - { #name, http_##name##_test, TT_ISOLATED, &basic_setup, NULL } +#define HTTP_CAST_ARG(a) ((void *)(a)) +#define HTTP_N(title, name, arg) \ + { #title, http_##name##_test, TT_ISOLATED, &basic_setup, HTTP_CAST_ARG(arg) } +#define HTTP(name) HTTP_N(name, name, NULL) #define HTTPS(name) \ { "https_" #name, https_##name##_test, TT_ISOLATED, &basic_setup, NULL } @@ -4382,7 +4517,20 @@ struct testcase_t http_testcases[] = { { "uriencode", http_uriencode_test, 0, NULL, NULL }, HTTP(basic), HTTP(simple), - HTTP(cancel), + + HTTP_N(cancel, cancel, BASIC), + HTTP_N(cancel_by_host, cancel, BY_HOST), + HTTP_N(cancel_by_host_no_ns, cancel, BY_HOST | NO_NS), + HTTP_N(cancel_by_host_inactive_server, cancel, BY_HOST | INACTIVE_SERVER), + HTTP_N(cancel_inactive_server, cancel, INACTIVE_SERVER), + HTTP_N(cancel_by_host_no_ns_inactive_server, cancel, BY_HOST | NO_NS | INACTIVE_SERVER), + HTTP_N(cancel_by_host_server_timeout, cancel, BY_HOST | INACTIVE_SERVER | SERVER_TIMEOUT), + HTTP_N(cancel_server_timeout, cancel, INACTIVE_SERVER | SERVER_TIMEOUT), + HTTP_N(cancel_by_host_no_ns_server_timeout, cancel, BY_HOST | NO_NS | INACTIVE_SERVER | SERVER_TIMEOUT), + HTTP_N(cancel_by_host_ns_timeout, cancel, BY_HOST | NO_NS | NS_TIMEOUT), + HTTP_N(cancel_by_host_ns_timeout_inactive_server, cancel, BY_HOST | NO_NS | NS_TIMEOUT | INACTIVE_SERVER), + HTTP_N(cancel_by_host_ns_timeout_server_timeout, cancel, BY_HOST | NO_NS | NS_TIMEOUT | INACTIVE_SERVER | SERVER_TIMEOUT), + HTTP(virtual_host), HTTP(post), HTTP(put), diff --git a/util-internal.h b/util-internal.h index ff6c5e48..b851e2ae 100644 --- a/util-internal.h +++ b/util-internal.h @@ -357,8 +357,10 @@ typedef struct evdns_getaddrinfo_request* (*evdns_getaddrinfo_fn)( const char *nodename, const char *servname, const struct evutil_addrinfo *hints_in, void (*cb)(int, struct evutil_addrinfo *, void *), void *arg); - void evutil_set_evdns_getaddrinfo_fn_(evdns_getaddrinfo_fn fn); +typedef void (*evdns_getaddrinfo_cancel_fn)( + struct evdns_getaddrinfo_request *req); +void evutil_set_evdns_getaddrinfo_cancel_fn_(evdns_getaddrinfo_cancel_fn fn); struct evutil_addrinfo *evutil_new_addrinfo_(struct sockaddr *sa, ev_socklen_t socklen, const struct evutil_addrinfo *hints); @@ -368,10 +370,12 @@ void evutil_adjust_hints_for_addrconfig_(struct evutil_addrinfo *hints); int evutil_getaddrinfo_common_(const char *nodename, const char *servname, struct evutil_addrinfo *hints, struct evutil_addrinfo **res, int *portnum); -int evutil_getaddrinfo_async_(struct evdns_base *dns_base, +struct evdns_getaddrinfo_request *evutil_getaddrinfo_async_( + struct evdns_base *dns_base, const char *nodename, const char *servname, const struct evutil_addrinfo *hints_in, void (*cb)(int, struct evutil_addrinfo *, void *), void *arg); +void evutil_getaddrinfo_cancel_async_(struct evdns_getaddrinfo_request *data); /** Return true iff sa is a looback address. (That is, it is 127.0.0.1/8, or * ::1). */