From 7dc54fa6f2da81162f4a12469a775c8bc7b48bbd Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 25 May 2021 22:54:17 -0700 Subject: [PATCH] Refactor dispatch, resolver and request Since every dispsock was associated with a dispentry anyway (though not always vice versa), the members of dispsock have been combined into dispentry, which is now reference-counted. dispentry objects are now attached before connecting and detached afterward to prevent races between the connect callback and dns_dispatch_removeresponse(). Dispatch and dispatchmgr objects are now reference counted as well, and the shutdown process has been simplified. reference counting of resquery and request objects has also been cleaned up significantly. dns_dispatch_cancel() now flags a dispentry as having been canceled, so that if the connect callback runs after cancellation, it will not initiate a read. The isblackholed() function has been simplified. --- bin/named/server.c | 2 +- bin/nsupdate/nsupdate.c | 2 +- bin/tests/system/pipelined/pipequeries.c | 2 +- bin/tests/system/tkey/keycreate.c | 2 +- bin/tests/system/tkey/keydelete.c | 2 +- bin/tools/mdig.c | 2 +- lib/dns/client.c | 18 +- lib/dns/dispatch.c | 1335 +++++++++------------- lib/dns/include/dns/dispatch.h | 42 +- lib/dns/include/dns/request.h | 12 +- lib/dns/request.c | 351 +++--- lib/dns/resolver.c | 984 +++++++++------- lib/dns/tests/dispatch_test.c | 4 +- lib/dns/tests/resolver_test.c | 2 +- lib/dns/zone.c | 4 +- lib/ns/tests/nstest.c | 2 +- 16 files changed, 1360 insertions(+), 1406 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 0abefe8407..7957591281 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -10003,7 +10003,7 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { ns_interfacemgr_detach(&server->interfacemgr); - dns_dispatchmgr_destroy(&named_g_dispatchmgr); + dns_dispatchmgr_detach(&named_g_dispatchmgr); dns_zonemgr_shutdown(server->zonemgr); diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index a3bb130a7e..0690676db4 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -735,7 +735,7 @@ doshutdown(void) { } ddebug("Shutting down dispatch manager"); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } static void diff --git a/bin/tests/system/pipelined/pipequeries.c b/bin/tests/system/pipelined/pipequeries.c index 6b34dd808c..e41f14a566 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -286,7 +286,7 @@ main(int argc, char *argv[]) { dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchv4); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); diff --git a/bin/tests/system/tkey/keycreate.c b/bin/tests/system/tkey/keycreate.c index 9ede9a65b5..b83d613f82 100644 --- a/bin/tests/system/tkey/keycreate.c +++ b/bin/tests/system/tkey/keycreate.c @@ -255,7 +255,7 @@ main(int argc, char *argv[]) { dns_requestmgr_shutdown(requestmgr); dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchv4); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); isc_managers_destroy(&netmgr, &taskmgr, NULL, NULL); diff --git a/bin/tests/system/tkey/keydelete.c b/bin/tests/system/tkey/keydelete.c index c76d816132..9b32e2930e 100644 --- a/bin/tests/system/tkey/keydelete.c +++ b/bin/tests/system/tkey/keydelete.c @@ -198,7 +198,7 @@ main(int argc, char **argv) { dns_requestmgr_shutdown(requestmgr); dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchv4); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); isc_managers_destroy(&netmgr, &taskmgr, NULL, NULL); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index f3d37da52c..ca3acb8eac 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -2175,7 +2175,7 @@ main(int argc, char *argv[]) { dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchvx); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); diff --git a/lib/dns/client.c b/lib/dns/client.c index d33be3708f..030fa035b6 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -268,7 +268,6 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr6) { isc_result_t result; dns_client_t *client = NULL; - dns_dispatchmgr_t *dispatchmgr = NULL; dns_dispatch_t *dispatchv4 = NULL; dns_dispatch_t *dispatchv6 = NULL; dns_view_t *view = NULL; @@ -293,12 +292,11 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, goto cleanup_lock; } - result = dns_dispatchmgr_create(mctx, nm, &dispatchmgr); + result = dns_dispatchmgr_create(mctx, nm, &client->dispatchmgr); if (result != ISC_R_SUCCESS) { goto cleanup_task; } - client->dispatchmgr = dispatchmgr; - (void)setsourceports(mctx, dispatchmgr); + (void)setsourceports(mctx, client->dispatchmgr); /* * If only one address family is specified, use it. @@ -306,7 +304,7 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, */ client->dispatchv4 = NULL; if (localaddr4 != NULL || localaddr6 == NULL) { - result = getudpdispatch(AF_INET, dispatchmgr, taskmgr, + result = getudpdispatch(AF_INET, client->dispatchmgr, taskmgr, &dispatchv4, localaddr4); if (result == ISC_R_SUCCESS) { client->dispatchv4 = dispatchv4; @@ -315,7 +313,7 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, client->dispatchv6 = NULL; if (localaddr6 != NULL || localaddr4 == NULL) { - result = getudpdispatch(AF_INET6, dispatchmgr, taskmgr, + result = getudpdispatch(AF_INET6, client->dispatchmgr, taskmgr, &dispatchv6, localaddr6); if (result == ISC_R_SUCCESS) { client->dispatchv6 = dispatchv6; @@ -332,8 +330,8 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, /* Create the default view for class IN */ result = createview(mctx, dns_rdataclass_in, taskmgr, RESOLVER_NTASKS, - nm, timermgr, dispatchmgr, dispatchv4, dispatchv6, - &view); + nm, timermgr, client->dispatchmgr, dispatchv4, + dispatchv6, &view); if (result != ISC_R_SUCCESS) { goto cleanup_references; } @@ -366,7 +364,7 @@ cleanup_dispatchmgr: if (dispatchv6 != NULL) { dns_dispatch_detach(&dispatchv6); } - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&client->dispatchmgr); cleanup_task: isc_task_detach(&client->task); cleanup_lock: @@ -394,7 +392,7 @@ destroyclient(dns_client_t *client) { dns_dispatch_detach(&client->dispatchv6); } - dns_dispatchmgr_destroy(&client->dispatchmgr); + dns_dispatchmgr_detach(&client->dispatchmgr); isc_task_detach(&client->task); diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index 3f36a52223..75803f210d 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -41,8 +41,6 @@ typedef ISC_LIST(dns_dispentry_t) dns_displist_t; -typedef struct dispsocket dispsocket_t; - typedef struct dns_qid { unsigned int magic; isc_mutex_t lock; @@ -54,6 +52,7 @@ typedef struct dns_qid { struct dns_dispatchmgr { /* Unlocked. */ unsigned int magic; + isc_refcount_t references; isc_mem_t *mctx; dns_acl_t *blackhole; isc_stats_t *stats; @@ -69,8 +68,6 @@ struct dns_dispatchmgr { isc_mutex_t buffer_lock; unsigned int buffers; - isc_refcount_t irefs; - in_port_t *v4ports; /*%< available ports for IPv4 */ unsigned int nv4ports; /*%< # of available ports for IPv4 */ in_port_t *v6ports; /*%< available ports for IPv4 */ @@ -82,22 +79,25 @@ struct dns_dispatchmgr { struct dns_dispentry { unsigned int magic; + isc_refcount_t references; dns_dispatch_t *disp; - dns_messageid_t id; - in_port_t port; + isc_task_t *task; + isc_nmhandle_t *handle; /*%< netmgr handle for UDP connection */ unsigned int bucket; unsigned int timeout; + isc_sockaddr_t local; isc_sockaddr_t peer; - isc_task_t *task; + in_port_t port; + dns_messageid_t id; isc_nm_cb_t connected; isc_nm_cb_t sent; isc_nm_cb_t timedout; isc_taskaction_t action; void *arg; - bool item_out; - dispsocket_t *dispsocket; + bool canceled; ISC_LIST(dns_dispatchevent_t) items; ISC_LINK(dns_dispentry_t) link; + ISC_LINK(dns_dispentry_t) alink; }; /*% @@ -107,17 +107,6 @@ struct dns_dispentry { #define DNS_DISPATCH_UDPBUFSIZE 4096 #endif /* ifndef DNS_DISPATCH_UDPBUFSIZE */ -struct dispsocket { - unsigned int magic; - isc_nmhandle_t *handle; - dns_dispatch_t *disp; - isc_sockaddr_t local; - isc_sockaddr_t peer; - dns_dispentry_t *resp; - in_port_t port; - ISC_LINK(dispsocket_t) link; -}; - struct dns_dispatch { /* Unlocked. */ unsigned int magic; /*%< magic */ @@ -127,9 +116,6 @@ struct dns_dispatch { isc_sockaddr_t local; /*%< local address */ in_port_t localport; /*%< local UDP port */ isc_sockaddr_t peer; /*%< peer address (TCP) */ - isc_event_t *ctlevent; - - isc_mem_t *sepool; /*%< pool for socket events */ /*% Locked by mgr->lock. */ ISC_LINK(dns_dispatch_t) link; @@ -138,13 +124,13 @@ struct dns_dispatch { isc_mutex_t lock; /*%< locks all below */ isc_socktype_t socktype; unsigned int attributes; - isc_refcount_t refcount; - dns_dispatchevent_t *failsafe_ev; /*%< failsafe cancel event */ - unsigned int shutting_down : 1, shutdown_out : 1, recv_pending : 1; - isc_result_t shutdown_why; - ISC_LIST(dispsocket_t) activesockets; - ISC_LIST(dispsocket_t) inactivesockets; + isc_refcount_t references; + dns_dispatchevent_t failsafe_ev; /*%< failsafe cancel event */ + unsigned int shutdown_out : 1, recv_pending : 1; + + ISC_LIST(dns_dispentry_t) active; unsigned int nsockets; + unsigned int requests; /*%< how many requests we have */ unsigned int tcpbuffers; /*%< allocated buffers */ }; @@ -164,17 +150,6 @@ struct dns_dispatch { #define DNS_DISPATCHMGR_MAGIC ISC_MAGIC('D', 'M', 'g', 'r') #define VALID_DISPATCHMGR(e) ISC_MAGIC_VALID((e), DNS_DISPATCHMGR_MAGIC) -/*% - * Maximum number of dispatch sockets that can be pooled for reuse. The - * appropriate value may vary, but experiments have shown a busy caching server - * may need more than 1000 sockets concurrently opened. The maximum allowable - * number of dispatch sockets (per manager) will be set to the double of this - * value. - */ -#ifndef DNS_DISPATCH_POOLSOCKS -#define DNS_DISPATCH_POOLSOCKS 2048 -#endif /* ifndef DNS_DISPATCH_POOLSOCKS */ - /*% * Quota to control the number of UDP dispatch sockets. If a dispatch has * more than the quota of sockets, new queries will purge oldest ones, so @@ -194,14 +169,6 @@ struct dns_dispatch { #define DNS_DISPATCH_MAXBUFFERS 37268 #endif /* ifndef DNS_DISPATCH_MAXBUFFERS */ -/*% - * Number of dispatch sockets available for all dispatches in the - * socket memory pool. - */ -#ifndef DNS_DISPATCH_MAXSOCKETS -#define DNS_DISPATCH_MAXSOCKETS 32768 -#endif /* ifndef DNS_DISPATCH_MAXSOCKETS */ - /*% * Quota to control the number of concurrent requests that can be handled * by each TCP dispatch. (UDP dispatches do not currently support socket @@ -227,17 +194,12 @@ struct dns_dispatch { /* * Statics. */ +static void +dispatchmgr_destroy(dns_dispatchmgr_t *mgr); + static dns_dispentry_t * entry_search(dns_qid_t *, const isc_sockaddr_t *, dns_messageid_t, in_port_t, unsigned int); -static bool -destroy_disp_ok(dns_dispatch_t *); -static void -destroy_disp(isc_task_t *task, isc_event_t *event); -static void -destroy_dispsocket(dns_dispatch_t *, dispsocket_t **); -static void -deactivate_dispsocket(dns_dispatch_t *, dispsocket_t *); static void udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg); @@ -251,31 +213,19 @@ free_devent(dns_dispatch_t *disp, dns_dispatchevent_t *ev); static inline dns_dispatchevent_t * allocate_devent(dns_dispatch_t *disp); static void -do_cancel(dns_dispatch_t *disp); -static dns_dispentry_t * -linear_first(dns_qid_t *disp); -static dns_dispentry_t * -linear_next(dns_qid_t *disp, dns_dispentry_t *resp); -static void dispatch_free(dns_dispatch_t **dispp); static isc_result_t dispatch_createudp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, unsigned int attributes, dns_dispatch_t **dispp); -static bool -destroy_mgr_ok(dns_dispatchmgr_t *mgr); -static void -destroy_mgr(dns_dispatchmgr_t **mgrp); static void qid_allocate(dns_dispatchmgr_t *mgr, dns_qid_t **qidp); static void qid_destroy(isc_mem_t *mctx, dns_qid_t **qidp); -static isc_nmhandle_t * -gethandle(dns_dispatch_t *disp); -static isc_nmhandle_t * +static inline isc_nmhandle_t * getentryhandle(dns_dispentry_t *resp); static void -startrecv(dns_dispatch_t *disp, dispsocket_t *dispsocket); +startrecv(dns_dispatch_t *disp, dns_dispentry_t *resp); #define LVL(x) ISC_LOG_DEBUG(x) @@ -345,7 +295,6 @@ static void request_log(dns_dispatch_t *disp, dns_dispentry_t *resp, int level, const char *fmt, ...) { char msgbuf[2048]; - char peerbuf[256]; va_list ap; if (!isc_log_wouldlog(dns_lctx, level)) { @@ -357,6 +306,7 @@ request_log(dns_dispatch_t *disp, dns_dispentry_t *resp, int level, va_end(ap); if (VALID_RESPONSE(resp)) { + char peerbuf[256]; isc_sockaddr_format(&resp->peer, peerbuf, sizeof(peerbuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_DISPATCH, DNS_LOGMODULE_DISPATCH, level, @@ -387,136 +337,17 @@ dns_hash(dns_qid_t *qid, const isc_sockaddr_t *dest, dns_messageid_t id, return (ret); } -/* - * Find the first entry in 'qid'. Returns NULL if there are no entries. - */ -static dns_dispentry_t * -linear_first(dns_qid_t *qid) { - dns_dispentry_t *ret = NULL; - unsigned int bucket = 0; - - while (bucket < qid->qid_nbuckets) { - ret = ISC_LIST_HEAD(qid->qid_table[bucket]); - if (ret != NULL) { - return (ret); - } - bucket++; - } - - return (NULL); -} - -/* - * Find the next entry after 'resp' in 'qid'. Return NULL if there are - * no more entries. - */ -static dns_dispentry_t * -linear_next(dns_qid_t *qid, dns_dispentry_t *resp) { - dns_dispentry_t *ret = NULL; - unsigned int bucket; - - ret = ISC_LIST_NEXT(resp, link); - if (ret != NULL) { - return (ret); - } - - bucket = resp->bucket + 1; - while (bucket < qid->qid_nbuckets) { - ret = ISC_LIST_HEAD(qid->qid_table[bucket]); - if (ret != NULL) { - return (ret); - } - bucket++; - } - - return (NULL); -} - -/* - * The dispatch must be locked. - */ -static bool -destroy_disp_ok(dns_dispatch_t *disp) { - if (isc_refcount_current(&disp->refcount) != 0) { - return (false); - } - - if (disp->recv_pending != 0) { - return (false); - } - - if (!ISC_LIST_EMPTY(disp->activesockets)) { - return (false); - } - - if (disp->shutting_down == 0) { - return (false); - } - - return (true); -} - -/* - * Called when refcount reaches 0 (and safe to destroy). - * - * The dispatcher must be locked. - * The manager must not be locked. - */ -static void -destroy_disp(isc_task_t *task, isc_event_t *event) { - dns_dispatch_t *disp = NULL; - dns_dispatchmgr_t *mgr = NULL; - bool killmgr; - dispsocket_t *dispsocket = NULL; - - INSIST(event->ev_type == DNS_EVENT_DISPATCHCONTROL); - - UNUSED(task); - - disp = event->ev_arg; - mgr = disp->mgr; - - LOCK(&mgr->lock); - ISC_LIST_UNLINK(mgr->list, disp, link); - - dispatch_log(disp, LVL(90), "shutting down; detaching from handle %p", - disp->handle); - - if (disp->sepool != NULL) { - isc_mem_destroy(&disp->sepool); - } - - if (disp->handle != NULL) { - isc_nmhandle_detach(&disp->handle); - } - while ((dispsocket = ISC_LIST_HEAD(disp->inactivesockets)) != NULL) { - ISC_LIST_UNLINK(disp->inactivesockets, dispsocket, link); - destroy_dispsocket(disp, &dispsocket); - } - isc_task_detach(&disp->task); - isc_event_free(&event); - - dispatch_free(&disp); - - killmgr = destroy_mgr_ok(mgr); - UNLOCK(&mgr->lock); - if (killmgr) { - destroy_mgr(&mgr); - } -} - /*% - * Make a new socket for a single dispatch with a random port number. + * Choose a random port number for a dispatch entry. * The caller must hold the disp->lock */ static isc_result_t -get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest, - dispsocket_t **dispsockp, in_port_t *portp) { +setup_socket(dns_dispatch_t *disp, dns_dispentry_t *resp, + const isc_sockaddr_t *dest, in_port_t *portp) { dns_dispatchmgr_t *mgr = disp->mgr; - in_port_t port; - dispsocket_t *dispsock = NULL; unsigned int nports; in_port_t *ports = NULL; + in_port_t port; if (isc_sockaddr_pf(&disp->local) == AF_INET) { nports = mgr->nv4ports; @@ -529,81 +360,34 @@ get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest, return (ISC_R_ADDRNOTAVAIL); } - dispsock = ISC_LIST_HEAD(disp->inactivesockets); - if (dispsock != NULL) { - ISC_LIST_UNLINK(disp->inactivesockets, dispsock, link); - } else { - dispsock = isc_mem_get(mgr->mctx, sizeof(*dispsock)); - disp->nsockets++; + disp->nsockets++; - *dispsock = (dispsocket_t){ .disp = disp }; - ISC_LINK_INIT(dispsock, link); - dispsock->magic = DISPSOCK_MAGIC; - } + resp->local = disp->local; + resp->peer = *dest; - dispsock->local = disp->local; - dispsock->peer = *dest; - - /* Pick a random UDP port */ port = ports[isc_random_uniform(nports)]; - isc_sockaddr_setport(&dispsock->local, port); - dispsock->port = port; + isc_sockaddr_setport(&resp->local, port); + resp->port = port; - *dispsockp = dispsock; *portp = port; return (ISC_R_SUCCESS); } /*% - * Destroy a dedicated dispatch socket. + * Deactivate the socket for a dispatch entry. * The dispatch must be locked. */ static void -destroy_dispsocket(dns_dispatch_t *disp, dispsocket_t **dispsockp) { - dispsocket_t *dispsock = NULL; - - REQUIRE(dispsockp != NULL && *dispsockp != NULL); - - dispsock = *dispsockp; - *dispsockp = NULL; - - REQUIRE(!ISC_LINK_LINKED(dispsock, link)); - +deactivate_dispentry(dns_dispatch_t *disp, dns_dispentry_t *resp) { + if (ISC_LINK_LINKED(resp, alink)) { + ISC_LIST_UNLINK(disp->active, resp, alink); + } + if (resp->handle != NULL) { + isc_nm_cancelread(resp->handle); + isc_nmhandle_detach(&resp->handle); + } disp->nsockets--; - dispsock->magic = 0; - if (dispsock->handle != NULL) { - isc_nm_cancelread(dispsock->handle); - isc_nmhandle_detach(&dispsock->handle); - } - - isc_mem_put(disp->mgr->mctx, dispsock, sizeof(*dispsock)); -} - -/*% - * Deactivate a dedicated dispatch socket. Move it to the inactive list for - * future reuse unless the total number of sockets are exceeding the maximum. - * The dispatch must be locked. - */ -static void -deactivate_dispsocket(dns_dispatch_t *disp, dispsocket_t *dispsock) { - ISC_LIST_UNLINK(disp->activesockets, dispsock, link); - if (dispsock->resp != NULL) { - INSIST(dispsock->resp->dispsocket == dispsock); - dispsock->resp->dispsocket = NULL; - dispsock->resp = NULL; - } - - if (disp->nsockets > DNS_DISPATCH_POOLSOCKS) { - destroy_dispsocket(disp, &dispsock); - } else { - if (dispsock->handle != NULL) { - isc_nm_cancelread(dispsock->handle); - isc_nmhandle_detach(&dispsock->handle); - } - - ISC_LIST_APPEND(disp->inactivesockets, dispsock, link); - } } /* @@ -671,7 +455,8 @@ allocate_udp_buffer(dns_dispatch_t *disp) { static inline void free_devent(dns_dispatch_t *disp, dns_dispatchevent_t *ev) { - if (disp->failsafe_ev == ev) { + /* failsafe_ev is not allocated */ + if (&(disp->failsafe_ev) == ev) { INSIST(disp->shutdown_out == 1); disp->shutdown_out = 0; @@ -684,8 +469,9 @@ free_devent(dns_dispatch_t *disp, dns_dispatchevent_t *ev) { ev->region.length = 0; } - isc_refcount_decrement(&disp->mgr->irefs); isc_mem_put(disp->mgr->mctx, ev, sizeof(*ev)); + + dns_dispatch_detach(&disp); } static inline dns_dispatchevent_t * @@ -693,14 +479,95 @@ allocate_devent(dns_dispatch_t *disp) { dns_dispatchevent_t *ev = NULL; ev = isc_mem_get(disp->mgr->mctx, sizeof(*ev)); - isc_refcount_increment0(&disp->mgr->irefs); *ev = (dns_dispatchevent_t){ 0 }; + + dns_dispatch_attach(disp, &ev->dispatch); + ISC_EVENT_INIT(ev, sizeof(*ev), 0, NULL, 0, NULL, NULL, NULL, NULL, NULL); return (ev); } +#define dispentry_attach(r, rp) \ + __dispentry_attach(r, rp, __func__, __FILE__, __LINE__) + +static void +__dispentry_attach(dns_dispentry_t *resp, dns_dispentry_t **respp, + const char *func, const char *file, unsigned int line) { + uint_fast32_t ref; + + REQUIRE(VALID_RESPONSE(resp)); + REQUIRE(respp != NULL && *respp == NULL); + + ref = isc_refcount_increment(&resp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, resp, respp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* DISPATCH_TRACE */ + + *respp = resp; +} + +static void +__dispentry_destroy(dns_dispentry_t *resp) { + dns_dispatch_t *disp = resp->disp; + + resp->magic = 0; + + if (resp->handle != NULL) { + isc_nmhandle_detach(&resp->handle); + } + + isc_refcount_destroy(&resp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_detach(%p) -> %p\n", __FILE__, + __LINE__, __func__, &resp->task, resp->task); +#endif /* DISPATCH_TRACE */ + + isc_task_detach(&resp->task); + isc_mem_put(disp->mgr->mctx, resp, sizeof(*resp)); + + dns_dispatch_detach(&disp); +} + +#define dispentry_detach(rp) \ + __dispentry_detach(rp, __func__, __FILE__, __LINE__) + +static void +__dispentry_detach(dns_dispentry_t **respp, const char *func, const char *file, + unsigned int line) { + dns_dispentry_t *resp = NULL; + uint_fast32_t ref; + + REQUIRE(respp != NULL && VALID_RESPONSE(*respp)); + + resp = *respp; + *respp = NULL; + + ref = isc_refcount_decrement(&resp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, resp, respp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* DISPATCH_TRACE */ + + if (ref == 1) { + __dispentry_destroy(resp); + } +} + /* * General flow: * @@ -718,60 +585,43 @@ allocate_devent(dns_dispatch_t *disp) { static void udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg) { - dispsocket_t *dispsock = (dispsocket_t *)arg; + dns_dispentry_t *resp = (dns_dispentry_t *)arg; dns_dispatch_t *disp = NULL; dns_messageid_t id; isc_result_t dres; isc_buffer_t source; unsigned int flags; - dns_dispentry_t *resp = NULL; dns_dispatchevent_t *rev = NULL; - bool killit; - bool queue_response; - dns_dispatchmgr_t *mgr = NULL; isc_sockaddr_t peer; isc_netaddr_t netaddr; int match; + isc_taskaction_t action = NULL; + bool nomore = true; - REQUIRE(VALID_DISPSOCK(dispsock)); + REQUIRE(VALID_RESPONSE(resp)); - disp = dispsock->disp; + disp = resp->disp; LOCK(&disp->lock); - mgr = disp->mgr; - - LOCK(&disp->mgr->buffer_lock); - dispatch_log(disp, LVL(90), "got packet: requests %d, recvs %d", - disp->requests, disp->recv_pending); - UNLOCK(&disp->mgr->buffer_lock); - - if (eresult == ISC_R_CANCELED || dispsock->resp == NULL) { - /* - * dispsock->resp can be NULL if this transaction was canceled - * just after receiving a response. So we can just move on. - */ - dispsock = NULL; + if (isc_log_wouldlog(dns_lctx, LVL(90))) { + LOCK(&disp->mgr->buffer_lock); + dispatch_log(disp, LVL(90), "got packet: requests %d, recvs %d", + disp->requests, disp->recv_pending); + UNLOCK(&disp->mgr->buffer_lock); } - if (disp->shutting_down == 1) { + if (eresult == ISC_R_CANCELED) { /* * This dispatcher is shutting down. */ - killit = destroy_disp_ok(disp); - UNLOCK(&disp->lock); - if (killit) { - isc_task_send(disp->task, &disp->ctlevent); - } - - return; + goto unlock; } - if (dispsock == NULL) { - goto next; + if (!ISC_LINK_LINKED(resp, alink)) { + goto unlock; } - resp = dispsock->resp; id = resp->id; peer = isc_nmhandle_peeraddr(handle); @@ -780,7 +630,8 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, if (eresult == ISC_R_TIMEDOUT && resp->timedout != NULL) { resp->timedout(handle, ISC_R_TIMEDOUT, resp->arg); if (isc_nmhandle_timer_running(handle)) { - goto next; + nomore = false; + goto unlock; } } @@ -810,7 +661,7 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, dispatch_log(disp, LVL(10), "blackholed packet from %s", netaddrstr); } - goto next; + goto unlock; } /* @@ -821,7 +672,7 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, dres = dns_message_peekheader(&source, &id, &flags); if (dres != ISC_R_SUCCESS) { dispatch_log(disp, LVL(10), "got garbage packet"); - goto next; + goto unlock; } dispatch_log(disp, LVL(92), @@ -834,7 +685,7 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, */ if ((flags & DNS_MESSAGEFLAG_QR) == 0) { /* query */ - goto next; + goto unlock; } /* @@ -842,13 +693,12 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, */ if (resp->id != id || !isc_sockaddr_equal(&peer, &resp->peer)) { dispatch_log(disp, LVL(90), "response doesn't match"); - inc_stats(mgr, dns_resstatscounter_mismatch); - goto next; + inc_stats(disp->mgr, dns_resstatscounter_mismatch); + goto unlock; } sendevent: rev = allocate_devent(disp); - queue_response = resp->item_out; /* * At this point, rev contains the event we want to fill in, and @@ -856,7 +706,6 @@ sendevent: * Send the event off. */ rev->result = eresult; - resp->item_out = true; if (region != NULL) { rev->region.base = allocate_udp_buffer(disp); rev->region.length = DNS_DISPATCH_UDPBUFSIZE; @@ -866,23 +715,26 @@ sendevent: isc_buffer_add(&rev->buffer, rev->region.length); } - if (queue_response) { - ISC_LIST_APPEND(resp->items, rev, ev_link); - } else { - ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, - resp->action, resp->arg, resp, NULL, NULL); - request_log(disp, resp, LVL(90), - "[a] Sent event %p buffer %p len %d to task %p", - rev, rev->buffer.base, rev->buffer.length, - resp->task); - isc_task_send(resp->task, ISC_EVENT_PTR(&rev)); + ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, + resp->action, resp->arg, resp, NULL, NULL); + + request_log(disp, resp, LVL(90), + "[a] Sent event %p buffer %p len %d to task %p", rev, + rev->buffer.base, rev->buffer.length, resp->task); + + action = resp->action; + +unlock: + UNLOCK(&disp->lock); + + if (action != NULL) { + action(resp->task, (isc_event_t *)rev); } - UNLOCK(&disp->lock); + if (nomore) { + dispentry_detach(&resp); + } return; - -next: - UNLOCK(&disp->lock); } /* @@ -907,17 +759,15 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, dns_messageid_t id; isc_result_t dres; unsigned int flags; - dispsocket_t *dispsock = NULL; dns_dispentry_t *resp = NULL; dns_dispatchevent_t *rev = NULL; unsigned int bucket; - bool killit; - bool queue_response; dns_qid_t *qid = NULL; int level; char buf[ISC_SOCKADDR_FORMATSIZE]; isc_buffer_t source; isc_sockaddr_t peer; + isc_taskaction_t action = NULL; REQUIRE(VALID_DISPATCH(disp)); @@ -932,79 +782,48 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, INSIST(disp->recv_pending != 0); disp->recv_pending = 0; - if (isc_refcount_current(&disp->refcount) == 0) { - /* - * This dispatcher is shutting down. Force cancellation. - */ - eresult = ISC_R_CANCELED; - } - peer = isc_nmhandle_peeraddr(handle); - isc_nmhandle_detach(&handle); if (eresult != ISC_R_SUCCESS) { - disp->shutdown_why = eresult; - switch (eresult) { case ISC_R_CANCELED: dispatch_log(disp, LVL(90), "shutting down on cancel"); - do_cancel(disp); break; case ISC_R_EOF: dispatch_log(disp, LVL(90), "shutting down on EOF"); - do_cancel(disp); break; - case ISC_R_CONNECTIONRESET: - level = ISC_LOG_INFO; - goto logit; - case ISC_R_TIMEDOUT: /* * Time out the first active response for which * no event has already been sent. */ - for (dispsock = ISC_LIST_HEAD(disp->activesockets); - dispsock != NULL; - dispsock = ISC_LIST_NEXT(dispsock, link)) + for (resp = ISC_LIST_HEAD(disp->active); resp != NULL; + resp = ISC_LIST_NEXT(resp, alink)) { - resp = dispsock->resp; - if (resp->item_out) { - continue; - } - ISC_LIST_UNLINK(disp->activesockets, dispsock, - link); - ISC_LIST_APPEND(disp->activesockets, dispsock, - link); + ISC_LIST_UNLINK(disp->active, resp, alink); + ISC_LIST_APPEND(disp->active, resp, alink); goto sendevent; } - INSIST(0); - ISC_UNREACHABLE(); + break; default: - level = ISC_LOG_ERROR; - logit: + if (eresult == ISC_R_CONNECTIONRESET) { + level = ISC_LOG_INFO; + } else { + level = ISC_LOG_ERROR; + } + isc_sockaddr_format(&peer, buf, sizeof(buf)); dispatch_log(disp, level, "shutting down due to TCP " "receive error: %s: %s", buf, isc_result_totext(eresult)); - do_cancel(disp); break; } - disp->shutting_down = 1; - - /* - * If the recv() was canceled pass the word on. - */ - killit = destroy_disp_ok(disp); - UNLOCK(&disp->lock); - if (killit) { - isc_task_send(disp->task, &disp->ctlevent); - } - return; + goto unlock; } dispatch_log(disp, LVL(90), "result %d, length == %d, addr = %p", @@ -1055,7 +874,6 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, } sendevent: - queue_response = resp->item_out; rev = allocate_devent(disp); /* @@ -1064,7 +882,6 @@ sendevent: * Send the event off. */ rev->result = eresult; - resp->item_out = true; if (region != NULL) { disp->tcpbuffers++; rev->region.base = isc_mem_get(disp->mgr->mctx, region->length); @@ -1075,84 +892,26 @@ sendevent: isc_buffer_add(&rev->buffer, rev->region.length); } - if (queue_response) { - ISC_LIST_APPEND(resp->items, rev, ev_link); - } else { - ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, - resp->action, resp->arg, resp, NULL, NULL); - request_log(disp, resp, LVL(90), - "[b] Sent event %p buffer %p len %d to task %p", - rev, rev->buffer.base, rev->buffer.length, - resp->task); - isc_task_send(resp->task, ISC_EVENT_PTR(&rev)); - } + ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, + resp->action, resp->arg, resp, NULL, NULL); + request_log(disp, resp, LVL(90), + "[b] Sent event %p buffer %p len %d to task %p", rev, + rev->buffer.base, rev->buffer.length, resp->task); + + action = resp->action; next: startrecv(disp, NULL); + +unlock: + isc_nmhandle_detach(&handle); UNLOCK(&disp->lock); -} -/* - * Mgr must be locked when calling this function. - */ -static bool -destroy_mgr_ok(dns_dispatchmgr_t *mgr) { - mgr_log(mgr, LVL(90), - "destroy_mgr_ok: shuttingdown=%d, listnonempty=%d, ", - MGR_IS_SHUTTINGDOWN(mgr), !ISC_LIST_EMPTY(mgr->list)); - if (!MGR_IS_SHUTTINGDOWN(mgr)) { - return (false); + dns_dispatch_detach(&disp); + + if (action != NULL) { + action(resp->task, (isc_event_t *)rev); } - if (!ISC_LIST_EMPTY(mgr->list)) { - return (false); - } - if (isc_refcount_current(&mgr->irefs) != 0) { - return (false); - } - - return (true); -} - -/* - * Mgr must be unlocked when calling this function. - */ -static void -destroy_mgr(dns_dispatchmgr_t **mgrp) { - dns_dispatchmgr_t *mgr = NULL; - - mgr = *mgrp; - *mgrp = NULL; - - mgr->magic = 0; - isc_mutex_destroy(&mgr->lock); - mgr->state = 0; - - if (mgr->qid != NULL) { - qid_destroy(mgr->mctx, &mgr->qid); - } - - isc_mutex_destroy(&mgr->buffer_lock); - - if (mgr->blackhole != NULL) { - dns_acl_detach(&mgr->blackhole); - } - - if (mgr->stats != NULL) { - isc_stats_detach(&mgr->stats); - } - - if (mgr->v4ports != NULL) { - isc_mem_put(mgr->mctx, mgr->v4ports, - mgr->nv4ports * sizeof(in_port_t)); - } - if (mgr->v6ports != NULL) { - isc_mem_put(mgr->mctx, mgr->v6ports, - mgr->nv6ports * sizeof(in_port_t)); - } - - isc_nm_detach(&mgr->nm); - - isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(dns_dispatchmgr_t)); } /*% @@ -1231,7 +990,7 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm, mgr = isc_mem_get(mctx, sizeof(dns_dispatchmgr_t)); *mgr = (dns_dispatchmgr_t){ .magic = 0 }; - isc_refcount_init(&mgr->irefs, 0); + isc_refcount_init(&mgr->references, 1); isc_mem_attach(mctx, &mgr->mctx); isc_nm_attach(nm, &mgr->nm); @@ -1256,6 +1015,56 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm, return (ISC_R_SUCCESS); } +void +dns__dispatchmgr_attach(dns_dispatchmgr_t *mgr, dns_dispatchmgr_t **mgrp, + const char *func, const char *file, unsigned int line) { + uint_fast32_t ref; + + REQUIRE(VALID_DISPATCHMGR(mgr)); + REQUIRE(mgrp != NULL && *mgrp == NULL); + + ref = isc_refcount_increment(&mgr->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, mgr, mgrp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* DISPATCH_TRACE */ + + *mgrp = mgr; +} + +void +dns__dispatchmgr_detach(dns_dispatchmgr_t **mgrp, const char *func, + const char *file, unsigned int line) { + dns_dispatchmgr_t *mgr = NULL; + uint_fast32_t ref; + + REQUIRE(mgrp != NULL && VALID_DISPATCHMGR(*mgrp)); + + mgr = *mgrp; + *mgrp = NULL; + + ref = isc_refcount_decrement(&mgr->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, mgr, mgrp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* DISPATCH_TRACE */ + + if (ref == 1) { + dispatchmgr_destroy(mgr); + } +} + void dns_dispatchmgr_setblackhole(dns_dispatchmgr_t *mgr, dns_acl_t *blackhole) { REQUIRE(VALID_DISPATCHMGR(mgr)); @@ -1278,27 +1087,40 @@ dns_dispatchmgr_setavailports(dns_dispatchmgr_t *mgr, isc_portset_t *v4portset, return (setavailports(mgr, v4portset, v6portset)); } -void -dns_dispatchmgr_destroy(dns_dispatchmgr_t **mgrp) { - dns_dispatchmgr_t *mgr = NULL; - bool killit; +static void +dispatchmgr_destroy(dns_dispatchmgr_t *mgr) { + REQUIRE(VALID_DISPATCHMGR(mgr)); - REQUIRE(mgrp != NULL); - REQUIRE(VALID_DISPATCHMGR(*mgrp)); + isc_refcount_destroy(&mgr->references); - mgr = *mgrp; - *mgrp = NULL; + mgr->magic = 0; + isc_mutex_destroy(&mgr->lock); + mgr->state = 0; - LOCK(&mgr->lock); - mgr->state |= MGR_SHUTTINGDOWN; - killit = destroy_mgr_ok(mgr); - UNLOCK(&mgr->lock); + qid_destroy(mgr->mctx, &mgr->qid); - mgr_log(mgr, LVL(90), "destroy: killit=%d", killit); + isc_mutex_destroy(&mgr->buffer_lock); - if (killit) { - destroy_mgr(&mgr); + if (mgr->blackhole != NULL) { + dns_acl_detach(&mgr->blackhole); } + + if (mgr->stats != NULL) { + isc_stats_detach(&mgr->stats); + } + + if (mgr->v4ports != NULL) { + isc_mem_put(mgr->mctx, mgr->v4ports, + mgr->nv4ports * sizeof(in_port_t)); + } + if (mgr->v6ports != NULL) { + isc_mem_put(mgr->mctx, mgr->v6ports, + mgr->nv6ports * sizeof(in_port_t)); + } + + isc_nm_detach(&mgr->nm); + + isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(dns_dispatchmgr_t)); } void @@ -1366,15 +1188,12 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, isc_socktype_t type, int pf, */ disp = isc_mem_get(mgr->mctx, sizeof(*disp)); - isc_refcount_increment0(&mgr->irefs); + *disp = (dns_dispatch_t){ .socktype = type }; - *disp = (dns_dispatch_t){ .mgr = mgr, - .socktype = type, - .shutdown_why = ISC_R_UNEXPECTED }; - isc_refcount_init(&disp->refcount, 1); + dns_dispatchmgr_attach(mgr, &disp->mgr); + isc_refcount_init(&disp->references, 1); ISC_LINK_INIT(disp, link); - ISC_LIST_INIT(disp->activesockets); - ISC_LIST_INIT(disp->inactivesockets); + ISC_LIST_INIT(disp->active); switch (type) { case isc_socktype_tcp: @@ -1409,7 +1228,11 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, isc_socktype_t type, int pf, disp->attributes |= attributes; isc_mutex_init(&disp->lock); - disp->failsafe_ev = allocate_devent(disp); + + /* failsafe_ev is not allocated */ + ISC_EVENT_INIT(&disp->failsafe_ev, sizeof(disp->failsafe_ev), 0, NULL, + 0, NULL, NULL, NULL, NULL, NULL); + disp->magic = DISPATCH_MAGIC; *dispp = disp; @@ -1427,26 +1250,17 @@ dispatch_free(dns_dispatch_t **dispp) { disp = *dispp; *dispp = NULL; + disp->magic = 0; + mgr = disp->mgr; REQUIRE(VALID_DISPATCHMGR(mgr)); INSIST(disp->requests == 0); INSIST(disp->recv_pending == 0); - INSIST(ISC_LIST_EMPTY(disp->activesockets)); - INSIST(ISC_LIST_EMPTY(disp->inactivesockets)); + INSIST(ISC_LIST_EMPTY(disp->active)); - isc_refcount_decrement(&mgr->irefs); - - if (disp->failsafe_ev != NULL) { - isc_mem_put(mgr->mctx, disp->failsafe_ev, - sizeof(*disp->failsafe_ev)); - disp->failsafe_ev = NULL; - } - - disp->mgr = NULL; isc_mutex_destroy(&disp->lock); - disp->magic = 0; - isc_refcount_decrement(&mgr->irefs); + isc_mem_put(mgr->mctx, disp, sizeof(*disp)); } @@ -1479,14 +1293,16 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, } result = isc_task_create(taskmgr, 50, &disp->task); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_create() -> %p\n", __FILE__, + __LINE__, __func__, disp->task); +#endif /* DISPATCH_TRACE */ + if (result != ISC_R_SUCCESS) { goto cleanup; } - disp->ctlevent = - isc_event_allocate(mgr->mctx, disp, DNS_EVENT_DISPATCHCONTROL, - destroy_disp, disp, sizeof(isc_event_t)); - isc_task_setname(disp->task, "tcpdispatch", disp); /* @@ -1520,10 +1336,7 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, const isc_sockaddr_t *localaddr, bool *connected, dns_dispatch_t **dispp) { dns_dispatch_t *disp = NULL; - isc_sockaddr_t peeraddr; - isc_sockaddr_t sockname; unsigned int attributes, mask; - bool match = false; REQUIRE(VALID_DISPATCHMGR(mgr)); REQUIRE(destaddr != NULL); @@ -1535,24 +1348,32 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, DNS_DISPATCHATTR_CONNECTED; LOCK(&mgr->lock); +again: disp = ISC_LIST_HEAD(mgr->list); - while (disp != NULL && !match) { + while (disp != NULL && *dispp == NULL) { + isc_sockaddr_t sockname; + isc_sockaddr_t peeraddr; + LOCK(&disp->lock); - if ((disp->shutting_down == 0) && - ATTRMATCH(disp->attributes, attributes, mask) && + + if (disp->handle != NULL) { + sockname = isc_nmhandle_localaddr(disp->handle); + peeraddr = isc_nmhandle_peeraddr(disp->handle); + } else { + sockname = disp->local; + peeraddr = disp->peer; + } + + if (ATTRMATCH(disp->attributes, attributes, mask) && (localaddr == NULL || isc_sockaddr_eqaddr(localaddr, &disp->local))) { - sockname = isc_nmhandle_localaddr(disp->handle); - peeraddr = isc_nmhandle_peeraddr(disp->handle); if (isc_sockaddr_equal(destaddr, &peeraddr) && (localaddr == NULL || isc_sockaddr_eqaddr(localaddr, &sockname))) { /* attach */ - isc_refcount_increment(&disp->refcount); - *dispp = disp; - match = true; + dns_dispatch_attach(disp, dispp); if (connected != NULL) { *connected = true; } @@ -1561,34 +1382,21 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, UNLOCK(&disp->lock); disp = ISC_LIST_NEXT(disp, link); } - if (match || connected == NULL) { + + if (*dispp != NULL) { UNLOCK(&mgr->lock); - return (match ? ISC_R_SUCCESS : ISC_R_NOTFOUND); + return (ISC_R_SUCCESS); } - /* Second pass, only if connected != NULL */ - attributes = DNS_DISPATCHATTR_TCP; - - disp = ISC_LIST_HEAD(mgr->list); - while (disp != NULL && !match) { - LOCK(&disp->lock); - if ((disp->shutting_down == 0) && - ATTRMATCH(disp->attributes, attributes, mask) && - (localaddr == NULL || - isc_sockaddr_eqaddr(localaddr, &disp->local)) && - isc_sockaddr_equal(destaddr, &disp->peer)) - { - /* attach */ - isc_refcount_increment(&disp->refcount); - *dispp = disp; - match = true; - } - UNLOCK(&disp->lock); - disp = ISC_LIST_NEXT(disp, link); + if (connected != NULL) { + /* Second pass, only if connected != NULL */ + attributes = DNS_DISPATCHATTR_TCP; + connected = NULL; + goto again; } + UNLOCK(&mgr->lock); - - return (match ? ISC_R_SUCCESS : ISC_R_NOTFOUND); + return (ISC_R_NOTFOUND); } isc_result_t @@ -1648,18 +1456,16 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, disp->local = *localaddr; result = isc_task_create(taskmgr, 0, &disp->task); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_create() -> %p\n", __FILE__, + __LINE__, __func__, disp->task); +#endif /* DISPATCH_TRACE */ + if (result != ISC_R_SUCCESS) { goto cleanup; } - disp->ctlevent = - isc_event_allocate(mgr->mctx, disp, DNS_EVENT_DISPATCHCONTROL, - destroy_disp, disp, sizeof(isc_event_t)); - - disp->sepool = NULL; - isc_mem_create(&disp->sepool); - isc_mem_setname(disp->sepool, "disp_sepool"); - /* * Append it to the dispatcher list. */ @@ -1681,53 +1487,90 @@ cleanup: return (result); } +static void +dns_dispatch_destroy(dns_dispatch_t *disp) { + dns_dispatchmgr_t *mgr = disp->mgr; + + LOCK(&mgr->lock); + ISC_LIST_UNLINK(mgr->list, disp, link); + UNLOCK(&mgr->lock); + + dispatch_log(disp, LVL(90), "shutting down; detaching from handle %p", + disp->handle); + + if (disp->handle != NULL) { + isc_nmhandle_detach(&disp->handle); + } + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_detach(%p) -> %p\n", __FILE__, + __LINE__, __func__, &disp->task, disp->task); +#endif /* DISPATCH_TRACE */ + + isc_task_detach(&disp->task); + + dispatch_free(&disp); + + /* Because dispatch uses mgr->mctx, we must detach after freeing + * dispatch, not before + */ + dns_dispatchmgr_detach(&mgr); +} + void -dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp) { +dns__dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp, + const char *func, const char *file, unsigned int line) { + uint_fast32_t ref; + REQUIRE(VALID_DISPATCH(disp)); REQUIRE(dispp != NULL && *dispp == NULL); - isc_refcount_increment(&disp->refcount); + ref = isc_refcount_increment(&disp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, disp, dispp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* DISPATCH_TRACE */ + *dispp = disp; } void -dns_dispatch_detach(dns_dispatch_t **dispp) { +dns__dispatch_detach(dns_dispatch_t **dispp, const char *func, const char *file, + unsigned int line) { dns_dispatch_t *disp = NULL; - dispsocket_t *dispsock = NULL; - bool killit = false; + uint_fast32_t ref; REQUIRE(dispp != NULL && VALID_DISPATCH(*dispp)); disp = *dispp; *dispp = NULL; - if (isc_refcount_decrement(&disp->refcount) == 1) { + ref = isc_refcount_decrement(&disp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, disp, dispp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* DISPATCH_TRACE */ + + dispatch_log(disp, LVL(90), "detach: refcount %" PRIuFAST32, ref - 1); + + if (ref == 1) { LOCK(&disp->lock); - if (disp->recv_pending != 0 && disp->handle != NULL) { - isc_nm_cancelread(disp->handle); - } - if (disp->handle != NULL) { - isc_nmhandle_detach(&disp->handle); - } - for (dispsock = ISC_LIST_HEAD(disp->activesockets); - dispsock != NULL; dispsock = ISC_LIST_NEXT(dispsock, link)) - { - if (dispsock->handle != NULL) { - isc_nmhandle_detach(&dispsock->handle); - } - } - disp->shutting_down = 1; - do_cancel(disp); - - killit = destroy_disp_ok(disp); + REQUIRE(disp->recv_pending == 0); + REQUIRE(ISC_LIST_EMPTY(disp->active)); UNLOCK(&disp->lock); - } - dispatch_log(disp, LVL(90), "detach: refcount %" PRIuFAST32, - isc_refcount_current(&disp->refcount)); - - if (killit) { - isc_task_send(disp->task, &disp->ctlevent); + dns_dispatch_destroy(disp); } } @@ -1738,15 +1581,16 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, isc_nm_cb_t sent, isc_taskaction_t action, isc_nm_cb_t timedout, void *arg, dns_messageid_t *idp, dns_dispentry_t **resp) { - isc_result_t result; dns_dispentry_t *res = NULL; - dispsocket_t *dispsocket = NULL; dns_qid_t *qid = NULL; in_port_t localport = 0; dns_messageid_t id; unsigned int bucket; bool ok = false; int i = 0; + isc_taskaction_t oldest_action = NULL; + isc_task_t *oldest_task = NULL; + isc_event_t *oldest_ev = NULL; REQUIRE(VALID_DISPATCH(disp)); REQUIRE(task != NULL); @@ -1758,11 +1602,6 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, LOCK(&disp->lock); - if (disp->shutting_down == 1) { - UNLOCK(&disp->lock); - return (ISC_R_SHUTTINGDOWN); - } - if (disp->requests >= DNS_DISPATCH_MAXREQUESTS) { UNLOCK(&disp->lock); return (ISC_R_QUOTA); @@ -1773,42 +1612,51 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, if (disp->socktype == isc_socktype_udp && disp->nsockets > DNS_DISPATCH_SOCKSQUOTA) { - dispsocket_t *oldestsocket = NULL; - dns_dispentry_t *oldestresp = NULL; + dns_dispentry_t *oldest = NULL; dns_dispatchevent_t *rev = NULL; /* * Kill oldest outstanding query if the number of sockets * exceeds the quota to keep the room for new queries. */ - oldestsocket = ISC_LIST_HEAD(disp->activesockets); - oldestresp = oldestsocket->resp; - if (oldestresp != NULL && !oldestresp->item_out) { - rev = allocate_devent(oldestresp->disp); + oldest = ISC_LIST_HEAD(disp->active); + if (oldest != NULL) { + rev = allocate_devent(oldest->disp); rev->buffer.base = NULL; rev->result = ISC_R_CANCELED; ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, - DNS_EVENT_DISPATCH, oldestresp->action, - oldestresp->arg, oldestresp, NULL, NULL); - oldestresp->item_out = true; - isc_task_send(oldestresp->task, ISC_EVENT_PTR(&rev)); + DNS_EVENT_DISPATCH, oldest->action, + oldest->arg, oldest, NULL, NULL); + + oldest_action = oldest->action; + oldest_task = oldest->task; + oldest_ev = (isc_event_t *)rev; + inc_stats(disp->mgr, dns_resstatscounter_dispabort); } - - /* - * Move this entry to the tail so that it won't (easily) be - * examined before actually being canceled. - */ - ISC_LIST_UNLINK(disp->activesockets, oldestsocket, link); - ISC_LIST_APPEND(disp->activesockets, oldestsocket, link); } + res = isc_mem_get(disp->mgr->mctx, sizeof(*res)); + + *res = (dns_dispentry_t){ .port = localport, + .timeout = timeout, + .peer = *dest, + .connected = connected, + .sent = sent, + .timedout = timedout, + .action = action, + .arg = arg }; + + isc_refcount_init(&res->references, 1); + + ISC_LIST_INIT(res->items); + ISC_LINK_INIT(res, link); + ISC_LINK_INIT(res, alink); + if (disp->socktype == isc_socktype_udp) { - /* - * Get a separate UDP socket with a random port number. - */ - result = get_dispsocket(disp, dest, &dispsocket, &localport); + isc_result_t result = setup_socket(disp, res, dest, &localport); if (result != ISC_R_SUCCESS) { + isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); UNLOCK(&disp->lock); inc_stats(disp->mgr, dns_resstatscounter_dispsockfail); return (result); @@ -1841,38 +1689,26 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, UNLOCK(&qid->lock); if (!ok) { + isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); UNLOCK(&disp->lock); return (ISC_R_NOMORE); } - res = isc_mem_get(disp->mgr->mctx, sizeof(*res)); - isc_refcount_increment0(&disp->mgr->irefs); - - *res = (dns_dispentry_t){ .id = id, - .port = localport, - .bucket = bucket, - .timeout = timeout, - .peer = *dest, - .connected = connected, - .sent = sent, - .timedout = timedout, - .action = action, - .arg = arg, - .dispsocket = dispsocket }; - dns_dispatch_attach(disp, &res->disp); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_attach(%p, %p)\n", __FILE__, + __LINE__, __func__, task, &res->task); +#endif /* DISPATCH_TRACE */ + isc_task_attach(task, &res->task); - ISC_LIST_INIT(res->items); - ISC_LINK_INIT(res, link); + res->id = id; + res->bucket = bucket; res->magic = RESPONSE_MAGIC; disp->requests++; - if (dispsocket != NULL) { - dispsocket->resp = res; - } - LOCK(&qid->lock); ISC_LIST_APPEND(qid->qid_table[bucket], res, link); UNLOCK(&qid->lock); @@ -1883,13 +1719,13 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, ? dns_resstatscounter_disprequdp : dns_resstatscounter_dispreqtcp); - if (dispsocket != NULL) { - ISC_LIST_APPEND(disp->activesockets, dispsocket, link); - } + ISC_LIST_APPEND(disp->active, res, alink); UNLOCK(&disp->lock); - INSIST(disp->socktype == isc_socktype_tcp || res->dispsocket != NULL); + if (oldest_action != NULL) { + oldest_action(oldest_task, oldest_ev); + } *idp = id; *resp = res; @@ -1901,6 +1737,7 @@ isc_result_t dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent) { dns_dispatch_t *disp = NULL; dns_dispatchevent_t *ev = NULL; + isc_taskaction_t action = NULL; REQUIRE(VALID_RESPONSE(resp)); REQUIRE(sockevent != NULL && *sockevent != NULL); @@ -1913,16 +1750,8 @@ dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent) { LOCK(&disp->lock); - REQUIRE(resp->item_out); - resp->item_out = false; - free_devent(disp, ev); - if (disp->shutting_down == 1) { - UNLOCK(&disp->lock); - return (ISC_R_SHUTTINGDOWN); - } - ev = ISC_LIST_HEAD(resp->items); if (ev != NULL) { ISC_LIST_UNLINK(resp->items, ev, ev_link); @@ -1931,40 +1760,44 @@ dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent) { request_log(disp, resp, LVL(90), "[c] Sent event %p buffer %p len %d to task %p", ev, ev->buffer.base, ev->buffer.length, resp->task); - resp->item_out = true; - isc_task_send(resp->task, ISC_EVENT_PTR(&ev)); + + action = resp->action; } - startrecv(disp, resp->dispsocket); + startrecv(disp, resp); UNLOCK(&disp->lock); + if (action != NULL) { + action(resp->task, (isc_event_t *)ev); + } + return (ISC_R_SUCCESS); } void -dns_dispatch_removeresponse(dns_dispentry_t **resp, +dns_dispatch_removeresponse(dns_dispentry_t **respp, dns_dispatchevent_t **sockevent) { dns_dispatchmgr_t *mgr = NULL; dns_dispatch_t *disp = NULL; - dns_dispentry_t *res = NULL; + dns_dispentry_t *resp = NULL; dns_dispatchevent_t *ev = NULL; - unsigned int bucket; - isc_eventlist_t events; dns_qid_t *qid = NULL; - REQUIRE(resp != NULL); - REQUIRE(VALID_RESPONSE(*resp)); + REQUIRE(respp != NULL); - res = *resp; - *resp = NULL; + resp = *respp; + + REQUIRE(VALID_RESPONSE(resp)); + + disp = resp->disp; - disp = res->disp; - res->disp = NULL; REQUIRE(VALID_DISPATCH(disp)); mgr = disp->mgr; + REQUIRE(VALID_DISPATCHMGR(mgr)); + qid = mgr->qid; if (sockevent != NULL) { @@ -1978,95 +1811,66 @@ dns_dispatch_removeresponse(dns_dispentry_t **resp, LOCK(&disp->lock); INSIST(disp->requests > 0); disp->requests--; + dec_stats(disp->mgr, (disp->socktype == isc_socktype_udp) ? dns_resstatscounter_disprequdp : dns_resstatscounter_dispreqtcp); - if (res->dispsocket != NULL) { - deactivate_dispsocket(disp, res->dispsocket); - } - UNLOCK(&disp->lock); - - bucket = res->bucket; + deactivate_dispentry(disp, resp); LOCK(&qid->lock); - ISC_LIST_UNLINK(qid->qid_table[bucket], res, link); + ISC_LIST_UNLINK(qid->qid_table[resp->bucket], resp, link); UNLOCK(&qid->lock); - - if (ev == NULL && res->item_out) { - unsigned int n; - - /* - * We've posted our event, but the caller hasn't gotten it - * yet. Take it back. - */ - ISC_LIST_INIT(events); - n = isc_task_unsend(res->task, res, DNS_EVENT_DISPATCH, NULL, - &events); - /* - * We had better have gotten it back. - */ - INSIST(n == 1); - ev = (dns_dispatchevent_t *)ISC_LIST_HEAD(events); - } + UNLOCK(&disp->lock); if (ev != NULL) { - REQUIRE(res->item_out); - res->item_out = false; free_devent(disp, ev); } - request_log(disp, res, LVL(90), "detaching from task %p", res->task); - isc_task_detach(&res->task); - /* * Free any buffered responses as well */ - ev = ISC_LIST_HEAD(res->items); + ev = ISC_LIST_HEAD(resp->items); while (ev != NULL) { - ISC_LIST_UNLINK(res->items, ev, ev_link); + ISC_LIST_UNLINK(resp->items, ev, ev_link); free_devent(disp, ev); - ev = ISC_LIST_HEAD(res->items); + ev = ISC_LIST_HEAD(resp->items); } - res->magic = 0; - isc_refcount_decrement(&disp->mgr->irefs); - isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); - dns_dispatch_detach(&disp); + dispentry_detach(respp); } /* * disp must be locked. */ static void -startrecv(dns_dispatch_t *disp, dispsocket_t *dispsocket) { +startrecv(dns_dispatch_t *disp, dns_dispentry_t *resp) { isc_nmhandle_t *handle = NULL; - - if (disp->shutting_down == 1) { - return; - } - - if (dispsocket == NULL) { - if (disp->socktype == isc_socktype_udp || - disp->recv_pending != 0) { - return; - } - isc_nmhandle_attach(disp->handle, &handle); - } else { - handle = dispsocket->handle; - } + dns_dispentry_t *tmp_resp = NULL; + dns_dispatch_t *tmp_disp = NULL; switch (disp->socktype) { case isc_socktype_udp: - isc_nm_read(handle, udp_recv, dispsocket); - break; + REQUIRE(resp != NULL); + REQUIRE(resp->handle != NULL); + dispentry_attach(resp, &tmp_resp); + + /* resp->handle is detached in _removeresponse() */ + isc_nm_read(resp->handle, udp_recv, resp); + + break; case isc_socktype_tcp: + if (resp != NULL) { + REQUIRE(resp->handle == NULL); + } + + isc_nmhandle_attach(disp->handle, &handle); + dns_dispatch_attach(disp, &tmp_disp); isc_nm_read(handle, tcp_recv, disp); INSIST(disp->recv_pending == 0); disp->recv_pending = 1; break; - default: INSIST(0); ISC_UNREACHABLE(); @@ -2077,36 +1881,64 @@ static void disp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { dns_dispentry_t *resp = (dns_dispentry_t *)arg; dns_dispatch_t *disp = resp->disp; - dispsocket_t *dispsocket = NULL; + if (resp->canceled && eresult == ISC_R_SUCCESS) { + eresult = ISC_R_CANCELED; + goto detach; + } + + if (MGR_IS_SHUTTINGDOWN(disp->mgr)) { + eresult = ISC_R_SHUTTINGDOWN; + } + + /* + * Start listening on success + */ if (eresult == ISC_R_SUCCESS) { - if (disp->socktype == isc_socktype_udp) { - dispsocket = resp->dispsocket; - isc_nmhandle_attach(handle, &dispsocket->handle); - } else if (disp->handle == NULL) { - disp->attributes |= DNS_DISPATCHATTR_CONNECTED; - isc_nmhandle_attach(handle, &disp->handle); + switch (disp->socktype) { + case isc_socktype_udp: + isc_nmhandle_attach(handle, &resp->handle); + startrecv(disp, resp); + break; + case isc_socktype_tcp: + if (disp->handle == NULL) { + LOCK(&disp->lock); + isc_nmhandle_attach(handle, &disp->handle); + disp->attributes |= DNS_DISPATCHATTR_CONNECTED; + UNLOCK(&disp->lock); + startrecv(disp, resp); + } + break; + default: + INSIST(0); + ISC_UNREACHABLE(); } - - startrecv(disp, dispsocket); } if (resp->connected != NULL) { resp->connected(handle, eresult, resp->arg); } + +detach: + dispentry_detach(&resp); } isc_result_t dns_dispatch_connect(dns_dispentry_t *resp) { dns_dispatch_t *disp = NULL; + dns_dispentry_t *tmp = NULL; REQUIRE(VALID_RESPONSE(resp)); disp = resp->disp; + dispentry_attach(resp, &tmp); /* detached in disp_connected */ + switch (disp->socktype) { case isc_socktype_tcp: + INSIST(disp->handle == NULL); if (disp->handle != NULL) { + /* We are already connected */ break; } @@ -2114,9 +1946,8 @@ dns_dispatch_connect(dns_dispentry_t *resp) { disp_connected, resp, resp->timeout, 0); break; case isc_socktype_udp: - isc_nm_udpconnect(disp->mgr->nm, &resp->dispsocket->local, - &resp->dispsocket->peer, disp_connected, resp, - resp->timeout, 0); + isc_nm_udpconnect(disp->mgr->nm, &resp->local, &resp->peer, + disp_connected, resp, resp->timeout, 0); break; default: return (ISC_R_NOTIMPLEMENTED); @@ -2125,9 +1956,25 @@ dns_dispatch_connect(dns_dispentry_t *resp) { return (ISC_R_SUCCESS); } +static void +send_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { + dns_dispentry_t *resp = (dns_dispentry_t *)cbarg; + + REQUIRE(VALID_RESPONSE(resp)); + + resp->sent(handle, result, resp->arg); + + if (result != ISC_R_SUCCESS) { + isc_nm_cancelread(handle); + } + + dispentry_detach(&resp); +} + void dns_dispatch_send(dns_dispentry_t *resp, isc_region_t *r, isc_dscp_t dscp) { isc_nmhandle_t *handle = NULL; + dns_dispentry_t *tmp = NULL; REQUIRE(VALID_RESPONSE(resp)); @@ -2149,108 +1996,38 @@ dns_dispatch_send(dns_dispentry_t *resp, isc_region_t *r, isc_dscp_t dscp) { } #endif - isc_nm_send(handle, r, resp->sent, resp->arg); + dispentry_attach(resp, &tmp); /* detached in send_done() */ + isc_nm_send(handle, r, send_done, resp); } void -dns_dispatch_cancel(dns_dispatch_t *disp, dns_dispentry_t *resp, bool sending, - bool connecting) { +dns_dispatch_cancel(dns_dispentry_t *resp) { + REQUIRE(VALID_RESPONSE(resp)); + + resp->canceled = true; + + if (resp->handle) { + isc_nm_cancelread(resp->handle); + } else if (resp->disp->handle != NULL) { + isc_nm_cancelread(resp->disp->handle); + } +} + +static inline isc_nmhandle_t * +getentryhandle(dns_dispentry_t *resp) { isc_nmhandle_t *handle = NULL; - REQUIRE(disp != NULL || resp != NULL); - - if (resp != NULL) { - REQUIRE(VALID_RESPONSE(resp)); - handle = getentryhandle(resp); - } else if (disp != NULL) { - REQUIRE(VALID_DISPATCH(disp)); - handle = gethandle(disp); - } else { - INSIST(0); - ISC_UNREACHABLE(); - } - - if (handle == NULL) { - return; - } - - if (connecting) { - // isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_CONNECT); - } - - if (sending) { - // isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_SEND); - } -} - -/* - * disp must be locked. - */ -static void -do_cancel(dns_dispatch_t *disp) { - dns_dispatchevent_t *ev = NULL; - dns_dispentry_t *resp = NULL; - dns_qid_t *qid = disp->mgr->qid; - - if (disp->shutdown_out == 1) { - return; - } - - /* - * Search for the first response handler without packets outstanding - * unless a specific handler is given. - */ - LOCK(&qid->lock); - for (resp = linear_first(qid); resp != NULL && resp->item_out; - /* Empty. */) - { - resp = linear_next(qid, resp); - } - - /* - * No one to send the cancel event to, so nothing to do. - */ - if (resp == NULL) { - goto unlock; - } - - /* - * Send the shutdown failsafe event to this resp. - */ - ev = disp->failsafe_ev; - disp->failsafe_ev = NULL; - ISC_EVENT_INIT(ev, sizeof(*ev), 0, NULL, DNS_EVENT_DISPATCH, - resp->action, resp->arg, resp, NULL, NULL); - ev->result = disp->shutdown_why; - ev->buffer.base = NULL; - ev->buffer.length = 0; - disp->shutdown_out = 1; - request_log(disp, resp, LVL(10), "cancel: failsafe event %p -> task %p", - ev, resp->task); - resp->item_out = true; - isc_task_send(resp->task, ISC_EVENT_PTR(&ev)); -unlock: - UNLOCK(&qid->lock); -} - -static isc_nmhandle_t * -gethandle(dns_dispatch_t *disp) { - REQUIRE(VALID_DISPATCH(disp)); - - return (disp->handle); -} - -static isc_nmhandle_t * -getentryhandle(dns_dispentry_t *resp) { REQUIRE(VALID_RESPONSE(resp)); if (resp->disp->socktype == isc_socktype_tcp) { - return (resp->disp->handle); - } else if (resp->dispsocket != NULL) { - return (resp->dispsocket->handle); + handle = resp->disp->handle; } else { - return (NULL); + handle = resp->handle; } + + INSIST(handle != NULL); + + return (handle); } isc_result_t @@ -2275,8 +2052,8 @@ dns_dispentry_getlocaladdress(dns_dispentry_t *resp, isc_sockaddr_t *addrp) { return (ISC_R_SUCCESS); } - if (resp->dispsocket != NULL && resp->dispsocket->handle != NULL) { - *addrp = isc_nmhandle_localaddr(resp->dispsocket->handle); + if (resp->handle != NULL) { + *addrp = isc_nmhandle_localaddr(resp->handle); return (ISC_R_SUCCESS); } @@ -2288,8 +2065,8 @@ dns_dispatch_getattributes(dns_dispatch_t *disp) { REQUIRE(VALID_DISPATCH(disp)); /* - * We don't bother locking disp here; it's the caller's responsibility - * to use only non volatile flags. + * We don't bother locking disp here; it's the caller's + * responsibility to use only non volatile flags. */ return (disp->attributes); } diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index 762a979ad8..01054a1dc0 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -79,9 +79,10 @@ ISC_LANG_BEGINDECLS struct dns_dispatchevent { ISC_EVENT_COMMON(dns_dispatchevent_t); /*%< standard event common */ - isc_result_t result; /*%< result code */ - isc_region_t region; /*%< data region */ - isc_buffer_t buffer; /*%< data buffer */ + isc_result_t result; /*%< result code */ + isc_region_t region; /*%< data region */ + isc_buffer_t buffer; /*%< data buffer */ + dns_dispatch_t *dispatch; }; /*% @@ -146,8 +147,19 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm, dns_dispatchmgr_t **mgrp); *\li anything else -- failure */ +#define dns_dispatchmgr_attach(mgr, mgrp) \ + dns__dispatchmgr_attach(mgr, mgrp, __func__, __FILE__, __LINE__) + void -dns_dispatchmgr_destroy(dns_dispatchmgr_t **mgrp); +dns__dispatchmgr_attach(dns_dispatchmgr_t *mgr, dns_dispatchmgr_t **mgrp, + const char *func, const char *file, unsigned int line); + +#define dns_dispatchmgr_detach(mgrp) \ + dns__dispatchmgr_detach(mgrp, __func__, __FILE__, __LINE__) + +void +dns__dispatchmgr_detach(dns_dispatchmgr_t **mgrp, const char *func, + const char *file, unsigned int line); /*%< * Destroys the dispatchmgr when it becomes empty. This could be * immediately. @@ -246,8 +258,12 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, *\li Anything else -- failure. */ +#define dns_dispatch_attach(d, dp) \ + dns__dispatch_attach(d, dp, __func__, __FILE__, __LINE__) + void -dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp); +dns__dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp, + const char *func, const char *file, unsigned int line); /*%< * Attach to a dispatch handle. * @@ -257,8 +273,12 @@ dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp); *\li dispp != NULL && *dispp == NULL */ +#define dns_dispatch_detach(dp) \ + dns__dispatch_detach(dp, __func__, __FILE__, __LINE__) + void -dns_dispatch_detach(dns_dispatch_t **dispp); +dns__dispatch_detach(dns_dispatch_t **dispp, const char *func, const char *file, + unsigned int line); /*%< * Detaches from the dispatch. * @@ -277,15 +297,13 @@ dns_dispatch_connect(dns_dispentry_t *resp); */ void -dns_dispatch_cancel(dns_dispatch_t *disp, dns_dispentry_t *resp, bool sending, - bool connecting); +dns_dispatch_cancel(dns_dispentry_t *resp); /*%< - * Cancel pending sends (if 'sending' is true) and connects (if - * 'connecting' is true) in 'resp' or 'disp'. + * Cancel pending connects in 'resp', by setting a flag so that + * a read is not started when the connect handler runs. * * Requires: - *\li 'resp' is NULL and 'disp' is valid, or - *\li 'disp' is NULL and 'resp' is valid. + *\li 'resp' is valid. */ void diff --git a/lib/dns/include/dns/request.h b/lib/dns/include/dns/request.h index 9554603b50..fd8e08cebe 100644 --- a/lib/dns/include/dns/request.h +++ b/lib/dns/include/dns/request.h @@ -124,8 +124,12 @@ dns_requestmgr_shutdown(dns_requestmgr_t *requestmgr); *\li 'requestmgr' is a valid requestmgr. */ +#define dns_requestmgr_attach(source, targetp) \ + dns__requestmgr_attach(source, targetp, __FILE__, __LINE__, __func__) + void -dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp); +dns__requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp, + const char *file, unsigned int line, const char *func); /*%< * Attach to the request manager. dns_requestmgr_shutdown() must not * have been called on 'source' prior to calling dns_requestmgr_attach(). @@ -137,8 +141,12 @@ dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp); *\li 'targetp' to be non NULL and '*targetp' to be NULL. */ +#define dns_requestmgr_detach(requestmgrp) \ + dns__requestmgr_detach(requestmgrp, __FILE__, __LINE__, __func__) + void -dns_requestmgr_detach(dns_requestmgr_t **requestmgrp); +dns__requestmgr_detach(dns_requestmgr_t **requestmgrp, const char *file, + unsigned int line, const char *func); /*%< * Detach from the given requestmgr. If this is the final detach * requestmgr will be destroyed. dns_requestmgr_shutdown() must diff --git a/lib/dns/request.c b/lib/dns/request.c index fcb378ed6a..7d2b938fa3 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -32,6 +32,8 @@ #include #include +#define REQ_TRACE + #define REQUESTMGR_MAGIC ISC_MAGIC('R', 'q', 'u', 'M') #define VALID_REQUESTMGR(mgr) ISC_MAGIC_VALID(mgr, REQUESTMGR_MAGIC) @@ -44,17 +46,17 @@ typedef ISC_LIST(dns_request_t) dns_requestlist_t; struct dns_requestmgr { unsigned int magic; + isc_refcount_t references; + isc_mutex_t lock; isc_mem_t *mctx; /* locked */ - int32_t eref; - int32_t iref; isc_taskmgr_t *taskmgr; dns_dispatchmgr_t *dispatchmgr; dns_dispatch_t *dispatchv4; dns_dispatch_t *dispatchv6; - bool exiting; + atomic_bool exiting; isc_eventlist_t whenshutdown; unsigned int hash; isc_mutex_t locks[DNS_REQUEST_NLOCKS]; @@ -63,6 +65,8 @@ struct dns_requestmgr { struct dns_request { unsigned int magic; + isc_refcount_t references; + unsigned int hash; isc_mem_t *mctx; int32_t flags; @@ -97,8 +101,6 @@ struct dns_request { static void mgr_destroy(dns_requestmgr_t *requestmgr); -static void -mgr_shutdown(dns_requestmgr_t *requestmgr); static unsigned int mgr_gethash(dns_requestmgr_t *requestmgr); static void @@ -118,12 +120,23 @@ req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg); static void req_timeout(isc_nmhandle_t *handle, isc_result_t eresult, void *arg); static void +__req_attach(dns_request_t *source, dns_request_t **targetp, const char *file, + unsigned int line, const char *func); +static void +__req_detach(dns_request_t **requestp, const char *file, unsigned int line, + const char *func); +static void req_destroy(dns_request_t *request); static void req_log(int level, const char *fmt, ...) ISC_FORMAT_PRINTF(2, 3); void request_cancel(dns_request_t *request); +#define req_attach(source, targetp) \ + __req_attach(source, targetp, __FILE__, __LINE__, __func__) +#define req_detach(requestp) \ + __req_detach(requestp, __FILE__, __LINE__, __func__) + /*** *** Public ***/ @@ -171,11 +184,10 @@ dns_requestmgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, } requestmgr->mctx = NULL; isc_mem_attach(mctx, &requestmgr->mctx); - requestmgr->eref = 1; /* implicit attach */ - requestmgr->iref = 0; + isc_refcount_init(&requestmgr->references, 1); ISC_LIST_INIT(requestmgr->whenshutdown); ISC_LIST_INIT(requestmgr->requests); - requestmgr->exiting = false; + atomic_init(&requestmgr->exiting, false); requestmgr->hash = 0; requestmgr->magic = REQUESTMGR_MAGIC; @@ -201,7 +213,7 @@ dns_requestmgr_whenshutdown(dns_requestmgr_t *requestmgr, isc_task_t *task, LOCK(&requestmgr->lock); - if (requestmgr->exiting) { + if (atomic_load_acquire(&requestmgr->exiting)) { /* * We're already shutdown. Send the event. */ @@ -218,126 +230,95 @@ dns_requestmgr_whenshutdown(dns_requestmgr_t *requestmgr, isc_task_t *task, void dns_requestmgr_shutdown(dns_requestmgr_t *requestmgr) { + dns_request_t *request; + REQUIRE(VALID_REQUESTMGR(requestmgr)); req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_shutdown: %p", requestmgr); - LOCK(&requestmgr->lock); - mgr_shutdown(requestmgr); - UNLOCK(&requestmgr->lock); -} - -static void -mgr_shutdown(dns_requestmgr_t *requestmgr) { - dns_request_t *request; - - if (!requestmgr->exiting) { - requestmgr->exiting = true; - for (request = ISC_LIST_HEAD(requestmgr->requests); - request != NULL; request = ISC_LIST_NEXT(request, link)) - { - dns_request_cancel(request); - } - if (requestmgr->iref == 0) { - INSIST(ISC_LIST_EMPTY(requestmgr->requests)); - send_shutdown_events(requestmgr); - } + if (!atomic_compare_exchange_strong(&requestmgr->exiting, + &(bool){ false }, true)) + { + return; } -} - -static void -requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp) { - /* - * Locked by caller. - */ - - REQUIRE(VALID_REQUESTMGR(source)); - REQUIRE(targetp != NULL && *targetp == NULL); - - REQUIRE(!source->exiting); - - source->iref++; - *targetp = source; - - req_log(ISC_LOG_DEBUG(3), "requestmgr_attach: %p: eref %d iref %d", - source, source->eref, source->iref); -} - -static void -requestmgr_detach(dns_requestmgr_t **requestmgrp) { - dns_requestmgr_t *requestmgr; - bool need_destroy = false; - - REQUIRE(requestmgrp != NULL); - requestmgr = *requestmgrp; - *requestmgrp = NULL; - REQUIRE(VALID_REQUESTMGR(requestmgr)); LOCK(&requestmgr->lock); - INSIST(requestmgr->iref > 0); - requestmgr->iref--; + for (request = ISC_LIST_HEAD(requestmgr->requests); request != NULL; + request = ISC_LIST_NEXT(request, link)) + { + dns_request_cancel(request); + } - req_log(ISC_LOG_DEBUG(3), "requestmgr_detach: %p: eref %d iref %d", - requestmgr, requestmgr->eref, requestmgr->iref); - - if (requestmgr->iref == 0 && requestmgr->exiting) { - INSIST(ISC_LIST_HEAD(requestmgr->requests) == NULL); + if (ISC_LIST_EMPTY(requestmgr->requests)) { send_shutdown_events(requestmgr); - if (requestmgr->eref == 0) { - need_destroy = true; - } } - UNLOCK(&requestmgr->lock); - if (need_destroy) { - mgr_destroy(requestmgr); - } + UNLOCK(&requestmgr->lock); } void -dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp) { +dns__requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp, + const char *file, unsigned int line, const char *func) { + uint_fast32_t ref; + REQUIRE(VALID_REQUESTMGR(source)); REQUIRE(targetp != NULL && *targetp == NULL); - REQUIRE(!source->exiting); - LOCK(&source->lock); - source->eref++; + REQUIRE(!atomic_load_acquire(&source->exiting)); + + ref = isc_refcount_increment(&source->references); + +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, source, targetp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ + + req_log(ISC_LOG_DEBUG(3), + "dns_requestmgr_attach: %p: references = %" PRIuFAST32, source, + ref + 1); + *targetp = source; - UNLOCK(&source->lock); - - req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_attach: %p: eref %d iref %d", - source, source->eref, source->iref); } void -dns_requestmgr_detach(dns_requestmgr_t **requestmgrp) { - dns_requestmgr_t *requestmgr; - bool need_destroy = false; +dns__requestmgr_detach(dns_requestmgr_t **requestmgrp, const char *file, + unsigned int line, const char *func) { + dns_requestmgr_t *requestmgr = NULL; + uint_fast32_t ref; + + REQUIRE(requestmgrp != NULL && VALID_REQUESTMGR(*requestmgrp)); - REQUIRE(requestmgrp != NULL); requestmgr = *requestmgrp; *requestmgrp = NULL; - REQUIRE(VALID_REQUESTMGR(requestmgr)); - LOCK(&requestmgr->lock); - INSIST(requestmgr->eref > 0); - requestmgr->eref--; + ref = isc_refcount_decrement(&requestmgr->references); - req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_detach: %p: eref %d iref %d", - requestmgr, requestmgr->eref, requestmgr->iref); +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, requestmgr, requestmgrp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ - if (requestmgr->eref == 0 && requestmgr->iref == 0) { - INSIST(requestmgr->exiting && - ISC_LIST_HEAD(requestmgr->requests) == NULL); - need_destroy = true; - } - UNLOCK(&requestmgr->lock); + req_log(ISC_LOG_DEBUG(3), + "dns_requestmgr_detach: %p: references = %" PRIuFAST32, + requestmgr, ref - 1); - if (need_destroy) { + if (ref == 1) { + INSIST(ISC_LIST_EMPTY(requestmgr->requests)); mgr_destroy(requestmgr); } } +/* FIXME */ static void send_shutdown_events(dns_requestmgr_t *requestmgr) { isc_event_t *event, *next_event; @@ -365,8 +346,7 @@ mgr_destroy(dns_requestmgr_t *requestmgr) { req_log(ISC_LOG_DEBUG(3), "mgr_destroy"); - REQUIRE(requestmgr->eref == 0); - REQUIRE(requestmgr->iref == 0); + isc_refcount_destroy(&requestmgr->references); isc_mutex_destroy(&requestmgr->lock); for (i = 0; i < DNS_REQUEST_NLOCKS; i++) { @@ -409,12 +389,13 @@ req_send(dns_request_t *request) { static isc_result_t new_request(isc_mem_t *mctx, dns_request_t **requestp) { - dns_request_t *request; + dns_request_t *request = NULL; request = isc_mem_get(mctx, sizeof(*request)); *request = (dns_request_t){ .dscp = -1 }; ISC_LINK_INIT(request, link); + isc_refcount_init(&request->references, 1); isc_mem_attach(mctx, &request->mctx); request->magic = REQUEST_MAGIC; @@ -426,25 +407,25 @@ static bool isblackholed(dns_dispatchmgr_t *dispatchmgr, const isc_sockaddr_t *destaddr) { dns_acl_t *blackhole; isc_netaddr_t netaddr; - int match; - bool drop = false; char netaddrstr[ISC_NETADDR_FORMATSIZE]; + int match; + isc_result_t result; blackhole = dns_dispatchmgr_getblackhole(dispatchmgr); - if (blackhole != NULL) { - isc_netaddr_fromsockaddr(&netaddr, destaddr); - if (dns_acl_match(&netaddr, NULL, blackhole, NULL, &match, - NULL) == ISC_R_SUCCESS && - match > 0) - { - drop = true; - } + if (blackhole == NULL) { + return (false); } - if (drop) { - isc_netaddr_format(&netaddr, netaddrstr, sizeof(netaddrstr)); - req_log(ISC_LOG_DEBUG(10), "blackholed address %s", netaddrstr); + + isc_netaddr_fromsockaddr(&netaddr, destaddr); + result = dns_acl_match(&netaddr, NULL, blackhole, NULL, &match, NULL); + if (result != ISC_R_SUCCESS || match == 0) { + return (false); } - return (drop); + + isc_netaddr_format(&netaddr, netaddrstr, sizeof(netaddrstr)); + req_log(ISC_LOG_DEBUG(10), "blackholed address %s", netaddrstr); + + return (true); } static isc_result_t @@ -553,6 +534,10 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, req_log(ISC_LOG_DEBUG(3), "dns_request_createraw"); + if (atomic_load_acquire(&requestmgr->exiting)) { + return (ISC_R_SHUTTINGDOWN); + } + if (isblackholed(requestmgr->dispatchmgr, destaddr)) { return (DNS_R_BLACKHOLED); } @@ -593,6 +578,12 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, request->timeout = udptimeout * 1000; } + isc_buffer_allocate(mctx, &request->query, r.length + (tcp ? 2 : 0)); + result = isc_buffer_copyregion(request->query, &r); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + again: result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, dscp, &connected, &request->dispatch); @@ -605,6 +596,9 @@ again: dispopt |= DNS_DISPATCHOPT_FIXEDID; } + dns_request_t *tmp = NULL; + req_attach(request, &tmp); + result = dns_dispatch_addresponse( request->dispatch, dispopt, request->timeout, destaddr, task, req_connected, req_senddone, req_response, req_timeout, request, @@ -619,24 +613,13 @@ again: goto cleanup; } - isc_buffer_allocate(mctx, &request->query, r.length + (tcp ? 2 : 0)); - result = isc_buffer_copyregion(request->query, &r); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - /* Add message ID. */ isc_buffer_usedregion(request->query, &r); r.base[0] = (id >> 8) & 0xff; r.base[1] = id & 0xff; LOCK(&requestmgr->lock); - if (requestmgr->exiting) { - UNLOCK(&requestmgr->lock); - result = ISC_R_SHUTTINGDOWN; - goto cleanup; - } - requestmgr_attach(requestmgr, &request->requestmgr); + dns_requestmgr_attach(requestmgr, &request->requestmgr); request->hash = mgr_gethash(requestmgr); ISC_LIST_APPEND(requestmgr->requests, request, link); UNLOCK(&requestmgr->lock); @@ -665,7 +648,7 @@ cleanup: if (tclone != NULL) { isc_task_detach(&tclone); } - req_destroy(request); + req_detach(&request); req_log(ISC_LOG_DEBUG(3), "dns_request_createraw: failed %s", dns_result_totext(result)); return (result); @@ -697,7 +680,6 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message, isc_mem_t *mctx; dns_messageid_t id; bool tcp = false; - bool settsigkey = true; bool connected = false; REQUIRE(VALID_REQUESTMGR(requestmgr)); @@ -712,6 +694,10 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message, req_log(ISC_LOG_DEBUG(3), "dns_request_createvia"); + if (atomic_load_acquire(&requestmgr->exiting)) { + return (ISC_R_SHUTTINGDOWN); + } + if (srcaddr != NULL && isc_sockaddr_pf(srcaddr) != isc_sockaddr_pf(destaddr)) { return (ISC_R_FAMILYMISMATCH); @@ -740,7 +726,11 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message, dns_tsigkey_attach(key, &request->tsigkey); } -use_tcp: + result = dns_message_settsigkey(message, request->tsigkey); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + if ((options & DNS_REQUESTOPT_TCP) != 0) { tcp = true; request->timeout = timeout * 1000; @@ -754,12 +744,16 @@ use_tcp: request->timeout = udptimeout * 1000; } +use_tcp: result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, dscp, &connected, &request->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup; } + dns_request_t *tmp = NULL; + req_attach(request, &tmp); + result = dns_dispatch_addresponse( request->dispatch, 0, request->timeout, destaddr, task, req_connected, req_senddone, req_response, req_timeout, request, @@ -769,14 +763,8 @@ use_tcp: } message->id = id; - if (settsigkey) { - result = dns_message_settsigkey(message, request->tsigkey); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - } result = req_render(message, &request->query, options, mctx); - if (result == DNS_R_USETCP && (options & DNS_REQUESTOPT_TCP) == 0) { + if (result == DNS_R_USETCP && !tcp) { /* * Try again using TCP. */ @@ -784,7 +772,7 @@ use_tcp: dns_dispatch_removeresponse(&request->dispentry, NULL); dns_dispatch_detach(&request->dispatch); options |= DNS_REQUESTOPT_TCP; - settsigkey = false; + tcp = true; goto use_tcp; } if (result != ISC_R_SUCCESS) { @@ -797,12 +785,7 @@ use_tcp: } LOCK(&requestmgr->lock); - if (requestmgr->exiting) { - UNLOCK(&requestmgr->lock); - result = ISC_R_SHUTTINGDOWN; - goto cleanup; - } - requestmgr_attach(requestmgr, &request->requestmgr); + dns_requestmgr_attach(requestmgr, &request->requestmgr); request->hash = mgr_gethash(requestmgr); ISC_LIST_APPEND(requestmgr->requests, request, link); UNLOCK(&requestmgr->lock); @@ -831,7 +814,7 @@ cleanup: if (tclone != NULL) { isc_task_detach(&tclone); } - req_destroy(request); + req_detach(&request); req_log(ISC_LOG_DEBUG(3), "dns_request_createvia: failed %s", dns_result_totext(result)); return (result); @@ -956,9 +939,7 @@ request_cancel(dns_request_t *request) { request->flags &= ~DNS_REQUEST_F_CONNECTING; if (request->dispentry != NULL) { - dns_dispatch_cancel(NULL, request->dispentry, - DNS_REQUEST_SENDING(request), - DNS_REQUEST_CONNECTING(request)); + dns_dispatch_cancel(request->dispentry); dns_dispatch_removeresponse(&request->dispentry, NULL); } @@ -1047,7 +1028,7 @@ dns_request_destroy(dns_request_t **requestp) { INSIST(request->dispentry == NULL); INSIST(request->dispatch == NULL); - req_destroy(request); + req_detach(&request); } /*** @@ -1059,15 +1040,17 @@ req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { UNUSED(handle); + req_log(ISC_LOG_DEBUG(3), "req_connected: request %p: %s", request, + isc_result_totext(eresult)); + if (eresult == ISC_R_CANCELED) { + req_detach(&request); return; } REQUIRE(VALID_REQUEST(request)); REQUIRE(DNS_REQUEST_CONNECTING(request)); - req_log(ISC_LOG_DEBUG(3), "req_connected: request %p", request); - LOCK(&request->requestmgr->locks[request->hash]); request->flags &= ~DNS_REQUEST_F_CONNECTING; @@ -1084,6 +1067,8 @@ req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { send_if_done(request, ISC_R_CANCELED); } UNLOCK(&request->requestmgr->locks[request->hash]); + + req_detach(&request); } static void @@ -1195,12 +1180,76 @@ req_sendevent(dns_request_t *request, isc_result_t result) { isc_task_sendanddetach(&task, (isc_event_t **)&request->event); } +static void +__req_attach(dns_request_t *source, dns_request_t **targetp, const char *file, + unsigned int line, const char *func) { + uint_fast32_t ref; + + REQUIRE(VALID_REQUEST(source)); + REQUIRE(targetp != NULL && *targetp == NULL); + + ref = isc_refcount_increment(&source->references); + +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, source, targetp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ + + *targetp = source; +} + +static void +__req_detach(dns_request_t **requestp, const char *file, unsigned int line, + const char *func) { + dns_request_t *request = NULL; + uint_fast32_t ref; + + REQUIRE(requestp != NULL && VALID_REQUEST(*requestp)); + + request = *requestp; + *requestp = NULL; + + ref = isc_refcount_decrement(&request->references); + +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, request, requestp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ + + if (request->requestmgr != NULL && + atomic_load_acquire(&request->requestmgr->exiting)) + { + /* We are shutting down and this was last request */ + LOCK(&request->requestmgr->lock); + if (ISC_LIST_EMPTY(request->requestmgr->requests)) { + send_shutdown_events(request->requestmgr); + } + UNLOCK(&request->requestmgr->lock); + } + + if (ref == 1) { + req_destroy(request); + } +} + static void req_destroy(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); req_log(ISC_LOG_DEBUG(3), "req_destroy: request %p", request); + isc_refcount_destroy(&request->references); + request->magic = 0; if (request->query != NULL) { isc_buffer_free(&request->query); @@ -1224,7 +1273,7 @@ req_destroy(dns_request_t *request) { dns_tsigkey_detach(&request->tsigkey); } if (request->requestmgr != NULL) { - requestmgr_detach(&request->requestmgr); + dns_requestmgr_detach(&request->requestmgr); } isc_mem_putanddetach(&request->mctx, request, sizeof(*request)); } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index d140628fb0..2a65090906 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -61,6 +61,11 @@ #include #include #include + +#define RESOLVER_TRACE 1 + +#define WANT_QUERYTRACE 1 + #ifdef WANT_QUERYTRACE #define RTRACE(m) \ isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, \ @@ -222,6 +227,7 @@ typedef struct fetchctx fetchctx_t; typedef struct query { /* Locked by task event serialization. */ unsigned int magic; + isc_refcount_t references; fetchctx_t *fctx; dns_message_t *rmessage; isc_mem_t *mctx; @@ -240,8 +246,6 @@ typedef struct query { int ednsversion; unsigned int options; isc_sockeventattr_t attributes; - unsigned int sends; - unsigned int connects; unsigned int udpsize; unsigned char data[512]; } resquery_t; @@ -543,7 +547,7 @@ struct dns_resolver { /* Locked by lock. */ isc_eventlist_t whenshutdown; - unsigned int activebuckets; + isc_refcount_t activebuckets; unsigned int spillat; /* clients-per-query */ dns_badcache_t *badcache; /* Bad cache. */ @@ -824,7 +828,7 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, static void rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo); -static void +static isc_result_t rctx_next(respctx_t *rctx); static void @@ -1122,19 +1126,27 @@ munge: } static inline void -resquery_destroy(resquery_t **queryp) { +resquery_destroy(resquery_t *query) { dns_resolver_t *res; bool empty; - resquery_t *query; fetchctx_t *fctx; unsigned int bucket; - REQUIRE(queryp != NULL); - query = *queryp; - *queryp = NULL; REQUIRE(!ISC_LINK_LINKED(query, link)); - INSIST(query->dispatch == NULL); + if (query->tsig != NULL) { + isc_buffer_free(&query->tsig); + } + + if (query->tsigkey != NULL) { + dns_tsigkey_detach(&query->tsigkey); + } + + if (query->dispatch != NULL) { + dns_dispatch_detach(&query->dispatch); + } + + isc_refcount_destroy(&query->references); fctx = query->fctx; res = fctx->res; @@ -1157,6 +1169,61 @@ resquery_destroy(resquery_t **queryp) { } } +#define resquery_attach(s, t) \ + __resquery_attach(s, t, __FILE__, __LINE__, __func__) + +static void __attribute__((unused)) +__resquery_attach(resquery_t *source, resquery_t **targetp, const char *file, + unsigned int line, const char *func) { + uint_fast32_t ref; + + REQUIRE(VALID_QUERY(source)); + REQUIRE(targetp != NULL && *targetp == NULL); + + ref = isc_refcount_increment(&source->references); + +#ifdef RESOLVER_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, source, targetp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* RESOLVER_TRACE */ + + *targetp = source; +} + +#define resquery_detach(q) __resquery_detach(q, __FILE__, __LINE__, __func__) + +static void +__resquery_detach(resquery_t **queryp, const char *file, unsigned int line, + const char *func) { + uint_fast32_t ref; + resquery_t *query = NULL; + + REQUIRE(queryp != NULL && VALID_QUERY(*queryp)); + + query = *queryp; + *queryp = NULL; + + ref = isc_refcount_decrement(&query->references); + +#ifdef RESOLVER_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, query, queryp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* RESOLVER_TRACE */ + + if (ref == 1) { + resquery_destroy(query); + } +} + /*% * Update EDNS statistics for a server after not getting a response to a UDP * query sent to it. @@ -1176,23 +1243,36 @@ update_edns_stats(resquery_t *query) { } } +#define fctx_cancelquery(q, d, f, n, a) \ + __fctx_cancelquery(q, d, f, n, a, __FILE__, __LINE__, __func__) + static void -fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, - isc_time_t *finish, bool no_response, bool age_untried) { +__fctx_cancelquery(resquery_t *query, dns_dispatchevent_t **deventp, + isc_time_t *finish, bool no_response, bool age_untried, + const char *file, unsigned int line, const char *func) { fetchctx_t *fctx = NULL; - resquery_t *query = NULL; unsigned int rtt, rttms; unsigned int factor; dns_adbfind_t *find = NULL; dns_adbaddrinfo_t *addrinfo; isc_stdtime_t now; - query = *queryp; fctx = query->fctx; +#ifdef RESOLVER_TRACE + fprintf(stderr, "%s:%s:%u:%s(query = %p, ...)\n", func, file, line, + __func__, query); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* RESOLVER_TRACE */ + FCTXTRACE("cancelquery"); - REQUIRE(!RESQUERY_CANCELED(query)); + if (RESQUERY_CANCELED(query)) { + return; + } query->attributes |= RESQUERY_ATTR_CANCELED; @@ -1352,16 +1432,14 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, } /* - * Check for any outstanding socket events. If they exist, cancel - * them and let the event handlers finish the cleanup. (XXX: - * Currently the resolver, rather than dispatch, tracks whether - * it's sending or connecting; this will be moved into dispatch - * later.) + * Check for any outstanding socket events. If they exist, + * cancel them and let the event handlers finish the cleanup. + * (XXX: Currently the resolver, rather than dispatch, tracks + * whether it's sending or connecting; this will be moved into + * dispatch later.) */ - dns_dispatch_cancel(query->dispatch, query->dispentry, - RESQUERY_SENDING(query), - RESQUERY_CONNECTING(query)); if (query->dispentry != NULL) { + dns_dispatch_cancel(query->dispentry); dns_dispatch_removeresponse(&query->dispentry, deventp); } @@ -1369,24 +1447,8 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, ISC_LIST_UNLINK(fctx->queries, query, link); } - if (query->tsig != NULL) { - isc_buffer_free(&query->tsig); - } - - if (query->tsigkey != NULL) { - dns_tsigkey_detach(&query->tsigkey); - } - - if (query->dispatch != NULL) { - dns_dispatch_detach(&query->dispatch); - } - - if (!(RESQUERY_CONNECTING(query) || RESQUERY_SENDING(query))) { - /* - * It's safe to destroy the query now. - */ - resquery_destroy(&query); - } + /* This is the final detach matching the "init" */ + resquery_detach(&query); } static void @@ -1398,7 +1460,7 @@ fctx_cancelqueries(fetchctx_t *fctx, bool no_response, bool age_untried) { for (query = ISC_LIST_HEAD(fctx->queries); query != NULL; query = next_query) { next_query = ISC_LIST_NEXT(query, link); - fctx_cancelquery(&query, NULL, NULL, no_response, age_untried); + fctx_cancelquery(query, NULL, NULL, no_response, age_untried); } } @@ -1734,61 +1796,53 @@ fctx_done(fetchctx_t *fctx, isc_result_t result, int line) { static void resquery_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { resquery_t *query = (resquery_t *)arg; - bool destroy_query = false; fetchctx_t *fctx = NULL; QTRACE("senddone"); UNUSED(handle); - INSIST(RESQUERY_SENDING(query)); - - query->sends--; - fctx = query->fctx; if (RESQUERY_CANCELED(query)) { - if (query->sends == 0 && query->connects == 0) { - destroy_query = true; - } - } else { - switch (eresult) { - case ISC_R_SUCCESS: - break; - - case ISC_R_HOSTUNREACH: - case ISC_R_NETUNREACH: - case ISC_R_NOPERM: - case ISC_R_ADDRNOTAVAIL: - case ISC_R_CONNREFUSED: - FCTXTRACE3("query canceled in resquery_senddone(): " - "no route to host; no response", - eresult); - - /* - * No route to remote. - */ - add_bad(fctx, query->rmessage, query->addrinfo, eresult, - badns_unreachable); - fctx_cancelquery(&query, NULL, NULL, true, false); - - FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); - fctx_try(fctx, true, false); - break; - - default: - FCTXTRACE3("query canceled in resquery_senddone() " - "due to unexpected result; responding", - eresult); - fctx_cancelquery(&query, NULL, NULL, false, false); - fctx_done(fctx, eresult, __LINE__); - break; - } + goto detach; } - if (destroy_query) { - resquery_destroy(&query); + switch (eresult) { + case ISC_R_SUCCESS: + /* Don't detach from resquery */ + return; + + case ISC_R_HOSTUNREACH: + case ISC_R_NETUNREACH: + case ISC_R_NOPERM: + case ISC_R_ADDRNOTAVAIL: + case ISC_R_CONNREFUSED: + FCTXTRACE3("query canceled in resquery_senddone(): " + "no route to host; no response", + eresult); + + /* + * No route to remote. + */ + add_bad(fctx, query->rmessage, query->addrinfo, eresult, + badns_unreachable); + fctx_cancelquery(query, NULL, NULL, true, false); + + FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); + fctx_try(fctx, true, false); + break; + default: + FCTXTRACE3("query canceled in resquery_senddone() " + "due to unexpected result; responding", + eresult); + fctx_cancelquery(query, NULL, NULL, false, false); + fctx_done(fctx, eresult, __LINE__); + break; } + +detach: + resquery_detach(&query); } static inline isc_result_t @@ -1961,9 +2015,9 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, srtt = addrinfo->srtt; /* - * Allow an additional second for the kernel to resend the SYN (or - * SYN without ECN in the case of stupid firewalls blocking ECN - * negotiation) over the current RTT estimate. + * Allow an additional second for the kernel to resend the SYN + * (or SYN without ECN in the case of stupid firewalls blocking + * ECN negotiation) over the current RTT estimate. */ if ((options & DNS_FETCHOPT_TCP) != 0) { srtt += US_PER_SEC; @@ -1992,9 +2046,11 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, .addrinfo = addrinfo, .dispatchmgr = res->dispatchmgr }; + isc_refcount_init(&query->references, 1); + /* - * Note that the caller MUST guarantee that 'addrinfo' will remain - * valid until this query is canceled. + * Note that the caller MUST guarantee that 'addrinfo' will + * remain valid until this query is canceled. */ dns_message_create(fctx->mctx, DNS_MESSAGE_INTENTPARSE, @@ -2112,9 +2168,9 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, /* * We should always have a valid dispatcher here. If we * don't support a protocol family, then its dispatcher - * will be NULL, but we shouldn't be finding addresses for - * protocol types we don't support, so the dispatcher - * we found should never be NULL. + * will be NULL, but we shouldn't be finding addresses + * for protocol types we don't support, so the + * dispatcher we found should never be NULL. */ INSIST(query->dispatch != NULL); } @@ -2137,6 +2193,9 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, fctx->nqueries++; UNLOCK(&res->buckets[fctx->bucketnum].lock); + resquery_t *tmp = NULL; + resquery_attach(query, &tmp); + /* Set up the dispatch and set the query ID */ result = dns_dispatch_addresponse( query->dispatch, 0, isc_interval_ms(&fctx->interval), @@ -2148,8 +2207,8 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, } /* Connect the socket */ - query->connects++; fctx_increference(fctx); + result = dns_dispatch_connect(query->dispentry); return (result); @@ -2159,11 +2218,10 @@ cleanup_dispatch: } cleanup_query: - if (query->connects == 0) { - query->magic = 0; - dns_message_detach(&query->rmessage); - isc_mem_put(fctx->mctx, query, sizeof(*query)); - } + + query->magic = 0; + dns_message_detach(&query->rmessage); + isc_mem_put(fctx->mctx, query, sizeof(*query)); return (result); } @@ -2271,7 +2329,8 @@ compute_cc(const resquery_t *query, uint8_t *cookie, const size_t len) { INSIST(len >= CLIENT_COOKIE_SIZE); STATIC_ASSERT(sizeof(query->fctx->res->view->secret) >= ISC_SIPHASH24_KEY_LENGTH, - "The view->secret size can't fit SipHash 2-4 key length"); + "The view->secret size can't fit SipHash 2-4 key " + "length"); uint8_t buf[16] ISC_NONSTRING = { 0 }; size_t buflen = add_serveraddr(buf, sizeof(buf), query); @@ -2361,8 +2420,8 @@ resquery_send(resquery_t *query) { qrdataset = NULL; /* - * Set RD if the client has requested that we do a recursive query, - * or if we're sending to a forwarder. + * Set RD if the client has requested that we do a recursive + * query, or if we're sending to a forwarder. */ if ((query->options & DNS_FETCHOPT_RECURSIVE) != 0 || ISFORWARDER(query->addrinfo)) @@ -2426,8 +2485,8 @@ resquery_send(resquery_t *query) { (void)dns_peerlist_peerbyaddr(fctx->res->view->peers, &ipaddr, &peer); /* - * The ADB does not know about servers with "edns no". Check this, - * and then inform the ADB for future use. + * The ADB does not know about servers with "edns no". Check + * this, and then inform the ADB for future use. */ if ((query->addrinfo->flags & DNS_FETCHOPT_NOEDNS0) == 0 && peer != NULL && @@ -2449,12 +2508,13 @@ resquery_send(resquery_t *query) { struct tried *tried; /* - * If this is the first timeout for this server in this fetch - * context, try setting EDNS UDP buffer size to the largest UDP - * response size we have seen from this server so far. + * If this is the first timeout for this server in this + * fetch context, try setting EDNS UDP buffer size to + * the largest UDP response size we have seen from this + * server so far. * - * If this server has already timed out twice or more in this - * fetch context, force TCP. + * If this server has already timed out twice or more in + * this fetch context, force TCP. */ if ((tried = triededns(fctx, sockaddr)) != NULL) { if (tried->count == 1U) { @@ -2468,8 +2528,8 @@ resquery_send(resquery_t *query) { fctx->timeout = false; /* - * Use EDNS0, unless the caller doesn't want it, or we know that the - * remote server doesn't like it. + * Use EDNS0, unless the caller doesn't want it, or we know that + * the remote server doesn't like it. */ if ((query->options & DNS_FETCHOPT_NOEDNS0) == 0) { if ((query->addrinfo->flags & DNS_FETCHOPT_NOEDNS0) == 0) { @@ -2483,26 +2543,26 @@ resquery_send(resquery_t *query) { uint16_t padding = 0; /* - * Set the default UDP size to what was configured as - * 'edns-buffer-size' + * Set the default UDP size to what was + * configured as 'edns-buffer-size' */ udpsize = res->udpsize; /* - * This server timed out for the first time in this - * fetch context and we received a response from it - * before (either in this fetch context or in a - * different one). Set 'udpsize' to the size of the - * largest UDP response we have received from this - * server so far. + * This server timed out for the first time in + * this fetch context and we received a response + * from it before (either in this fetch context + * or in a different one). Set 'udpsize' to the + * size of the largest UDP response we have + * received from this server so far. */ if (hint != 0U) { udpsize = hint; } /* - * If a fixed EDNS UDP buffer size is configured for - * this server, make sure we obey that. + * If a fixed EDNS UDP buffer size is configured + * for this server, make sure we obey that. */ if (peer != NULL) { (void)dns_peer_getudpsize(peer, &peerudpsize); @@ -2516,7 +2576,8 @@ resquery_send(resquery_t *query) { version >>= DNS_FETCHOPT_EDNSVERSIONSHIFT; } - /* Request NSID/COOKIE/VERSION for current peer? */ + /* Request NSID/COOKIE/VERSION for current peer? + */ if (peer != NULL) { uint8_t ednsversion; (void)dns_peer_getrequestnsid(peer, &reqnsid); @@ -2576,7 +2637,8 @@ resquery_send(resquery_t *query) { ednsopt++; } - /* Add PAD for current peer? Require TCP for now */ + /* Add PAD for current peer? Require TCP for now + */ if ((peer != NULL) && tcp) { (void)dns_peer_getpadding(peer, &padding); } @@ -2595,9 +2657,9 @@ resquery_send(resquery_t *query) { query->options |= DNS_FETCHOPT_WANTNSID; } else if (result != ISC_R_SUCCESS) { /* - * We couldn't add the OPT, but we'll press on. - * We're not using EDNS0, so set the NOEDNS0 - * bit. + * We couldn't add the OPT, but we'll + * press on. We're not using EDNS0, so + * set the NOEDNS0 bit. */ query->options |= DNS_FETCHOPT_NOEDNS0; query->ednsversion = -1; @@ -2606,8 +2668,8 @@ resquery_send(resquery_t *query) { } else { /* * We know this server doesn't like EDNS0, so we - * won't use it. Set the NOEDNS0 bit since we're - * not using EDNS0. + * won't use it. Set the NOEDNS0 bit since + * we're not using EDNS0. */ query->options |= DNS_FETCHOPT_NOEDNS0; query->ednsversion = -1; @@ -2622,7 +2684,8 @@ resquery_send(resquery_t *query) { query->udpsize = udpsize; /* - * If we need EDNS0 to do this query and aren't using it, we lose. + * If we need EDNS0 to do this query and aren't using it, we + * lose. */ if (NEEDEDNS0(fctx) && (query->options & DNS_FETCHOPT_NOEDNS0) != 0) { result = DNS_R_SERVFAIL; @@ -2704,7 +2767,6 @@ resquery_send(resquery_t *query) { isc_buffer_usedregion(&buffer, &r); dns_dispatch_send(query->dispentry, &r, query->dscp); - query->sends++; QTRACE("sent"); @@ -2755,6 +2817,7 @@ cleanup_temps: static void resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { resquery_t *query = (resquery_t *)arg; + resquery_t *connquery = query; isc_result_t result; fetchctx_t *fctx = NULL; dns_resolver_t *res = NULL; @@ -2768,7 +2831,6 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { UNUSED(handle); - query->connects--; fctx = query->fctx; res = fctx->res; @@ -2779,78 +2841,77 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { } if (RESQUERY_CANCELED(query)) { + goto detach; + } + + switch (eresult) { + case ISC_R_SUCCESS: /* - * This query was canceled while the connect() was - * in progress. + * We are connected. Update the dispatcher and + * send the query. */ - resquery_destroy(&query); - } else { - switch (eresult) { - case ISC_R_SUCCESS: - /* - * We are connected. Update the dispatcher and - * send the query. - */ - dns_dispatch_changeattributes( - query->dispatch, DNS_DISPATCHATTR_CONNECTED, - DNS_DISPATCHATTR_CONNECTED); + dns_dispatch_changeattributes(query->dispatch, + DNS_DISPATCHATTR_CONNECTED, + DNS_DISPATCHATTR_CONNECTED); - result = resquery_send(query); - if (result != ISC_R_SUCCESS) { - FCTXTRACE("query canceled: " - "resquery_send() failed; responding"); + result = resquery_send(query); + if (result != ISC_R_SUCCESS) { + FCTXTRACE("query canceled: " + "resquery_send() failed; " + "responding"); - fctx_cancelquery(&query, NULL, NULL, false, - false); - fctx_done(fctx, result, __LINE__); - } - - fctx->querysent++; - - ISC_LIST_APPEND(fctx->queries, query, link); - pf = isc_sockaddr_pf(&query->addrinfo->sockaddr); - if (pf == PF_INET) { - inc_stats(res, dns_resstatscounter_queryv4); - } else { - inc_stats(res, dns_resstatscounter_queryv6); - } - if (res->view->resquerystats != NULL) { - dns_rdatatypestats_increment( - res->view->resquerystats, fctx->type); - } - break; - - case ISC_R_NETUNREACH: - case ISC_R_HOSTUNREACH: - case ISC_R_CONNREFUSED: - case ISC_R_NOPERM: - case ISC_R_ADDRNOTAVAIL: - case ISC_R_CONNECTIONRESET: - case ISC_R_TIMEDOUT: - FCTXTRACE3("query canceled in resquery_connected(): " - "no route to host; no response", - eresult); - - /* - * Do not query this server again in this fetch context - * if the server is unavailable over TCP. - */ - add_bad(fctx, query->rmessage, query->addrinfo, eresult, - badns_unreachable); - fctx_cancelquery(&query, NULL, NULL, true, false); - - FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); - fctx_try(fctx, true, false); - break; - - default: - FCTXTRACE3("query canceled in resquery_connected() " - "due to unexpected result; responding", - eresult); - - fctx_cancelquery(&query, NULL, NULL, false, false); - fctx_done(fctx, eresult, __LINE__); + fctx_cancelquery(query, NULL, NULL, false, false); + fctx_done(fctx, result, __LINE__); } + + fctx->querysent++; + + ISC_LIST_APPEND(fctx->queries, query, link); + pf = isc_sockaddr_pf(&query->addrinfo->sockaddr); + if (pf == PF_INET) { + inc_stats(res, dns_resstatscounter_queryv4); + } else { + inc_stats(res, dns_resstatscounter_queryv6); + } + if (res->view->resquerystats != NULL) { + dns_rdatatypestats_increment(res->view->resquerystats, + fctx->type); + } + break; + + case ISC_R_NETUNREACH: + case ISC_R_HOSTUNREACH: + case ISC_R_CONNREFUSED: + case ISC_R_NOPERM: + case ISC_R_ADDRNOTAVAIL: + case ISC_R_CONNECTIONRESET: + case ISC_R_TIMEDOUT: + FCTXTRACE3("query canceled in " + "resquery_connected(): " + "no route to host; no response", + eresult); + + /* + * Do not query this server again in this fetch + * context if the server is unavailable over + * TCP. + */ + add_bad(fctx, query->rmessage, query->addrinfo, eresult, + badns_unreachable); + fctx_cancelquery(query, NULL, NULL, true, false); + + FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); + fctx_try(fctx, true, false); + break; + + default: + FCTXTRACE3("query canceled in " + "resquery_connected() " + "due to unexpected result; responding", + eresult); + + fctx_cancelquery(query, NULL, NULL, false, false); + fctx_done(fctx, eresult, __LINE__); } LOCK(&res->buckets[bucketnum].lock); @@ -2859,6 +2920,9 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { if (bucket_empty) { empty_bucket(res); } + +detach: + resquery_detach(&connquery); } static void @@ -2899,9 +2963,9 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) { fctx->findfail++; if (fctx->pending == 0) { /* - * We've got nothing else to wait for and don't - * know the answer. There's nothing to do but - * fail the fctx. + * We've got nothing else to wait for + * and don't know the answer. There's + * nothing to do but fail the fctx. */ FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); want_done = true; @@ -2923,7 +2987,8 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) { if (want_try) { fctx_try(fctx, true, false); } else if (want_done) { - FCTXTRACE("fetch failed in finddone(); return ISC_R_FAILURE"); + FCTXTRACE("fetch failed in finddone(); return " + "ISC_R_FAILURE"); fctx_done(fctx, ISC_R_FAILURE, __LINE__); } else if (dodestroy) { fctx_destroy(fctx); @@ -3057,8 +3122,8 @@ add_bad(fetchctx_t *fctx, dns_message_t *rmessage, dns_adbaddrinfo_t *addrinfo, break; /* counted as 'valfail' */ case badns_forwarder: /* - * We were called to prevent the given forwarder from - * being used again for this fetch context. + * We were called to prevent the given forwarder + * from being used again for this fetch context. */ break; } @@ -3304,9 +3369,11 @@ findname(fetchctx_t *fctx, const dns_name_t *name, in_port_t port, fctx->quotacount++; /* quota exceeded */ } else if ((find->options & DNS_ADBFIND_LAMEPRUNED) != 0) { - fctx->lamecount++; /* cached lame server */ + fctx->lamecount++; /* cached lame server + */ } else { - fctx->adberr++; /* unreachable server, etc. */ + fctx->adberr++; /* unreachable server, + etc. */ } /* @@ -3384,8 +3451,8 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) { /* * If we have DNS_FETCHOPT_NOFORWARD set and forwarding policy * allows us to not forward - skip forwarders and go straight - * to NSes. This is currently used to make sure that priming query - * gets root servers' IP addresses in ADDITIONAL section. + * to NSes. This is currently used to make sure that priming + * query gets root servers' IP addresses in ADDITIONAL section. */ if ((fctx->options & DNS_FETCHOPT_NOFORWARD) != 0 && (fctx->fwdpolicy != dns_fwdpolicy_only)) @@ -3468,8 +3535,8 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) { } /* - * If the forwarding policy is "only", we don't need the addresses - * of the nameservers. + * If the forwarding policy is "only", we don't need the + * addresses of the nameservers. */ if (fctx->fwdpolicy == dns_fwdpolicy_only) { goto out; @@ -3489,8 +3556,9 @@ normal_nses: * can use. * * We don't want to set this option all the time, since - * if fctx->restarts > 1, we've clearly been having trouble - * with the addresses we had, so getting more could help. + * if fctx->restarts > 1, we've clearly been having + * trouble with the addresses we had, so getting more + * could help. */ stdoptions |= DNS_ADBFIND_AVOIDFETCHES; } @@ -3598,8 +3666,9 @@ out: */ if (fctx->pending > 0) { /* - * We're fetching the addresses, but don't have any - * yet. Tell the caller to wait for an answer. + * We're fetching the addresses, but don't have + * any yet. Tell the caller to wait for an + * answer. */ result = DNS_R_WAIT; } else { @@ -3607,8 +3676,8 @@ out: isc_interval_t i; /* * We've lost completely. We don't know any - * addresses, and the ADB has told us it can't get - * them. + * addresses, and the ADB has told us it can't + * get them. */ FCTXTRACE("no addresses"); isc_interval_set(&i, DNS_RESOLVER_BADCACHETTL(fctx), 0); @@ -3626,8 +3695,8 @@ out: /* * If all of the addresses found were over the - * fetches-per-server quota, return the configured - * response. + * fetches-per-server quota, return the + * configured response. */ if (all_spilled) { result = res->quotaresp[dns_quotatype_server]; @@ -3636,8 +3705,8 @@ out: } } else { /* - * We've found some addresses. We might still be looking - * for more addresses. + * We've found some addresses. We might still be + * looking for more addresses. */ sort_finds(&fctx->finds, res->view->v6bias); sort_finds(&fctx->altfinds, 0); @@ -3958,9 +4027,9 @@ fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) { if (fctx->minimized && !fctx->forwarding) { unsigned int options = fctx->options; /* - * Also clear DNS_FETCHOPT_TRYSTALE_ONTIMEOUT here, otherwise - * every query minimization step will activate the try-stale - * timer again. + * Also clear DNS_FETCHOPT_TRYSTALE_ONTIMEOUT here, + * otherwise every query minimization step will activate + * the try-stale timer again. */ options &= ~(DNS_FETCHOPT_QMINIMIZE | DNS_FETCHOPT_TRYSTALE_ONTIMEOUT); @@ -4119,7 +4188,8 @@ resume_qmin(isc_task_t *task, isc_event_t *event) { fctx->qmin_labels = DNS_MAX_LABELS + 1; /* * We store the result. If we succeed in the end - * we'll issue a warning that the server is broken. + * we'll issue a warning that the server is + * broken. */ fctx->qmin_warning = result; } else { @@ -4140,9 +4210,9 @@ resume_qmin(isc_task_t *task, isc_event_t *event) { &fctx->nameservers, NULL); /* - * DNS_R_NXDOMAIN here means we have not loaded the root zone mirror - * yet - but DNS_R_NXDOMAIN is not a valid return value when doing - * recursion, we need to patch it. + * DNS_R_NXDOMAIN here means we have not loaded the root zone + * mirror yet - but DNS_R_NXDOMAIN is not a valid return value + * when doing recursion, we need to patch it. */ if (result == DNS_R_NXDOMAIN) { result = DNS_R_SERVFAIL; @@ -4174,9 +4244,10 @@ resume_qmin(isc_task_t *task, isc_event_t *event) { if (!fctx->minimized) { /* - * We have finished minimizing, but fctx->finds was filled at - * the beginning of the run - now we need to clear it before - * sending the final query to use proper nameservers. + * We have finished minimizing, but fctx->finds was + * filled at the beginning of the run - now we need to + * clear it before sending the final query to use proper + * nameservers. */ fctx_cancelqueries(fctx, false, false); fctx_cleanupall(fctx); @@ -4299,7 +4370,8 @@ fctx_shutdown(fetchctx_t *fctx) { isc_event_t *cevent; /* - * Start the shutdown process for fctx, if it isn't already underway. + * Start the shutdown process for fctx, if it isn't already + * underway. */ FCTXTRACE("shutdown"); @@ -4352,7 +4424,8 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) { /* * Cancel all pending validators. Note that this must be done - * without the bucket lock held, since that could cause deadlock. + * without the bucket lock held, since that could cause + * deadlock. */ validator = ISC_LIST_HEAD(fctx->validators); while (validator != NULL) { @@ -4429,8 +4502,8 @@ fctx_start(isc_task_t *task, isc_event_t *event) { INSIST(fctx->state == fetchstate_init); if (fctx->want_shutdown) { /* - * We haven't started this fctx yet, and we've been requested - * to shut it down. + * We haven't started this fctx yet, and we've been + * requested to shut it down. */ FCTX_ATTR_SET(fctx, FCTX_ATTR_SHUTTINGDOWN); fctx->state = fetchstate_done; @@ -4456,8 +4529,8 @@ fctx_start(isc_task_t *task, isc_event_t *event) { */ fctx->state = fetchstate_active; /* - * Reset the control event for later use in shutting down - * the fctx. + * Reset the control event for later use in shutting + * down the fctx. */ ISC_EVENT_INIT(event, sizeof(*event), 0, NULL, DNS_EVENT_FETCHCONTROL, fctx_doshutdown, fctx, @@ -4491,8 +4564,8 @@ fctx_add_event(fetchctx_t *fctx, isc_task_t *task, const isc_sockaddr_t *client, /* * We store the task we're going to send this event to in the - * sender field. We'll make the fetch the sender when we actually - * send the event. + * sender field. We'll make the fetch the sender when we + * actually send the event. */ isc_task_attach(task, &tclone); event = (dns_fetchevent_t *)isc_event_allocate(fctx->res->mctx, tclone, @@ -4510,8 +4583,8 @@ fctx_add_event(fetchctx_t *fctx, isc_task_t *task, const isc_sockaddr_t *client, event->foundname = dns_fixedname_initname(&event->fname); /* - * Store the sigrdataset in the first event in case it is needed by - * any of the events. + * Store the sigrdataset in the first event in case it is needed + * by any of the events. */ if (event->sigrdataset != NULL) { ISC_LIST_PREPEND(fctx->events, event, ev_link); @@ -4567,7 +4640,8 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, size_t p; /* - * Caller must be holding the lock for bucket number 'bucketnum'. + * Caller must be holding the lock for bucket number + * 'bucketnum'. */ REQUIRE(fctxp != NULL && *fctxp == NULL); @@ -4682,8 +4756,8 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, /* * The caller didn't supply a query domain and - * nameservers, and we're not in forward-only mode, - * so find the best nameservers to use. + * nameservers, and we're not in forward-only + * mode, so find the best nameservers to use. */ if (dns_rdatatype_atparent(fctx->type)) { findoptions |= DNS_DBFIND_NOEXACT; @@ -4702,7 +4776,8 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, fctx->ns_ttl_ok = true; } else { /* - * We're in forward-only mode. Set the query domain. + * We're in forward-only mode. Set the query + * domain. */ dns_name_copy(fname, fctx->domain); dns_name_copy(fname, fctx->qmindcname); @@ -4757,15 +4832,15 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, } /* - * Default retry interval initialization. We set the interval now - * mostly so it won't be uninitialized. It will be set to the - * correct value before a query is issued. + * Default retry interval initialization. We set the interval + * now mostly so it won't be uninitialized. It will be set to + * the correct value before a query is issued. */ isc_interval_set(&fctx->interval, 2, 0); /* - * If stale answers are enabled, compute an expiration time after - * which stale data will be served, if the target RRset is + * If stale answers are enabled, compute an expiration time + * after which stale data will be served, if the target RRset is * available in cache. */ if ((options & DNS_FETCHOPT_TRYSTALE_ONTIMEOUT) != 0) { @@ -5023,7 +5098,8 @@ clone_results(fetchctx_t *fctx) { for (event = ISC_LIST_HEAD(fctx->events); event != NULL; event = ISC_LIST_NEXT(event, ev_link)) { - /* This is the the head event; keep a pointer and move on */ + /* This is the the head event; keep a pointer and move + * on */ if (hevent == NULL) { hevent = ISC_LIST_HEAD(fctx->events); continue; @@ -5032,17 +5108,18 @@ clone_results(fetchctx_t *fctx) { if (event->ev_type == DNS_EVENT_TRYSTALE) { /* * We don't need to clone resulting data to this - * type of event, as its associated callback is only - * called when stale-answer-client-timeout triggers, - * and the logic in there doesn't expect any result - * as input, as it will itself lookup for stale data - * in cache to use as result, if any is available. + * type of event, as its associated callback is + * only called when stale-answer-client-timeout + * triggers, and the logic in there doesn't + * expect any result as input, as it will itself + * lookup for stale data in cache to use as + * result, if any is available. * - * Also, if we reached this point, then the whole fetch - * context is done, it will cancel timers, process - * associated callbacks of type DNS_EVENT_FETCHDONE, and - * silently remove/free events of type - * DNS_EVENT_TRYSTALE. + * Also, if we reached this point, then the + * whole fetch context is done, it will cancel + * timers, process associated callbacks of type + * DNS_EVENT_FETCHDONE, and silently remove/free + * events of type DNS_EVENT_TRYSTALE. */ continue; } @@ -5086,7 +5163,8 @@ clone_results(fetchctx_t *fctx) { * '*fctx' is shutting down. * * Returns: - * true if the resolver is exiting and this is the last fctx in the bucket. + * true if the resolver is exiting and this is the last fctx in the + *bucket. */ static bool maybe_destroy(fetchctx_t *fctx, bool locked) { @@ -5205,8 +5283,8 @@ validated(isc_task_t *task, isc_event_t *event) { /* * If shutting down, ignore the results. Check to see if we're - * done waiting for validator completions and ADB pending events; if - * so, destroy the fctx. + * done waiting for validator completions and ADB pending + * events; if so, destroy the fctx. */ if (SHUTTINGDOWN(fctx) && !sentresponse) { bool bucket_empty; @@ -5221,8 +5299,8 @@ validated(isc_task_t *task, isc_event_t *event) { isc_stdtime_get(&now); /* - * If chaining, we need to make sure that the right result code is - * returned, and that the rdatasets are bound. + * If chaining, we need to make sure that the right result code + * is returned, and that the rdatasets are bound. */ if (vevent->result == ISC_R_SUCCESS && !negative && vevent->rdataset != NULL && CHAINING(vevent->rdataset)) @@ -5239,9 +5317,9 @@ validated(isc_task_t *task, isc_event_t *event) { } /* - * Either we're not shutting down, or we are shutting down but want - * to cache the result anyway (if this was a validation started by - * a query with cd set) + * Either we're not shutting down, or we are shutting down but + * want to cache the result anyway (if this was a validation + * started by a query with cd set) */ hevent = ISC_LIST_HEAD(fctx->events); @@ -5289,7 +5367,8 @@ validated(isc_task_t *task, isc_event_t *event) { } if (fctx->vresult == DNS_R_BROKENCHAIN && !negative) { /* - * Cache the data as pending for later validation. + * Cache the data as pending for later + * validation. */ result = ISC_R_NOTFOUND; if (vevent->rdataset != NULL) { @@ -5320,7 +5399,8 @@ validated(isc_task_t *task, isc_event_t *event) { if (fctx->validator != NULL) { dns_validator_send(fctx->validator); } else if (sentresponse) { - fctx_done(fctx, result, __LINE__); /* Locks bucket. */ + fctx_done(fctx, result, __LINE__); /* Locks + bucket. */ } else if (result == DNS_R_BROKENCHAIN) { isc_result_t tresult; isc_time_t expire; @@ -5336,7 +5416,8 @@ validated(isc_task_t *task, isc_event_t *event) { dns_resolver_addbadcache(res, fctx->name, fctx->type, &expire); } - fctx_done(fctx, result, __LINE__); /* Locks bucket. */ + fctx_done(fctx, result, __LINE__); /* Locks + bucket. */ } else { fctx_try(fctx, true, true); /* Locks bucket. */ } @@ -5458,8 +5539,8 @@ validated(isc_task_t *task, isc_event_t *event) { if (sentresponse) { bool bucket_empty = false; /* - * If we only deferred the destroy because we wanted to cache - * the data, destroy now. + * If we only deferred the destroy because we wanted to + * cache the data, destroy now. */ dns_db_detachnode(fctx->cache, &node); if (SHUTTINGDOWN(fctx)) { @@ -5823,11 +5904,12 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, anodep = &event->node; /* - * If this is an ANY, SIG or RRSIG query, we're not - * going to return any rdatasets, unless we encountered - * a CNAME or DNAME as "the answer". In this case, - * we're going to return DNS_R_CNAME or DNS_R_DNAME - * and we must set up the rdatasets. + * If this is an ANY, SIG or RRSIG query, we're + * not going to return any rdatasets, unless we + * encountered a CNAME or DNAME as "the answer". + * In this case, we're going to return + * DNS_R_CNAME or DNS_R_DNAME and we must set up + * the rdatasets. */ if ((fctx->type != dns_rdatatype_any && fctx->type != dns_rdatatype_rrsig && @@ -5920,21 +6002,21 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * If this RRset is in a secure domain, is in bailiwick, - * and is not glue, attempt DNSSEC validation. (We do not - * attempt to validate glue or out-of-bailiwick data--even - * though there might be some performance benefit to doing - * so--because it makes it simpler and safer to ensure that - * records from a secure domain are only cached if validated - * within the context of a query to the domain that owns - * them.) + * and is not glue, attempt DNSSEC validation. (We do + * not attempt to validate glue or out-of-bailiwick + * data--even though there might be some performance + * benefit to doing so--because it makes it simpler and + * safer to ensure that records from a secure domain are + * only cached if validated within the context of a + * query to the domain that owns them.) */ if (secure_domain && rdataset->trust != dns_trust_glue && !EXTERNAL(rdataset)) { dns_trust_t trust; /* - * RRSIGs are validated as part of validating the - * type they cover. + * RRSIGs are validated as part of validating + * the type they cover. */ if (rdataset->type == dns_rdatatype_rrsig) { continue; @@ -5944,7 +6026,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, !ANSWER(rdataset)) { /* * Ignore unrelated non-answer - * rdatasets that are missing signatures. + * rdatasets that are missing + * signatures. */ continue; } @@ -5969,9 +6052,9 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * Cache this rdataset/sigrdataset pair as - * pending data. Track whether it was additional - * or not. If this was a priming query, additional - * should be cached as glue. + * pending data. Track whether it was + * additional or not. If this was a priming + * query, additional should be cached as glue. */ if (rdataset->trust == dns_trust_additional) { trust = dns_trust_pending_additional; @@ -6016,11 +6099,13 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, ardataset != NULL && NEGATIVE(ardataset)) { /* - * The answer in the cache is - * better than the answer we - * found, and is a negative - * cache entry, so we must set - * eresult appropriately. + * The answer in the + * cache is better than + * the answer we found, + * and is a negative + * cache entry, so we + * must set eresult + * appropriately. */ if (NXDOMAIN(ardataset)) { eresult = @@ -6030,10 +6115,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, DNS_R_NCACHENXRRSET; } /* - * We have a negative response - * from the cache so don't - * attempt to add the RRSIG - * rrset. + * We have a negative + * response from the + * cache so don't + * attempt to add the + * RRSIG rrset. */ continue; } @@ -6065,9 +6151,10 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, { /* * This is The Answer. We will - * validate it, but first we cache - * the rest of the response - it may - * contain useful keys. + * validate it, but first we + * cache the rest of the + * response - it may contain + * useful keys. */ INSIST(valrdataset == NULL && valsigrdataset == NULL); @@ -6124,10 +6211,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, rdataset->covers == dns_rdatatype_ns))) { /* - * If the trust level is 'dns_trust_glue' - * then we are adding data from a referral - * we got while executing the search algorithm. - * New referral data always takes precedence + * If the trust level is + * 'dns_trust_glue' then we are adding + * data from a referral we got while + * executing the search algorithm. New + * referral data always takes precedence * over the existing cache contents. */ options = DNS_DBADD_FORCE; @@ -6162,10 +6250,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, if (ANSWER(rdataset) && ardataset != NULL && NEGATIVE(ardataset)) { /* - * The answer in the cache is better - * than the answer we found, and is - * a negative cache entry, so we - * must set eresult appropriately. + * The answer in the cache is + * better than the answer we + * found, and is a negative + * cache entry, so we must set + * eresult appropriately. */ if (NXDOMAIN(ardataset)) { eresult = DNS_R_NCACHENXDOMAIN; @@ -6199,7 +6288,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); if (event != NULL) { /* - * Negative results must be indicated in event->result. + * Negative results must be indicated in + * event->result. */ if (dns_rdataset_isassociated(event->rdataset) && NEGATIVE(event->rdataset)) { @@ -6268,7 +6358,8 @@ cache_message(fetchctx_t *fctx, dns_message_t *message, } /* - * Do what dns_ncache_addoptout() does, and then compute an appropriate eresult. + * Do what dns_ncache_addoptout() does, and then compute an appropriate + * eresult. */ static isc_result_t ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, @@ -6308,10 +6399,10 @@ ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, } else { /* * Either we don't care about the nature of the - * cache rdataset (because no fetch is interested - * in the outcome), or the cache rdataset is not - * a negative cache entry. Whichever case it is, - * we can return success. + * cache rdataset (because no fetch is + * interested in the outcome), or the cache + * rdataset is not a negative cache entry. + * Whichever case it is, we can return success. * * XXXRTH There's a CNAME/DNAME problem here. */ @@ -6413,9 +6504,9 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, NULL, NULL, valoptions, res->buckets[fctx->bucketnum].task); /* - * If validation is necessary, return now. Otherwise continue - * to process the message, letting the validation complete - * in its own good time. + * If validation is necessary, return now. Otherwise + * continue to process the message, letting the + * validation complete in its own good time. */ return (result); } @@ -6618,8 +6709,8 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, } /* - * If the owner name matches one in the exclusion list, either exactly - * or partially, allow it. + * If the owner name matches one in the exclusion list, either + * exactly or partially, allow it. */ if (view->answeracl_exclude != NULL) { dns_rbtnode_t *node = NULL; @@ -6633,9 +6724,9 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, } /* - * Otherwise, search the filter list for a match for each address - * record. If a match is found, the address should be filtered, - * so should the entire answer. + * Otherwise, search the filter list for a match for each + * address record. If a match is found, the address should be + * filtered, so should the entire answer. */ for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; result = dns_rdataset_next(rdataset)) @@ -6748,8 +6839,8 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, } /* - * If the owner name matches one in the exclusion list, either exactly - * or partially, allow it. + * If the owner name matches one in the exclusion list, either + * exactly or partially, allow it. */ if (view->answernames_exclude != NULL) { result = dns_rbt_findnode(view->answernames_exclude, qname, @@ -6760,12 +6851,13 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, } /* - * If the target name is a subdomain of the search domain, allow it. + * If the target name is a subdomain of the search domain, allow + * it. * - * Note that if BIND is configured as a forwarding DNS server, the - * search domain will always match the root domain ("."), so we - * must also check whether forwarding is enabled so that filters - * can be applied; see GL #1574. + * Note that if BIND is configured as a forwarding DNS server, + * the search domain will always match the root domain ("."), so + * we must also check whether forwarding is enabled so that + * filters can be applied; see GL #1574. */ if (!fctx->forwarding && dns_name_issubdomain(tname, fctx->domain)) { return (true); @@ -6956,7 +7048,8 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) { dns_rdataset_t *nsrdataset = NULL; /* - * Retrieve state from fctx->nsfetch before we destroy it. + * Retrieve state from fctx->nsfetch before we destroy + * it. */ domain = dns_fixedname_initname(&fixed); dns_name_copy(fctx->nsfetch->private->domain, domain); @@ -7283,9 +7376,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { /* * Is the question the same as the one we asked? - * NOERROR/NXDOMAIN/YXDOMAIN/REFUSED/SERVFAIL/BADCOOKIE must have - * the same question. - * FORMERR/NOTIMP if they have a question section then it must match. + * NOERROR/NXDOMAIN/YXDOMAIN/REFUSED/SERVFAIL/BADCOOKIE must + * have the same question. FORMERR/NOTIMP if they have a + * question section then it must match. */ switch (query->rmessage->rcode) { case dns_rcode_notimp: @@ -7328,21 +7421,15 @@ resquery_response(isc_task_t *task, isc_event_t *event) { } /* - * The dispatcher should ensure we only get responses with QR set. + * The dispatcher should ensure we only get responses with QR + * set. */ INSIST((query->rmessage->flags & DNS_MESSAGEFLAG_QR) != 0); - /* - * INSIST() that the message comes from the place we sent it to, - * since the dispatch code should ensure this. - * - * INSIST() that the message id is correct (this should also be - * ensured by the dispatch code). - */ /* - * If we have had a server cookie and don't get one retry over TCP. - * This may be a misconfigured anycast server or an attempt to send - * a spoofed response. Skip if we have a valid tsig. + * If we have had a server cookie and don't get one retry over + * TCP. This may be a misconfigured anycast server or an attempt + * to send a spoofed response. Skip if we have a valid tsig. */ if (dns_message_gettsig(query->rmessage, NULL) == NULL && !query->rmessage->cc_ok && !query->rmessage->cc_bad && @@ -7359,7 +7446,8 @@ resquery_response(isc_task_t *task, isc_event_t *event) { isc_log_write( dns_lctx, DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, - "missing expected cookie from %s", + "missing expected cookie " + "from %s", addrbuf); } rctx.retryopts |= DNS_FETCHOPT_TCP; @@ -7367,10 +7455,6 @@ resquery_response(isc_task_t *task, isc_event_t *event) { rctx_done(&rctx, result); return; } - /* - * XXXMPA When support for DNS COOKIE becomes ubiquitous, fall - * back to TCP for all non-COOKIE responses. - */ } rctx_edns(&rctx); @@ -7459,8 +7543,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { rctx_delonly_zone(&rctx); /* - * Optionally call dns_rdata_checkowner() and dns_rdata_checknames() - * to validate the names in the response message. + * Optionally call dns_rdata_checkowner() and + * dns_rdata_checknames() to validate the names in the response + * message. */ if ((fctx->res->options & DNS_RESOLVER_CHECKNAMES) != 0) { checknames(query->rmessage); @@ -7497,7 +7582,8 @@ resquery_response(isc_task_t *task, isc_event_t *event) { case DNS_R_CHASEDSSERVERS: break; case DNS_R_DELEGATION: - /* With NOFOLLOW we want to pass the result code */ + /* With NOFOLLOW we want to pass the result code + */ if ((fctx->options & DNS_FETCHOPT_NOFOLLOW) == 0) { result = ISC_R_SUCCESS; } @@ -7531,8 +7617,8 @@ resquery_response(isc_task_t *task, isc_event_t *event) { rctx_additional(&rctx); /* - * Cache the cacheable parts of the message. This may also cause - * work to be queued to the DNSSEC validator. + * Cache the cacheable parts of the message. This may also + * cause work to be queued to the DNSSEC validator. */ if (WANTCACHE(fctx)) { isc_result_t tresult; @@ -7556,8 +7642,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { /* * rctx_respinit(): - * Initialize the response context structure 'rctx' to all zeroes, then set - * the task, event, query and fctx information from resquery_response(). + * Initialize the response context structure 'rctx' to all zeroes, then + * set the task, event, query and fctx information from + * resquery_response(). */ static void rctx_respinit(isc_task_t *task, dns_dispatchevent_t *devent, resquery_t *query, @@ -7579,8 +7666,8 @@ rctx_respinit(isc_task_t *task, dns_dispatchevent_t *devent, resquery_t *query, * rctx_answer_init(): * Clear and reinitialize those portions of 'rctx' that will be needed * when scanning the answer section of the response message. This can be - * called more than once if scanning needs to be restarted (though currently - * there are no cases in which this occurs). + * called more than once if scanning needs to be restarted (though + * currently there are no cases in which this occurs). */ static void rctx_answer_init(respctx_t *rctx) { @@ -7646,8 +7733,8 @@ rctx_dispfail(respctx_t *rctx) { (rctx->retryopts & DNS_FETCHOPT_NOEDNS0) == 0) { /* - * The problem might be that they don't understand EDNS0. - * Turn it off and try again. + * The problem might be that they don't understand + * EDNS0. Turn it off and try again. */ rctx->retryopts |= DNS_FETCHOPT_NOEDNS0; rctx->resend = true; @@ -7659,9 +7746,9 @@ rctx_dispfail(respctx_t *rctx) { rctx->next_server = true; /* * If this is a network error, mark the server as bad so - * that we won't try it for this fetch again. Also adjust - * finish and no_response so that we penalize this address - * in SRTT adjustment later. + * that we won't try it for this fetch again. Also + * adjust finish and no_response so that we penalize + * this address in SRTT adjustment later. */ if (devent->result == ISC_R_HOSTUNREACH || devent->result == ISC_R_NETUNREACH || @@ -7920,15 +8007,16 @@ rctx_edns(respctx_t *rctx) { (rctx->retryopts & DNS_FETCHOPT_NOEDNS0) == 0) { /* - * We didn't get a OPT record in response to a EDNS query. + * We didn't get a OPT record in response to a EDNS + * query. * * Old versions of named incorrectly drop the OPT record - * when there is a signed, truncated response so we check - * that TC is not set. + * when there is a signed, truncated response so we + * check that TC is not set. * - * Record that the server is not talking EDNS. While this - * should be safe to do for any rcode we limit it to NOERROR - * and NXDOMAIN. + * Record that the server is not talking EDNS. While + * this should be safe to do for any rcode we limit it + * to NOERROR and NXDOMAIN. */ dns_message_logpacket( query->rmessage, "received packet (no opt) from", @@ -8122,8 +8210,8 @@ rctx_answer_positive(respctx_t *rctx) { } /* - * We didn't end with an incomplete chain, so the rcode should be - * "no error". + * We didn't end with an incomplete chain, so the rcode should + * be "no error". */ if (rctx->query->rmessage->rcode != dns_rcode_noerror) { log_formerr(fctx, "CNAME/DNAME chain complete, but RCODE " @@ -8152,8 +8240,8 @@ rctx_answer_positive(respctx_t *rctx) { /* * rctx_answer_scan(): * Perform a single pass over the answer section of a response, looking - * for an answer that matches QNAME/QTYPE, or a CNAME matching QNAME, or a - * covering DNAME. If more than one rdataset is found matching these + * for an answer that matches QNAME/QTYPE, or a CNAME matching QNAME, or + * a covering DNAME. If more than one rdataset is found matching these * criteria, then only one is kept. Order of preference is 1) the * shortest DNAME, 2) the first matching answer, or 3) the first CNAME. */ @@ -8213,8 +8301,9 @@ rctx_answer_scan(respctx_t *rctx) { } /* - * We are looking for the shortest DNAME if there - * are multiple ones (which there shouldn't be). + * We are looking for the shortest DNAME if + * there are multiple ones (which there + * shouldn't be). */ for (rdataset = ISC_LIST_HEAD(name->list); rdataset != NULL; @@ -8236,9 +8325,9 @@ rctx_answer_scan(respctx_t *rctx) { /* * If a DNAME was found, then any CNAME or other answer matching - * QNAME that may also have been found must be ignored. Similarly, - * if a matching answer was found along with a CNAME, the CNAME - * must be ignored. + * QNAME that may also have been found must be ignored. + * Similarly, if a matching answer was found along with a CNAME, + * the CNAME must be ignored. */ if (rctx->dname != NULL) { rctx->aname = NULL; @@ -8497,9 +8586,9 @@ rctx_answer_dname(respctx_t *rctx) { /* * rctx_authority_positive(): * Examine the records in the authority section (if there are any) for a - * positive answer. We expect the names for all rdatasets in this section - * to be subdomains of the domain being queried; any that are not are - * skipped. We expect to find only *one* owner name; any names + * positive answer. We expect the names for all rdatasets in this + * section to be subdomains of the domain being queried; any that are + * not are skipped. We expect to find only *one* owner name; any names * after the first one processed are ignored. We expect to find only * rdatasets of type NS, RRSIG, or SIG; all others are ignored. Whatever * remains can be cached at trust level authauthority or additional @@ -8554,8 +8643,8 @@ rctx_authority_positive(respctx_t *rctx) { rctx->ns_rdataset = rdataset; } /* - * Mark any additional data related - * to this rdataset. + * Mark any additional data + * related to this rdataset. */ (void)dns_rdataset_additionaldata( rdataset, name, check_related, @@ -8587,8 +8676,8 @@ rctx_answer_none(respctx_t *rctx) { rctx_answer_init(rctx); /* - * Sometimes we can tell if its a negative response by looking at - * the message header. + * Sometimes we can tell if its a negative response by looking + * at the message header. */ if (rctx->query->rmessage->rcode == dns_rcode_nxdomain || (rctx->query->rmessage->counts[DNS_SECTION_ANSWER] == 0 && @@ -8908,7 +8997,8 @@ rctx_authority_dnssec(respctx_t *rctx) { } if (!dns_name_issubdomain(name, fctx->domain)) { - /* Invalid name found; preserve it for logging later */ + /* Invalid name found; preserve it for logging + * later */ rctx->found_name = name; rctx->found_type = ISC_LIST_HEAD(name->list)->type; continue; @@ -8947,16 +9037,17 @@ rctx_authority_dnssec(respctx_t *rctx) { rdataset->trust = dns_trust_additional; } /* - * No additional data needs to be marked. + * No additional data needs to be + * marked. */ break; case dns_rdatatype_ds: /* * DS or SIG DS. * - * These should only be here if this is a - * referral, and there should only be one - * DS RRset. + * These should only be here if this is + * a referral, and there should only be + * one DS RRset. */ if (rctx->ns_name == NULL) { log_formerr(fctx, "DS with no " @@ -9127,8 +9218,8 @@ rctx_referral(respctx_t *rctx) { /* * Reinitialize 'rctx' to prepare for following the delegation: - * set the get_nameservers and next_server flags appropriately and - * reset the fetch context counters. + * set the get_nameservers and next_server flags appropriately + * and reset the fetch context counters. * */ if ((rctx->fctx->options & DNS_FETCHOPT_NOFOLLOW) == 0) { @@ -9241,7 +9332,8 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, } if (!dns_name_issubdomain(fname, fctx->domain)) { /* - * The best nameservers are now above our QDOMAIN. + * The best nameservers are now above our + * QDOMAIN. */ FCTXTRACE("nameservers now above QDOMAIN"); fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); @@ -9273,9 +9365,10 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, /* * rctx_resend(): * - * Resend the query, probably with the options changed. Calls fctx_query(), - * passing rctx->retryopts (which is based on query->options, but may have been - * updated since the last time fctx_query() was called). + * Resend the query, probably with the options changed. Calls + * fctx_query(), passing rctx->retryopts (which is based on + * query->options, but may have been updated since the last time + * fctx_query() was called). */ static void rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) { @@ -9305,12 +9398,12 @@ rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) { /* * rctx_next(): - * We got what appeared to be a response but it didn't match the question - * or the cookie; it may have been meant for someone else, or it may be a - * spoofing attack. Drop it and continue listening for the response we - * wanted. + * We got what appeared to be a response but it didn't match the + * question or the cookie; it may have been meant for someone else, or + * it may be a spoofing attack. Drop it and continue listening for the + * response we wanted. */ -static void +static isc_result_t rctx_next(respctx_t *rctx) { #ifdef WANT_QUERYTRACE fetchctx_t *fctx = rctx->fctx; @@ -9325,11 +9418,14 @@ rctx_next(respctx_t *rctx) { if (result != ISC_R_SUCCESS) { fctx_done(rctx->fctx, result, __LINE__); } + + return (result); } /* * rctx_chaseds(): - * Look up the parent zone's NS records so that DS records can be fetched. + * Look up the parent zone's NS records so that DS records can be + * fetched. */ static void rctx_chaseds(respctx_t *rctx, dns_message_t *message, @@ -9363,10 +9459,11 @@ rctx_chaseds(respctx_t *rctx, dns_message_t *message, /* * rctx_done(): - * This resolver query response is finished, either because we encountered - * a problem or because we've gotten all the information from it that we - * can. We either wait for another response, resend the query to the - * same server, resend to a new server, or clean up and shut down the fetch. + * This resolver query response is finished, either because we + * encountered a problem or because we've gotten all the information + * from it that we can. We either wait for another response, resend the + * query to the same server, resend to a new server, or clean up and + * shut down the fetch. */ static void rctx_done(respctx_t *rctx, isc_result_t result) { @@ -9390,7 +9487,7 @@ rctx_done(respctx_t *rctx, isc_result_t result) { * Cancel the query. */ if (!rctx->nextitem) { - fctx_cancelquery(&query, &rctx->devent, rctx->finish, + fctx_cancelquery(query, &rctx->devent, rctx->finish, rctx->no_response, false); } @@ -9399,7 +9496,7 @@ rctx_done(respctx_t *rctx, isc_result_t result) { (rctx->next_server || rctx->resend || rctx->nextitem)) { if (rctx->nextitem) { - fctx_cancelquery(&query, &rctx->devent, rctx->finish, + fctx_cancelquery(query, &rctx->devent, rctx->finish, rctx->no_response, false); } fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); @@ -9412,7 +9509,9 @@ rctx_done(respctx_t *rctx, isc_result_t result) { } else if (rctx->resend) { rctx_resend(rctx, addrinfo); } else if (rctx->nextitem) { - rctx_next(rctx); + if (rctx_next(rctx) != ISC_R_SUCCESS) { + resquery_detach(&query); + } } else if (result == DNS_R_CHASEDSSERVERS) { rctx_chaseds(rctx, message, addrinfo, result); } else if (result == ISC_R_SUCCESS && !HAVE_ANSWER(fctx)) { @@ -9759,15 +9858,11 @@ static void empty_bucket(dns_resolver_t *res) { RTRACE("empty_bucket"); - LOCK(&res->lock); - - INSIST(res->activebuckets > 0); - res->activebuckets--; - if (res->activebuckets == 0) { + if (isc_refcount_decrement(&res->activebuckets) == 1) { + LOCK(&res->lock); send_shutdown_events(res); + UNLOCK(&res->lock); } - - UNLOCK(&res->lock); } static void @@ -9847,10 +9942,11 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr, .maxdepth = DEFAULT_RECURSION_DEPTH, .maxqueries = DEFAULT_MAX_QUERIES, .nbuckets = ntasks, - .activebuckets = ntasks, .querydscp4 = -1, .querydscp6 = -1 }; + atomic_init(&res->activebuckets, ntasks); + res->quotaresp[dns_quotatype_zone] = DNS_R_DROP; res->quotaresp[dns_quotatype_server] = DNS_R_SERVFAIL; isc_refcount_init(&res->references, 1); @@ -9877,8 +9973,8 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr, isc_mutex_init(&res->buckets[i].lock); /* - * Since we have a pool of tasks we bind them to task queues - * to spread the load evenly + * Since we have a pool of tasks we bind them to task + * queues to spread the load evenly */ res->buckets[i].task = NULL; result = isc_task_create_bound(taskmgr, 0, @@ -10048,12 +10144,12 @@ dns_resolver_prime(dns_resolver_t *res) { if (want_priming) { /* * To avoid any possible recursive locking problems, we - * start the priming fetch like any other fetch, and holding - * no resolver locks. No one else will try to start it - * because we're the ones who set res->priming to true. - * Any other callers of dns_resolver_prime() while we're - * running will see that res->priming is already true and - * do nothing. + * start the priming fetch like any other fetch, and + * holding no resolver locks. No one else will try to + * start it because we're the ones who set res->priming + * to true. Any other callers of dns_resolver_prime() + * while we're running will see that res->priming is + * already true and do nothing. */ RTRACE("priming"); rdataset = isc_mem_get(res->mctx, sizeof(*rdataset)); @@ -10117,7 +10213,9 @@ dns_resolver_whenshutdown(dns_resolver_t *res, isc_task_t *task, LOCK(&res->lock); - if (atomic_load_acquire(&res->exiting) && res->activebuckets == 0) { + if (atomic_load_acquire(&res->exiting) && + atomic_load_acquire(&res->activebuckets) == 0) + { /* * We're already shutdown. Send the event. */ @@ -10139,6 +10237,7 @@ dns_resolver_shutdown(dns_resolver_t *res) { fetchctx_t *fctx; isc_result_t result; bool is_false = false; + bool is_done = false; REQUIRE(VALID_RESOLVER(res)); @@ -10157,12 +10256,14 @@ dns_resolver_shutdown(dns_resolver_t *res) { } atomic_store(&res->buckets[i].exiting, true); if (ISC_LIST_EMPTY(res->buckets[i].fctxs)) { - INSIST(res->activebuckets > 0); - res->activebuckets--; + if (isc_refcount_decrement( + &res->activebuckets) == 1) { + is_done = true; + } } UNLOCK(&res->buckets[i].lock); } - if (res->activebuckets == 0) { + if (is_done) { send_shutdown_events(res); } result = isc_timer_reset(res->spillattimer, @@ -10185,10 +10286,8 @@ dns_resolver_detach(dns_resolver_t **resp) { RTRACE("detach"); if (isc_refcount_decrement(&res->references) == 1) { - LOCK(&res->lock); + isc_refcount_destroy(&res->activebuckets); INSIST(atomic_load_acquire(&res->exiting)); - INSIST(res->activebuckets == 0); - UNLOCK(&res->lock); destroy(res); } } @@ -10255,7 +10354,8 @@ fctx_minimize_qname(fetchctx_t *fctx) { * boundaries at /16, /32, /48, /56, /64 and /128 * In 'label count' terms that's equal to * 7 11 15 17 19 35 - * We fix fctx->qmin_labels to point to the nearest boundary + * We fix fctx->qmin_labels to point to the nearest + * boundary */ if (fctx->qmin_labels < 7) { fctx->qmin_labels = 7; @@ -10452,8 +10552,9 @@ dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name, isc_task_send(res->buckets[bucketnum].task, &event); } else { /* - * We don't care about the result of fctx_unlink() - * since we know we're not exiting. + * We don't care about the result of + * fctx_unlink() since we know we're not + * exiting. */ (void)fctx_unlink(fctx); dodestroy = true; @@ -10787,7 +10888,8 @@ dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, memmove(tmp, algorithms, *algorithms); } tmp[len - 1] |= mask; - /* 'tmp[0]' should contain the length of 'tmp'. */ + /* 'tmp[0]' should contain the length of 'tmp'. + */ *tmp = len; node->data = tmp; /* Free the older bitfield. */ @@ -11155,7 +11257,9 @@ dns_resolver_dumpfetches(dns_resolver_t *resolver, isc_statsformat_t format, fc = ISC_LIST_NEXT(fc, link)) { dns_name_print(fc->domain, fp); - fprintf(fp, ": %u active (%u spilled, %u allowed)\n", + fprintf(fp, + ": %u active (%u spilled, %u " + "allowed)\n", fc->count, fc->dropped, fc->allowed); } UNLOCK(&resolver->dbuckets[i].lock); diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c index 65a2f3d441..b8d083b687 100644 --- a/lib/dns/tests/dispatch_test.c +++ b/lib/dns/tests/dispatch_test.c @@ -168,7 +168,7 @@ reset(void) { dns_dispatchset_destroy(&dset); } if (dispatchmgr != NULL) { - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } } @@ -393,7 +393,7 @@ dispatch_getnext(void **state) { * Shutdown the dispatch. */ dns_dispatch_detach(&dispatch); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } int diff --git a/lib/dns/tests/resolver_test.c b/lib/dns/tests/resolver_test.c index 2989af3041..2c8b6f4e9a 100644 --- a/lib/dns/tests/resolver_test.c +++ b/lib/dns/tests/resolver_test.c @@ -70,7 +70,7 @@ _teardown(void **state) { dns_dispatch_detach(&dispatch); dns_view_detach(&view); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); dns_test_end(); return (0); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 9b26c61ffe..d2e1b75bbf 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -14011,8 +14011,8 @@ refresh_callback(isc_task_t *task, isc_event_t *event) { result = dns_request_getresponse(revent->request, msg, 0); if (result != ISC_R_SUCCESS) { dns_zone_log(zone, ISC_LOG_INFO, - "refresh: failure trying master " - "%s (source %s): %s", + "refresh: unable to get response, master " + "%s, source %s: %s", master, source, dns_result_totext(result)); goto next_master; } diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 8ece586163..8f49e53047 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -165,7 +165,7 @@ shutdown_managers(isc_task_t *task, isc_event_t *event) { } if (dispatchmgr != NULL) { - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } atomic_store(&shutdown_done, true);