From 14971a833c18fbab0819322f4e7b7a29f0cc6448 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Sat, 11 May 2013 03:53:11 +0400 Subject: [PATCH 1/3] Fix SEGFAULT after evdns_base_resume if no nameservers installed. If there is no nameservers installed, using evdns_base_nameserver_ip_add(), than evdns_base_resume() will SEGFAULT, because of NULL dereference in evdns_requests_pump_waiting_queue() --- evdns.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/evdns.c b/evdns.c index 534da837..7e667f82 100644 --- a/evdns.c +++ b/evdns.c @@ -742,23 +742,31 @@ request_reissue(struct request *req) { /* this function looks for space on the inflight queue and promotes */ /* requests from the waiting queue if it can. */ +/* */ +/* TODO: */ +/* add return code, see at nameserver_pick() and other functions. */ static void evdns_requests_pump_waiting_queue(struct evdns_base *base) { ASSERT_LOCKED(base); while (base->global_requests_inflight < base->global_max_requests_inflight && base->global_requests_waiting) { struct request *req; - /* move a request from the waiting queue to the inflight queue */ + EVUTIL_ASSERT(base->req_waiting_head); req = base->req_waiting_head; + + req->ns = nameserver_pick(base); + if (!req->ns) + return; + + /* move a request from the waiting queue to the inflight queue */ + req->ns->requests_inflight++; + evdns_request_remove(req, &base->req_waiting_head); base->global_requests_waiting--; base->global_requests_inflight++; - req->ns = nameserver_pick(base); - req->ns->requests_inflight++; - request_trans_id_set(req, transaction_id_pick(base)); evdns_request_insert(req, &REQ_HEAD(base, req->trans_id)); @@ -2468,6 +2476,7 @@ evdns_base_resume(struct evdns_base *base) EVDNS_LOCK(base); evdns_requests_pump_waiting_queue(base); EVDNS_UNLOCK(base); + return 0; } From 1cd9ff591deca4bc39ae84985ea0c5b157faba2b Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 13 May 2013 19:20:42 +0000 Subject: [PATCH 2/3] Add tests for evdns_base_resume(). - leak_resume - leak_cancel_and_resume - leak_resume_send_err - leak_cancel_and_resume_send_err --- test/regress_dns.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/regress_dns.c b/test/regress_dns.c index 9990b0e6..babd90e5 100644 --- a/test/regress_dns.c +++ b/test/regress_dns.c @@ -1755,6 +1755,39 @@ test_dbg_leak_cancel(void *env_) env->base = 0; } +static void +dbg_leak_resume(void *env_, int cancel, int send_err_shutdown) +{ + /* cancel, loop, free/dns, free/base */ + struct testleak_env_t *env = env_; + if (cancel) { + evdns_cancel_request(env->dns_base, env->req); + tt_assert(!evdns_base_resume(env->dns_base)); + } else { + /* TODO: No nameservers, request can't be processed, must be errored */ + tt_assert(!evdns_base_resume(env->dns_base)); + } + + event_base_loop(env->base, EVLOOP_NONBLOCK); + +end: + evdns_base_free(env->dns_base, send_err_shutdown); + env->dns_base = 0; + event_base_free(env->base); + env->base = 0; +} + +#define IMPL_DBG_LEAK_RESUME(name, cancel, send_err_shutdown) \ + static void \ + test_dbg_leak_##name##_(void *env_) \ + { \ + dbg_leak_resume(env_, cancel, send_err_shutdown); \ + } +IMPL_DBG_LEAK_RESUME(resume, 0, 0) +IMPL_DBG_LEAK_RESUME(cancel_and_resume, 1, 0) +IMPL_DBG_LEAK_RESUME(resume_send_err, 0, 1) +IMPL_DBG_LEAK_RESUME(cancel_and_resume_send_err, 1, 1) + static void test_dbg_leak_shutdown(void *env_) { @@ -1856,6 +1889,14 @@ struct testcase_t dns_testcases[] = { #ifdef EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED { "leak_shutdown", test_dbg_leak_shutdown, TT_FORK, &testleak_funcs, NULL }, { "leak_cancel", test_dbg_leak_cancel, TT_FORK, &testleak_funcs, NULL }, + + { "leak_resume", test_dbg_leak_resume_, TT_FORK, &testleak_funcs, NULL }, + { "leak_cancel_and_resume", test_dbg_leak_cancel_and_resume_, + TT_FORK, &testleak_funcs, NULL }, + { "leak_resume_send_err", test_dbg_leak_resume_send_err_, + TT_FORK, &testleak_funcs, NULL }, + { "leak_cancel_and_resume_send_err", test_dbg_leak_cancel_and_resume_send_err_, + TT_FORK, &testleak_funcs, NULL }, #endif END_OF_TESTCASES From 7e876df71b5ac8355050018a0a130002a88801cb Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 13 May 2013 19:50:30 +0000 Subject: [PATCH 3/3] Fix dns/leak_resume_send_err test. Because we don't cancel request, and want our callback to recieve DNS_ERR_SHUTDOWN, we use deferred callback, and there was - one extra malloc(), @see reply_schedule_callback() - and one missing free @see request_finished() (req->handle->pending_cb = 1) than we don't need to count in testleak_cleanup() So just decrement allocated_chunks to 2, like we already take care about it. --- test/regress_dns.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/regress_dns.c b/test/regress_dns.c index babd90e5..7bf5fa70 100644 --- a/test/regress_dns.c +++ b/test/regress_dns.c @@ -1768,6 +1768,23 @@ dbg_leak_resume(void *env_, int cancel, int send_err_shutdown) tt_assert(!evdns_base_resume(env->dns_base)); } + /** + * Because we don't cancel request, + * and want our callback to recieve DNS_ERR_SHUTDOWN, + * we use deferred callback, and there was + * - one extra malloc(), + * @see reply_schedule_callback() + * - and one missing free + * @see request_finished() (req->handle->pending_cb = 1) + * than we don't need to count in testleak_cleanup() + * + * So just decrement allocated_chunks to 2, + * like we already take care about it. + */ + if (!cancel && send_err_shutdown) { + allocated_chunks -= 2; + } + event_base_loop(env->base, EVLOOP_NONBLOCK); end: