From 227510d5770f7e568a2d434885ed7abd1c43d1a1 Mon Sep 17 00:00:00 2001 From: Vladislav Gusev Date: Thu, 13 Jul 2023 22:20:33 +0300 Subject: [PATCH] Fix EVDNS_BASE_DISABLE_WHEN_INACTIVE (#1493) I faced with strange problem: event loop doesn't exit after dns resolving with `EVDNS_BASE_DISABLE_WHEN_INACTIVE`. Stand: - Ubuntu 22; - libevent release-2.1.12-stable - `resolve.conf` contains 2 nameservers; - I use `evdns_base_new` with `EVDNS_BASE_DISABLE_WHEN_INACTIVE | EVDNS_BASE_INITIALIZE_NAMESERVERS` to avoid OS specific code. After small investigation, look like events related with dns sockets added to event_base before `evdns->disable_when_inactive` was initialized. `libevent` did epoll_ctl(DEL) after resolving completed on the first socket, but the second socket remained in the `epoll` interest list. --- evdns.c | 9 +++--- evutil.c | 17 ++++++++++ test/regress_dns.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++ util-internal.h | 6 ++++ 4 files changed, 106 insertions(+), 4 deletions(-) diff --git a/evdns.c b/evdns.c index 4e8ee21d..19efbde9 100644 --- a/evdns.c +++ b/evdns.c @@ -4880,6 +4880,10 @@ evdns_base_new(struct event_base *event_base, int flags) } #undef EVDNS_BASE_ALL_FLAGS + if (flags & EVDNS_BASE_DISABLE_WHEN_INACTIVE) { + base->disable_when_inactive = 1; + } + if (flags & EVDNS_BASE_INITIALIZE_NAMESERVERS) { int r; int opts = DNS_OPTIONS_ALL; @@ -4890,16 +4894,13 @@ evdns_base_new(struct event_base *event_base, int flags) #ifdef _WIN32 r = evdns_base_config_windows_nameservers(base); #else - r = evdns_base_resolv_conf_parse(base, opts, "/etc/resolv.conf"); + r = evdns_base_resolv_conf_parse(base, opts, evutil_resolvconf_filename_()); #endif if (r && (EVDNS_ERROR_NO_NAMESERVERS_CONFIGURED != r)) { evdns_base_free_and_unlock(base, 0); return NULL; } } - if (flags & EVDNS_BASE_DISABLE_WHEN_INACTIVE) { - base->disable_when_inactive = 1; - } EVDNS_UNLOCK(base); return base; diff --git a/evutil.c b/evutil.c index 84c0dcc4..4308bf2e 100644 --- a/evutil.c +++ b/evutil.c @@ -1849,6 +1849,23 @@ evutil_set_evdns_getaddrinfo_cancel_fn_(evdns_getaddrinfo_cancel_fn fn) evdns_getaddrinfo_cancel_impl = fn; } +static const char *evutil_custom_resolvconf_filename = NULL; + +void +evutil_set_resolvconf_filename_(const char *filename) +{ + evutil_custom_resolvconf_filename = filename; +} + +const char * +evutil_resolvconf_filename_(void) +{ + if (evutil_custom_resolvconf_filename) + return evutil_custom_resolvconf_filename; + + return "/etc/resolv.conf"; +} + /* 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. diff --git a/test/regress_dns.c b/test/regress_dns.c index 959f2f10..992c10e4 100644 --- a/test/regress_dns.c +++ b/test/regress_dns.c @@ -1185,6 +1185,80 @@ end: if (dns) evdns_base_free(dns, 0); } + +static const char *dns_resolvconf_with_one_nameserver = + "nameserver 127.0.0.53\n"; + +static void +dns_initialize_inactive_one_nameserver_test(void *arg) +{ + struct basic_test_data *data = arg; + struct event_base *base = data->base; + struct evdns_base *dns = NULL; + + char *filename = NULL; + + tt_int_op(regress_make_tmpfile(dns_resolvconf_with_one_nameserver, + strlen(dns_resolvconf_with_one_nameserver), &filename), + !=, -1); + + tt_assert(filename); + + evutil_set_resolvconf_filename_(filename); + + dns = evdns_base_new(base, + EVDNS_BASE_INITIALIZE_NAMESERVERS | EVDNS_BASE_DISABLE_WHEN_INACTIVE); + tt_assert(dns); + + tt_int_op(event_base_loop(base, EVLOOP_NONBLOCK), ==, 1); + +end: + if (dns) + evdns_base_free(dns, 0); + if (filename) { + unlink(filename); + free(filename); + } + evutil_set_resolvconf_filename_(NULL); +} + +static const char *dns_resolvconf_with_two_nameservers = + "nameserver 127.0.0.53\n" + "nameserver 127.0.0.53\n"; + +static void +dns_initialize_inactive_two_nameservers_test(void *arg) +{ + struct basic_test_data *data = arg; + struct event_base *base = data->base; + struct evdns_base *dns = NULL; + + char *filename = NULL; + + tt_int_op(regress_make_tmpfile(dns_resolvconf_with_two_nameservers, + strlen(dns_resolvconf_with_two_nameservers), &filename), + !=, -1); + + tt_assert(filename); + + evutil_set_resolvconf_filename_(filename); + + dns = evdns_base_new(base, + EVDNS_BASE_INITIALIZE_NAMESERVERS | EVDNS_BASE_DISABLE_WHEN_INACTIVE); + tt_assert(dns); + + tt_int_op(event_base_loop(base, EVLOOP_NONBLOCK), ==, 1); + +end: + if (dns) + evdns_base_free(dns, 0); + if (filename) { + unlink(filename); + free(filename); + } + evutil_set_resolvconf_filename_(NULL); +} + #ifndef _WIN32 #define RESOLV_FILE "empty-resolv.conf" static void @@ -3036,6 +3110,10 @@ struct testcase_t dns_testcases[] = { { "initialize_nameservers", dns_initialize_nameservers_test, TT_FORK|TT_NEED_BASE, &basic_setup, NULL }, #ifndef _WIN32 + {"initialize_with_one_inactive_nameserver", dns_initialize_inactive_one_nameserver_test, + TT_FORK | TT_NEED_BASE, &basic_setup, NULL}, + {"initialize_with_two_inactive_nameservers", dns_initialize_inactive_two_nameservers_test, + TT_FORK | TT_NEED_BASE, &basic_setup, NULL}, { "nameservers_no_default", dns_nameservers_no_default_test, TT_FORK|TT_NEED_BASE, &basic_setup, NULL }, { "no_nameservers_configured", dns_nameservers_no_nameservers_configured_test, diff --git a/util-internal.h b/util-internal.h index cfb3f3a7..19c5de93 100644 --- a/util-internal.h +++ b/util-internal.h @@ -417,6 +417,12 @@ typedef void (*evdns_getaddrinfo_cancel_fn)( EVENT2_EXPORT_SYMBOL void evutil_set_evdns_getaddrinfo_cancel_fn_(evdns_getaddrinfo_cancel_fn fn); +/* Customization point to override "/etc/resolv.conf" */ +EVENT2_EXPORT_SYMBOL +void evutil_set_resolvconf_filename_(const char *filename); +EVENT2_EXPORT_SYMBOL +const char *evutil_resolvconf_filename_(void); + EVENT2_EXPORT_SYMBOL struct evutil_addrinfo *evutil_new_addrinfo_(struct sockaddr *sa, ev_socklen_t socklen, const struct evutil_addrinfo *hints);