From: Nick Mathewson Date: Mon, 3 Aug 2009 20:15:32 +0000 (+0000) Subject: Fix an annoying evdns crash bug, and add more unit tests for evdns. X-Git-Tag: release-2.0.3-alpha~132 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=213dc2a2ef719a310de867cbae858e98360952d0;p=libevent Fix an annoying evdns crash bug, and add more unit tests for evdns. svn:r1406 --- diff --git a/evdns.c b/evdns.c index 9c16c1fd..60c836c1 100644 --- a/evdns.c +++ b/evdns.c @@ -623,7 +623,7 @@ static void request_finished(struct evdns_request *const req, struct evdns_request **head) { struct evdns_base *base = req->base; int was_inflight = (head != &base->req_waiting_head); - EVDNS_LOCK(req->base); + EVDNS_LOCK(base); if (head) evdns_request_remove(req, head); @@ -648,6 +648,7 @@ request_finished(struct evdns_request *const req, struct evdns_request **head) { mm_free(req); evdns_requests_pump_waiting_queue(base); + EVDNS_UNLOCK(base); } /* This is called when a server returns a funny error code. */ @@ -849,6 +850,8 @@ reply_handle(struct evdns_request *const req, u16 flags, u32 ttl, struct reply * } else { /* all ok, tell the user */ reply_schedule_callback(req, ttl, 0, reply); + if (req == req->ns->probe_request) + req->ns->probe_request = NULL; /* Avoid double-free */ nameserver_up(req->ns); request_finished(req, &REQ_HEAD(req->base, req->trans_id)); } @@ -2075,11 +2078,12 @@ evdns_server_request_get_requesting_addr(struct evdns_server_request *_req, stru static void evdns_request_timeout_callback(evutil_socket_t fd, short events, void *arg) { struct evdns_request *const req = (struct evdns_request *) arg; + struct evdns_base *base = req->base; (void) fd; (void) events; log(EVDNS_LOG_DEBUG, "Request %lx timed out", (unsigned long) arg); - EVDNS_LOCK(req->base); + EVDNS_LOCK(base); req->ns->timedout++; if (req->ns->timedout > req->base->global_max_nameserver_timeout) { @@ -2096,7 +2100,7 @@ evdns_request_timeout_callback(evutil_socket_t fd, short events, void *arg) { (void) evtimer_del(&req->timeout_event); evdns_request_transmit(req); } - EVDNS_UNLOCK(req->base); + EVDNS_UNLOCK(base); } /* try to send a request to a given server. */ @@ -2502,6 +2506,27 @@ static void evdns_request_remove(struct evdns_request *req, struct evdns_request **head) { ASSERT_LOCKED(req->base); + +#if 0 + { + struct evdns_request *ptr; + int found = 0; + assert(*head != NULL); + + ptr = *head; + do { + if (ptr == req) { + found = 1; + break; + } + ptr = ptr->next; + } while (ptr != *head); + assert(found); + + assert(req->next); + } +#endif + if (req->next == req) { /* only item in the list */ *head = NULL; @@ -2510,6 +2535,7 @@ evdns_request_remove(struct evdns_request *req, struct evdns_request **head) req->prev->next = req->next; if (*head == req) *head = req->next; } + req->next = req->prev = NULL; } /* insert into the tail of the queue */ @@ -3061,13 +3087,27 @@ strtok_r(char *s, const char *delim, char **state) { /* helper version of atoi which returns -1 on error */ static int -strtoint(const char *const str) { +strtoint(const char *const str) +{ char *endptr; const int r = strtol(str, &endptr, 10); if (*endptr) return -1; return r; } +/* Parse a number of seconds into a timeval; return -1 on error. */ +static int +strtotimeval(const char *const str, struct timeval *out) +{ + double d; + char *endptr; + d = strtod(str, &endptr); + if (*endptr) return -1; + out->tv_sec = (int) d; + out->tv_usec = (int) ((d - (int) d)*1000000); + return 0; +} + /* helper version of atoi that returns -1 on error and clips to bounds. */ static int strtoint_clipped(const char *const str, int min, int max) @@ -3142,11 +3182,11 @@ evdns_base_set_option_impl(struct evdns_base *base, if (!base->global_search_state) return -1; base->global_search_state->ndots = ndots; } else if (!strncmp(option, "timeout:", 8)) { - const int timeout = strtoint(val); - if (timeout == -1) return -1; + struct timeval tv; + if (strtotimeval(val, &tv) == -1) return -1; if (!(flags & DNS_OPTION_MISC)) return 0; - log(EVDNS_LOG_DEBUG, "Setting timeout to %d", timeout); - base->global_timeout.tv_sec = timeout; + log(EVDNS_LOG_DEBUG, "Setting timeout to %s", val); + memcpy(&base->global_timeout, &tv, sizeof(struct timeval)); } else if (!strncmp(option, "max-timeouts:", 12)) { const int maxtimeout = strtoint_clipped(val, 1, 255); if (maxtimeout == -1) return -1; diff --git a/test/regress_dns.c b/test/regress_dns.c index be93bc76..ed7e6a1e 100644 --- a/test/regress_dns.c +++ b/test/regress_dns.c @@ -452,7 +452,8 @@ end: static struct evdns_server_port * get_generic_server(struct event_base *base, ev_uint64_t portnum, - struct generic_dns_server_table *tab) + evdns_request_callback_fn_type cb, + void *arg) { struct evdns_server_port *port = NULL; evutil_socket_t sock; @@ -472,7 +473,7 @@ get_generic_server(struct event_base *base, if (bind(sock, (struct sockaddr*)&my_addr, sizeof(my_addr)) < 0) { tt_abort_perror("bind"); } - port = evdns_add_server_port_with_base(base, sock, 0, generic_dns_server_cb, tab); + port = evdns_add_server_port_with_base(base, sock, 0, cb, arg); return port; end: @@ -544,7 +545,8 @@ dns_search_test(void *arg) struct generic_dns_callback_result r1, r2, r3, r4, r5; - port = get_generic_server(base, 53900, search_table); + port = get_generic_server(base, 53900, generic_dns_server_cb, + search_table); tt_assert(port); dns = evdns_base_new(base, 0); @@ -576,10 +578,161 @@ dns_search_test(void *arg) tt_int_op(r5.result, ==, DNS_ERR_NOTEXIST); end: + if (dns) + evdns_base_free(dns, 0); if (port) evdns_close_server_port(port); } +static void +fail_server_cb(struct evdns_server_request *req, void *data) +{ + const char *question; + int *count = data; + struct in_addr in; + + /* Drop the first N requests that we get. */ + if (*count > 0) { + --*count; + tt_want(! evdns_server_request_drop(req)); + return; + } + + if (req->nquestions != 1) + TT_DIE(("Only handling one question at a time; got %d", + req->nquestions)); + + question = req->questions[0]->name; + + if (!evutil_ascii_strcasecmp(question, "google.com")) { + /* Detect a probe, and get out of the loop. */ + event_base_loopexit(exit_base, NULL); + } + + evutil_inet_pton(AF_INET, "16.32.64.128", &in); + evdns_server_request_add_a_reply(req, question, 1, &in.s_addr, + 100); + tt_assert(! evdns_server_request_respond(req, 0)) + return; +end: + tt_want(! evdns_server_request_drop(req)); +} + +static void +dns_retry_test(void *arg) +{ + struct basic_test_data *data = arg; + struct event_base *base = data->base; + struct evdns_server_port *port = NULL; + struct evdns_base *dns = NULL; + int drop_count = 2; + + struct generic_dns_callback_result r1; + + port = get_generic_server(base, 53900, fail_server_cb, + &drop_count); + tt_assert(port); + + dns = evdns_base_new(base, 0); + tt_assert(!evdns_base_nameserver_ip_add(dns, "127.0.0.1:53900")); + tt_assert(! evdns_base_set_option(dns, "timeout:", "0.3", DNS_OPTIONS_ALL)); + tt_assert(! evdns_base_set_option(dns, "max-timeouts:", "10", DNS_OPTIONS_ALL)); + + evdns_base_resolve_ipv4(dns, "host.example.com", 0, + generic_dns_callback, &r1); + + n_replies_left = 1; + exit_base = base; + + event_base_dispatch(base); + + tt_int_op(drop_count, ==, 0); + + tt_int_op(r1.type, ==, DNS_IPv4_A); + tt_int_op(r1.count, ==, 1); + tt_int_op(((ev_uint32_t*)r1.addrs)[0], ==, htonl(0x10204080)); + + + /* Now try again, but this time have the server get treated as + * failed, so we can send it a test probe. */ + drop_count = 4; + tt_assert(! evdns_base_set_option(dns, "max-timeouts:", "3", DNS_OPTIONS_ALL)); + tt_assert(! evdns_base_set_option(dns, "attempts:", "4", DNS_OPTIONS_ALL)); + memset(&r1, 0, sizeof(r1)); + + evdns_base_resolve_ipv4(dns, "host.example.com", 0, + generic_dns_callback, &r1); + + n_replies_left = 2; + + /* This will run until it answers the "google.com" probe request. */ + /* XXXX It takes 10 seconds to retry the probe, which makes the test + * slow. */ + event_base_dispatch(base); + + /* We'll treat the server as failed here. */ + tt_int_op(r1.result, ==, DNS_ERR_TIMEOUT); + + /* It should work this time. */ + tt_int_op(drop_count, ==, 0); + evdns_base_resolve_ipv4(dns, "host.example.com", 0, + generic_dns_callback, &r1); + + event_base_dispatch(base); + tt_int_op(r1.type, ==, DNS_IPv4_A); + tt_int_op(((ev_uint32_t*)r1.addrs)[0], ==, htonl(0x10204080)); + +end: + if (dns) + evdns_base_free(dns, 0); + if (port) + evdns_close_server_port(port); +} + +#if 0 +static void +dns_probe_test(void *arg) +{ + struct basic_test_data *data = arg; + struct event_base *base = data->base; + struct evdns_server_port *port = NULL; + struct evdns_base *dns = NULL; + int drop_count = 4; + + struct generic_dns_callback_result r1; + + port = get_generic_server(base, 53900, fail_twice_server_cb, + &drop_count); + tt_assert(port); + + dns = evdns_base_new(base, 0); + tt_assert(!evdns_base_nameserver_ip_add(dns, "127.0.0.1:53900")); + tt_assert(! evdns_base_set_option(dns, "timeout:", "0.2", DNS_OPTIONS_ALL)); + tt_assert(! evdns_base_set_option(dns, "max-timeouts:", "4", DNS_OPTIONS_ALL)); + + evdns_base_resolve_ipv4(dns, "host.example.com", 0, + generic_dns_callback, &r1); + + n_replies_left = 1; + exit_base = base; + + event_base_dispatch(base); + + tt_int_op(drop_count, ==, 1); + + tt_int_op(r1.type, ==, DNS_IPv4_A); + tt_int_op(r1.count, ==, 1); + tt_int_op(((ev_uint32_t*)r1.addrs)[0], ==, htonl(0x10204080)); + + event_base_dispatch(base); + +end: + if (dns) + evdns_base_free(dns, 0); + if (port) + evdns_close_server_port(port); +} +#endif #define DNS_LEGACY(name, flags) \ { #name, run_legacy_test_fn, flags|TT_LEGACY, &legacy_setup, \ @@ -592,6 +745,7 @@ struct testcase_t dns_testcases[] = { DNS_LEGACY(gethostbyaddr, TT_FORK|TT_NEED_BASE|TT_NEED_DNS), { "resolve_reverse", dns_resolve_reverse, TT_FORK, NULL, NULL }, { "search", dns_search_test, TT_FORK|TT_NEED_BASE, &basic_setup, NULL }, + { "retry", dns_retry_test, TT_FORK|TT_NEED_BASE, &basic_setup, NULL }, END_OF_TESTCASES };