diff --git a/bin/delv/delv.c b/bin/delv/delv.c index d045bfaa8c..2e6f82c7e5 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -1927,6 +1927,7 @@ cleanup: static void shutdown_server(void) { if (requestmgr != NULL) { + dns_requestmgr_shutdown(requestmgr); dns_requestmgr_detach(&requestmgr); } if (interfacemgr != NULL) { @@ -2095,7 +2096,7 @@ sendquery(void *arg) { NULL, 0)); CHECK(dns_message_setopt(message, opt)); - CHECK(dns_requestmgr_create(mctx, dispatchmgr, NULL, NULL, + CHECK(dns_requestmgr_create(mctx, loopmgr, dispatchmgr, NULL, NULL, &requestmgr)); dns_view_attach(view, &(dns_view_t *){ NULL }); diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index bedab93704..d90473b039 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -967,7 +967,7 @@ setup_system(void) { dns_transport_set_always_verify_remote(transport, tls_always_verify_remote); - result = dns_requestmgr_create(gmctx, dispatchmgr, dispatchv4, + result = dns_requestmgr_create(gmctx, loopmgr, dispatchmgr, dispatchv4, dispatchv6, &requestmgr); check_result(result, "dns_requestmgr_create"); diff --git a/bin/tests/system/pipelined/pipequeries.c b/bin/tests/system/pipelined/pipequeries.c index 805e3c5344..b242118fc2 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -183,6 +183,32 @@ sendqueries(void *arg) { return; } +static void +teardown_view(void *arg) { + dns_view_t *view = arg; + dns_view_detach(&view); +} + +static void +teardown_requestmgr(void *arg) { + dns_requestmgr_t *mgr = arg; + + dns_requestmgr_shutdown(mgr); + dns_requestmgr_detach(&mgr); +} + +static void +teardown_dispatchv4(void *arg) { + dns_dispatch_t *dispatchv4 = arg; + dns_dispatch_detach(&dispatchv4); +} + +static void +teardown_dispatchmgr(void *arg) { + dns_dispatchmgr_t *dispatchmgr = arg; + dns_dispatchmgr_detach(&dispatchmgr); +} + int main(int argc, char *argv[]) { isc_sockaddr_t bind_any; @@ -253,22 +279,19 @@ main(int argc, char *argv[]) { RUNCHECK(dns_dispatch_createudp( dispatchmgr, have_src ? &srcaddr : &bind_any, &dispatchv4)); - RUNCHECK(dns_requestmgr_create(mctx, dispatchmgr, dispatchv4, NULL, - &requestmgr)); + RUNCHECK(dns_requestmgr_create(mctx, loopmgr, dispatchmgr, dispatchv4, + NULL, &requestmgr)); RUNCHECK(dns_view_create(mctx, 0, "_test", &view)); isc_loopmgr_setup(loopmgr, sendqueries, NULL); + isc_loopmgr_teardown(loopmgr, teardown_view, view); + isc_loopmgr_teardown(loopmgr, teardown_requestmgr, requestmgr); + isc_loopmgr_teardown(loopmgr, teardown_dispatchv4, dispatchv4); + isc_loopmgr_teardown(loopmgr, teardown_dispatchmgr, dispatchmgr); + isc_loopmgr_run(loopmgr); - dns_view_detach(&view); - - dns_requestmgr_shutdown(requestmgr); - dns_requestmgr_detach(&requestmgr); - - dns_dispatch_detach(&dispatchv4); - dns_dispatchmgr_detach(&dispatchmgr); - dst_lib_destroy(); isc_log_destroy(&lctx); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index 7a366c0e85..24ac710a56 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -2068,6 +2068,32 @@ set_source_ports(dns_dispatchmgr_t *manager) { isc_portset_destroy(mctx, &v6portset); } +static void +teardown_view(void *arg) { + dns_view_t *view = arg; + dns_view_detach(&view); +} + +static void +teardown_requestmgr(void *arg) { + dns_requestmgr_t *mgr = arg; + + dns_requestmgr_shutdown(mgr); + dns_requestmgr_detach(&mgr); +} + +static void +teardown_dispatch(void *arg) { + dns_dispatch_t *dispatchv4 = arg; + dns_dispatch_detach(&dispatchv4); +} + +static void +teardown_dispatchmgr(void *arg) { + dns_dispatchmgr_t *dispatchmgr = arg; + dns_dispatchmgr_detach(&dispatchmgr); +} + /*% Main processing routine for mdig */ int main(int argc, char *argv[]) { @@ -2138,13 +2164,17 @@ main(int argc, char *argv[]) { dispatchmgr, have_src ? &srcaddr : &bind_any, &dispatchvx)); RUNCHECK(dns_requestmgr_create( - mctx, dispatchmgr, have_ipv4 ? dispatchvx : NULL, + mctx, loopmgr, dispatchmgr, have_ipv4 ? dispatchvx : NULL, have_ipv6 ? dispatchvx : NULL, &requestmgr)); RUNCHECK(dns_view_create(mctx, 0, "_mdig", &view)); query = ISC_LIST_HEAD(queries); isc_loopmgr_setup(loopmgr, sendqueries, query); + isc_loopmgr_teardown(loopmgr, teardown_view, view); + isc_loopmgr_teardown(loopmgr, teardown_requestmgr, requestmgr); + isc_loopmgr_teardown(loopmgr, teardown_dispatch, dispatchvx); + isc_loopmgr_teardown(loopmgr, teardown_dispatchmgr, dispatchmgr); /* * Stall to the start of a new second. @@ -2174,14 +2204,6 @@ main(int argc, char *argv[]) { isc_loopmgr_run(loopmgr); - dns_view_detach(&view); - - dns_requestmgr_shutdown(requestmgr); - dns_requestmgr_detach(&requestmgr); - - dns_dispatch_detach(&dispatchvx); - dns_dispatchmgr_detach(&dispatchmgr); - dst_lib_destroy(); isc_log_destroy(&lctx); diff --git a/lib/dns/include/dns/request.h b/lib/dns/include/dns/request.h index e33119742b..5e112cb437 100644 --- a/lib/dns/include/dns/request.h +++ b/lib/dns/include/dns/request.h @@ -36,10 +36,14 @@ #include +#include #include +#include #include +#undef DNS_REQUEST_TRACE + #define DNS_REQUESTOPT_TCP 0x00000001U #define DNS_REQUESTOPT_CASE 0x00000002U #define DNS_REQUESTOPT_FIXEDID 0x00000004U @@ -48,7 +52,8 @@ ISC_LANG_BEGINDECLS isc_result_t -dns_requestmgr_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispatchmgr, +dns_requestmgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, + dns_dispatchmgr_t *dispatchmgr, dns_dispatch_t *dispatchv4, dns_dispatch_t *dispatchv6, dns_requestmgr_t **requestmgrp); /*%< @@ -90,33 +95,19 @@ dns_requestmgr_shutdown(dns_requestmgr_t *requestmgr); *\li 'requestmgr' is a valid requestmgr. */ -void -dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp); -/*%< - * Attach to the request manager. dns_requestmgr_shutdown() must not - * have been called on 'source' prior to calling dns_requestmgr_attach(). - * - * Requires: - * - *\li 'source' is a valid requestmgr. - * - *\li 'targetp' to be non NULL and '*targetp' to be NULL. - */ - -void -dns_requestmgr_detach(dns_requestmgr_t **requestmgrp); -/*%< - * Detach from the given requestmgr. If this is the final detach - * requestmgr will be destroyed. dns_requestmgr_shutdown() must - * be called before the final detach. - * - * Requires: - * - *\li '*requestmgrp' is a valid requestmgr. - * - * Ensures: - *\li '*requestmgrp' is NULL. - */ +#if DNS_REQUEST_TRACE +#define dns_requestmgr_ref(ptr) \ + dns_requestmgr__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_requestmgr_unref(ptr) \ + dns_requestmgr__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_requestmgr_attach(ptr, ptrp) \ + dns_requestmgr__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_requestmgr_detach(ptrp) \ + dns_requestmgr__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(dns_requestmgr); +#else +ISC_REFCOUNT_DECL(dns_requestmgr); +#endif isc_result_t dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, @@ -302,5 +293,17 @@ dns_request_getresult(dns_request_t *request); * completion handler.) */ +#if DNS_REQUEST_TRACE +#define dns_request_ref(ptr) dns_request__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_request_unref(ptr) \ + dns_request__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_request_attach(ptr, ptrp) \ + dns_request__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_request_detach(ptrp) \ + dns_request__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(dns_request); +#else ISC_REFCOUNT_DECL(dns_request); +#endif + ISC_LANG_ENDDECLS diff --git a/lib/dns/request.c b/lib/dns/request.c index f5d4ed25cf..f675fb495f 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -45,37 +45,33 @@ typedef ISC_LIST(dns_request_t) dns_requestlist_t; -#define DNS_REQUEST_NLOCKS 7 - struct dns_requestmgr { unsigned int magic; - isc_refcount_t references; - - isc_mutex_t lock; isc_mem_t *mctx; + isc_refcount_t references; + isc_loopmgr_t *loopmgr; + + atomic_bool shuttingdown; - /* locked */ dns_dispatchmgr_t *dispatchmgr; dns_dispatch_t *dispatchv4; dns_dispatch_t *dispatchv6; - atomic_bool exiting; - unsigned int hash; - isc_mutex_t locks[DNS_REQUEST_NLOCKS]; - dns_requestlist_t requests; + dns_requestlist_t *requests; }; struct dns_request { unsigned int magic; isc_refcount_t references; - unsigned int hash; isc_mem_t *mctx; int32_t flags; + isc_loop_t *loop; + unsigned int tid; + isc_result_t result; isc_job_cb cb; void *arg; - bool complete; ISC_LINK(dns_request_t) link; isc_buffer_t *query; isc_buffer_t *answer; @@ -89,23 +85,19 @@ struct dns_request { unsigned int udpcount; }; -#define DNS_REQUEST_F_CONNECTING 0x0001 -#define DNS_REQUEST_F_SENDING 0x0002 -#define DNS_REQUEST_F_CANCELED 0x0004 -#define DNS_REQUEST_F_TCP 0x0010 +#define DNS_REQUEST_F_CONNECTING (1 << 0) +#define DNS_REQUEST_F_SENDING (1 << 1) +#define DNS_REQUEST_F_COMPLETE (1 << 2) +#define DNS_REQUEST_F_TCP (1 << 3) -#define DNS_REQUEST_CANCELED(r) (((r)->flags & DNS_REQUEST_F_CANCELED) != 0) #define DNS_REQUEST_CONNECTING(r) (((r)->flags & DNS_REQUEST_F_CONNECTING) != 0) #define DNS_REQUEST_SENDING(r) (((r)->flags & DNS_REQUEST_F_SENDING) != 0) +#define DNS_REQUEST_COMPLETE(r) (((r)->flags & DNS_REQUEST_F_COMPLETE) != 0) /*** *** Forward ***/ -static void -mgr_destroy(dns_requestmgr_t *requestmgr); -static unsigned int -mgr_gethash(dns_requestmgr_t *requestmgr); static isc_result_t req_render(dns_message_t *message, isc_buffer_t **buffer, unsigned int options, isc_mem_t *mctx); @@ -114,6 +106,8 @@ req_response(isc_result_t result, isc_region_t *region, void *arg); static void req_senddone(isc_result_t eresult, isc_region_t *region, void *arg); static void +req_cleanup(dns_request_t *request); +static void req_sendevent(dns_request_t *request, isc_result_t result); static void req_connected(isc_result_t eresult, isc_region_t *region, void *arg); @@ -121,131 +115,124 @@ 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); /*** *** Public ***/ isc_result_t -dns_requestmgr_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispatchmgr, +dns_requestmgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, + dns_dispatchmgr_t *dispatchmgr, dns_dispatch_t *dispatchv4, dns_dispatch_t *dispatchv6, dns_requestmgr_t **requestmgrp) { - dns_requestmgr_t *requestmgr; - int i; - - req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_create"); - REQUIRE(requestmgrp != NULL && *requestmgrp == NULL); REQUIRE(dispatchmgr != NULL); - requestmgr = isc_mem_get(mctx, sizeof(*requestmgr)); - *requestmgr = (dns_requestmgr_t){ 0 }; + req_log(ISC_LOG_DEBUG(3), "%s", __func__); + + dns_requestmgr_t *requestmgr = isc_mem_get(mctx, sizeof(*requestmgr)); + *requestmgr = (dns_requestmgr_t){ + .magic = REQUESTMGR_MAGIC, + .loopmgr = loopmgr, + }; + isc_mem_attach(mctx, &requestmgr->mctx); + + uint32_t nloops = isc_loopmgr_nloops(requestmgr->loopmgr); + requestmgr->requests = isc_mem_getx( + requestmgr->mctx, nloops * sizeof(requestmgr->requests[0]), + ISC_MEM_ZERO); + for (size_t i = 0; i < nloops; i++) { + ISC_LIST_INIT(requestmgr->requests[i]); + + /* unreferenced in requests_shutdown() */ + isc_loop_ref(isc_loop_get(requestmgr->loopmgr, i)); + } dns_dispatchmgr_attach(dispatchmgr, &requestmgr->dispatchmgr); - isc_mutex_init(&requestmgr->lock); - for (i = 0; i < DNS_REQUEST_NLOCKS; i++) { - isc_mutex_init(&requestmgr->locks[i]); - } if (dispatchv4 != NULL) { dns_dispatch_attach(dispatchv4, &requestmgr->dispatchv4); } if (dispatchv6 != NULL) { dns_dispatch_attach(dispatchv6, &requestmgr->dispatchv6); } - isc_mem_attach(mctx, &requestmgr->mctx); isc_refcount_init(&requestmgr->references, 1); - ISC_LIST_INIT(requestmgr->requests); - - atomic_init(&requestmgr->exiting, false); - - requestmgr->magic = REQUESTMGR_MAGIC; - - req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_create: %p", requestmgr); + req_log(ISC_LOG_DEBUG(3), "%s: %p", __func__, requestmgr); *requestmgrp = requestmgr; return (ISC_R_SUCCESS); } +static void +requests_shutdown(void *arg) { + dns_requestmgr_t *requestmgr = arg; + dns_request_t *request = NULL, *next = NULL; + uint32_t tid = isc_tid(); + + ISC_LIST_FOREACH_SAFE (requestmgr->requests[tid], request, link, next) { + req_log(ISC_LOG_DEBUG(3), "%s(%" PRIu32 ": request %p", + __func__, tid, request); + if (DNS_REQUEST_COMPLETE(request)) { + /* The callback has been already scheduled */ + continue; + } + req_sendevent(request, ISC_R_SHUTTINGDOWN); + } + + isc_loop_unref(isc_loop_get(requestmgr->loopmgr, tid)); + dns_requestmgr_detach(&requestmgr); +} + 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); + req_log(ISC_LOG_DEBUG(3), "%s: %p", __func__, requestmgr); - if (!atomic_compare_exchange_strong(&requestmgr->exiting, - &(bool){ false }, true)) - { - return; - } + rcu_read_lock(); + INSIST(atomic_compare_exchange_strong(&requestmgr->shuttingdown, + &(bool){ false }, true)); + rcu_read_unlock(); - LOCK(&requestmgr->lock); - for (request = ISC_LIST_HEAD(requestmgr->requests); request != NULL; - request = ISC_LIST_NEXT(request, link)) - { - dns_request_cancel(request); - } - UNLOCK(&requestmgr->lock); -} + /* + * Wait until all dns_request_create{raw}() are finished, so + * there will be no new requests added to the lists. + */ + synchronize_rcu(); -void -dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp) { - uint_fast32_t ref; + uint32_t tid = isc_tid(); + uint32_t nloops = isc_loopmgr_nloops(requestmgr->loopmgr); + for (size_t i = 0; i < nloops; i++) { + dns_requestmgr_ref(requestmgr); - REQUIRE(VALID_REQUESTMGR(source)); - REQUIRE(targetp != NULL && *targetp == NULL); + if (i == tid) { + /* Run the current loop synchronously */ + requests_shutdown(requestmgr); + continue; + } - REQUIRE(!atomic_load_acquire(&source->exiting)); - - ref = isc_refcount_increment(&source->references); - - req_log(ISC_LOG_DEBUG(3), - "dns_requestmgr_attach: %p: references = %" PRIuFAST32, source, - ref + 1); - - *targetp = source; -} - -void -dns_requestmgr_detach(dns_requestmgr_t **requestmgrp) { - dns_requestmgr_t *requestmgr = NULL; - uint_fast32_t ref; - - REQUIRE(requestmgrp != NULL && VALID_REQUESTMGR(*requestmgrp)); - - requestmgr = *requestmgrp; - *requestmgrp = NULL; - - ref = isc_refcount_decrement(&requestmgr->references); - - req_log(ISC_LOG_DEBUG(3), - "dns_requestmgr_detach: %p: references = %" PRIuFAST32, - requestmgr, ref - 1); - - if (ref == 1) { - INSIST(ISC_LIST_EMPTY(requestmgr->requests)); - mgr_destroy(requestmgr); + isc_loop_t *loop = isc_loop_get(requestmgr->loopmgr, i); + isc_async_run(loop, requests_shutdown, requestmgr); } } static void -mgr_destroy(dns_requestmgr_t *requestmgr) { - int i; +requestmgr_destroy(dns_requestmgr_t *requestmgr) { + req_log(ISC_LOG_DEBUG(3), "%s", __func__); - req_log(ISC_LOG_DEBUG(3), "mgr_destroy"); + INSIST(atomic_load(&requestmgr->shuttingdown)); isc_refcount_destroy(&requestmgr->references); - isc_mutex_destroy(&requestmgr->lock); - for (i = 0; i < DNS_REQUEST_NLOCKS; i++) { - isc_mutex_destroy(&requestmgr->locks[i]); + size_t nloops = isc_loopmgr_nloops(requestmgr->loopmgr); + for (size_t i = 0; i < nloops; i++) { + INSIST(ISC_LIST_EMPTY(requestmgr->requests[i])); } + isc_mem_put(requestmgr->mctx, requestmgr->requests, + nloops * sizeof(requestmgr->requests[0])); + if (requestmgr->dispatchv4 != NULL) { dns_dispatch_detach(&requestmgr->dispatchv4); } @@ -260,21 +247,17 @@ mgr_destroy(dns_requestmgr_t *requestmgr) { sizeof(*requestmgr)); } -static unsigned int -mgr_gethash(dns_requestmgr_t *requestmgr) { - req_log(ISC_LOG_DEBUG(3), "mgr_gethash"); - /* - * Locked by caller. - */ - requestmgr->hash++; - return (requestmgr->hash % DNS_REQUEST_NLOCKS); -} +#if DNS_REQUEST_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_requestmgr, requestmgr_destroy); +#else +ISC_REFCOUNT_IMPL(dns_requestmgr, requestmgr_destroy); +#endif static void req_send(dns_request_t *request) { isc_region_t r; - req_log(ISC_LOG_DEBUG(3), "req_send: request %p", request); + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); REQUIRE(VALID_REQUEST(request)); @@ -287,20 +270,38 @@ req_send(dns_request_t *request) { dns_dispatch_send(request->dispentry, &r); } -static isc_result_t -new_request(isc_mem_t *mctx, dns_request_t **requestp) { - dns_request_t *request = NULL; - - request = isc_mem_get(mctx, sizeof(*request)); - *request = (dns_request_t){ 0 }; - ISC_LINK_INIT(request, link); +static dns_request_t * +new_request(isc_mem_t *mctx, isc_loop_t *loop, isc_job_cb cb, void *arg, + bool tcp, unsigned int timeout, unsigned int udptimeout, + unsigned int udpretries) { + dns_request_t *request = isc_mem_get(mctx, sizeof(*request)); + *request = (dns_request_t){ + .magic = REQUEST_MAGIC, + .loop = loop, + .tid = isc_tid(), + .cb = cb, + .arg = arg, + .link = ISC_LINK_INITIALIZER, + .result = ISC_R_FAILURE, + .udpcount = udpretries + 1, + }; isc_refcount_init(&request->references, 1); isc_mem_attach(mctx, &request->mctx); - request->magic = REQUEST_MAGIC; - *requestp = request; - return (ISC_R_SUCCESS); + if (tcp) { + request->timeout = timeout * 1000; + } else { + if (udptimeout == 0) { + udptimeout = timeout / request->udpcount; + } + if (udptimeout == 0) { + udptimeout = 1; + } + request->timeout = udptimeout * 1000; + } + + return (request); } static bool @@ -429,62 +430,44 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, mctx = requestmgr->mctx; - req_log(ISC_LOG_DEBUG(3), "dns_request_createraw"); + req_log(ISC_LOG_DEBUG(3), "%s", __func__); - if (atomic_load_acquire(&requestmgr->exiting)) { - return (ISC_R_SHUTTINGDOWN); + rcu_read_lock(); + + if (atomic_load_acquire(&requestmgr->shuttingdown)) { + result = ISC_R_SHUTTINGDOWN; + goto done; } if (isblackholed(requestmgr->dispatchmgr, destaddr)) { - return (DNS_R_BLACKHOLED); + result = DNS_R_BLACKHOLED; + goto done; } - /* detached in dns_request_destroy() */ - result = new_request(mctx, &request); - if (result != ISC_R_SUCCESS) { - return (result); - } - - request->loop = loop; - request->cb = cb; - request->arg = arg; - request->result = ISC_R_FAILURE; - request->udpcount = udpretries + 1; - isc_buffer_usedregion(msgbuf, &r); if (r.length < DNS_MESSAGE_HEADERLEN || r.length > 65535) { result = DNS_R_FORMERR; - goto cleanup; + goto done; } if ((options & DNS_REQUESTOPT_TCP) != 0 || r.length > 512) { tcp = true; - request->timeout = timeout * 1000; - } else { - if (udptimeout == 0) { - udptimeout = timeout / request->udpcount; - } - if (udptimeout == 0) { - udptimeout = 1; - } - request->timeout = udptimeout * 1000; } + request = new_request(mctx, loop, cb, arg, tcp, timeout, udptimeout, + udpretries); + 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; } - /* detached in req_connected() */ - dns_request_ref(request); - again: - result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, &request->dispatch); if (result != ISC_R_SUCCESS) { - goto detach; + goto cleanup; } if ((options & DNS_REQUESTOPT_FIXEDID) != 0) { @@ -498,12 +481,12 @@ again: req_response, request, &id, &request->dispentry); if (result != ISC_R_SUCCESS) { if ((options & DNS_REQUESTOPT_FIXEDID) != 0 && !newtcp) { - newtcp = true; dns_dispatch_detach(&request->dispatch); + newtcp = true; goto again; } - goto detach; + goto cleanup; } /* Add message ID. */ @@ -511,41 +494,35 @@ again: r.base[0] = (id >> 8) & 0xff; r.base[1] = id & 0xff; - LOCK(&requestmgr->lock); - dns_requestmgr_attach(requestmgr, &request->requestmgr); - request->hash = mgr_gethash(requestmgr); - ISC_LIST_APPEND(requestmgr->requests, request, link); - UNLOCK(&requestmgr->lock); - request->destaddr = *destaddr; - request->flags |= DNS_REQUEST_F_CONNECTING; if (tcp) { request->flags |= DNS_REQUEST_F_TCP; } + dns_requestmgr_attach(requestmgr, &request->requestmgr); + ISC_LIST_APPEND(requestmgr->requests[request->tid], request, link); + + dns_request_ref(request); /* detached in req_connected() */ result = dns_dispatch_connect(request->dispentry); if (result != ISC_R_SUCCESS) { - goto unlink; + dns_request_unref(request); + goto cleanup; } - req_log(ISC_LOG_DEBUG(3), "dns_request_createraw: request %p", request); + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); *requestp = request; - return (ISC_R_SUCCESS); - -unlink: - LOCK(&requestmgr->lock); - ISC_LIST_UNLINK(requestmgr->requests, request, link); - UNLOCK(&requestmgr->lock); - -detach: - /* connect failed, unref here */ - dns_request_unref(request); cleanup: - dns_request_detach(&request); - req_log(ISC_LOG_DEBUG(3), "dns_request_createraw: failed %s", - isc_result_totext(result)); + if (result != ISC_R_SUCCESS) { + req_cleanup(request); + dns_request_detach(&request); + req_log(ISC_LOG_DEBUG(3), "%s: failed %s", __func__, + isc_result_totext(result)); + } + +done: + rcu_read_unlock(); return (result); } @@ -573,35 +550,34 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, REQUIRE(timeout > 0); REQUIRE(udpretries != UINT_MAX); - mctx = requestmgr->mctx; - - req_log(ISC_LOG_DEBUG(3), "dns_request_create"); - - 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); } + mctx = requestmgr->mctx; + + req_log(ISC_LOG_DEBUG(3), "%s", __func__); + + rcu_read_lock(); + + if (atomic_load_acquire(&requestmgr->shuttingdown)) { + result = ISC_R_SHUTTINGDOWN; + goto done; + } + if (isblackholed(requestmgr->dispatchmgr, destaddr)) { - return (DNS_R_BLACKHOLED); + result = DNS_R_BLACKHOLED; + goto done; } - /* detached in dns_request_destroy() */ - result = new_request(mctx, &request); - if (result != ISC_R_SUCCESS) { - return (result); + if ((options & DNS_REQUESTOPT_TCP) != 0) { + tcp = true; } - request->loop = loop; - request->cb = cb; - request->arg = arg; - request->result = ISC_R_FAILURE; - request->udpcount = udpretries + 1; + request = new_request(mctx, loop, cb, arg, tcp, timeout, udptimeout, + udpretries); if (key != NULL) { dns_tsigkey_attach(key, &request->tsigkey); @@ -612,27 +588,11 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, goto cleanup; } - if ((options & DNS_REQUESTOPT_TCP) != 0) { - tcp = true; - request->timeout = timeout * 1000; - } else { - if (udptimeout == 0) { - udptimeout = timeout / request->udpcount; - } - if (udptimeout == 0) { - udptimeout = 1; - } - request->timeout = udptimeout * 1000; - } - - /* detached in req_connected() */ - dns_request_ref(request); - again: result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, &request->dispatch); if (result != ISC_R_SUCCESS) { - goto detach; + goto cleanup; } result = dns_dispatch_add(request->dispatch, loop, 0, request->timeout, @@ -640,65 +600,57 @@ again: req_connected, req_senddone, req_response, request, &id, &request->dispentry); if (result != ISC_R_SUCCESS) { - goto detach; + goto cleanup; } message->id = id; result = req_render(message, &request->query, options, mctx); if (result == DNS_R_USETCP && !tcp) { - /* - * Try again using TCP. - */ + /* Try again using TCP. */ dns_message_renderreset(message); dns_dispatch_done(&request->dispentry); dns_dispatch_detach(&request->dispatch); options |= DNS_REQUESTOPT_TCP; tcp = true; goto again; - } - if (result != ISC_R_SUCCESS) { - goto detach; + } else if (result != ISC_R_SUCCESS) { + goto cleanup; } result = dns_message_getquerytsig(message, mctx, &request->tsig); if (result != ISC_R_SUCCESS) { - goto detach; + goto cleanup; } - LOCK(&requestmgr->lock); - dns_requestmgr_attach(requestmgr, &request->requestmgr); - request->hash = mgr_gethash(requestmgr); - ISC_LIST_APPEND(requestmgr->requests, request, link); - UNLOCK(&requestmgr->lock); - request->destaddr = *destaddr; request->flags |= DNS_REQUEST_F_CONNECTING; if (tcp) { request->flags |= DNS_REQUEST_F_TCP; } + dns_requestmgr_attach(requestmgr, &request->requestmgr); + ISC_LIST_APPEND(requestmgr->requests[request->tid], request, link); + + dns_request_ref(request); /* detached in req_connected() */ result = dns_dispatch_connect(request->dispentry); if (result != ISC_R_SUCCESS) { - goto unlink; + dns_request_unref(request); + goto cleanup; } - req_log(ISC_LOG_DEBUG(3), "dns_request_create: request %p", request); + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); *requestp = request; - return (ISC_R_SUCCESS); - -unlink: - LOCK(&requestmgr->lock); - ISC_LIST_UNLINK(requestmgr->requests, request, link); - UNLOCK(&requestmgr->lock); - -detach: - /* connect failed, unref here */ - dns_request_unref(request); cleanup: - dns_request_detach(&request); - req_log(ISC_LOG_DEBUG(3), "dns_request_create: failed %s", - isc_result_totext(result)); + if (result != ISC_R_SUCCESS) { + req_cleanup(request); + dns_request_detach(&request); + req_log(ISC_LOG_DEBUG(3), "%s: failed %s", __func__, + isc_result_totext(result)); + } +done: + rcu_read_unlock(); + return (result); } @@ -714,7 +666,7 @@ req_render(dns_message_t *message, isc_buffer_t **bufferp, unsigned int options, REQUIRE(bufferp != NULL && *bufferp == NULL); - req_log(ISC_LOG_DEBUG(3), "request_render"); + req_log(ISC_LOG_DEBUG(3), "%s", __func__); /* * Create buffer able to hold largest possible message. @@ -792,32 +744,18 @@ cleanup: return (result); } -void -request_cancel(dns_request_t *request) { - if (!DNS_REQUEST_CANCELED(request)) { - req_log(ISC_LOG_DEBUG(3), "request_cancel: request %p", - request); - - request->flags |= DNS_REQUEST_F_CANCELED; - request->flags &= ~DNS_REQUEST_F_CONNECTING; - - if (request->dispentry != NULL) { - dns_dispatch_done(&request->dispentry); - } - - dns_dispatch_detach(&request->dispatch); - } -} - void dns_request_cancel(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); - req_log(ISC_LOG_DEBUG(3), "dns_request_cancel: request %p", request); - LOCK(&request->requestmgr->locks[request->hash]); - request_cancel(request); - req_sendevent(request, ISC_R_CANCELED); - UNLOCK(&request->requestmgr->locks[request->hash]); + if (DNS_REQUEST_COMPLETE(request)) { + /* The request callback was already called */ + return; + } + + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); + req_sendevent(request, ISC_R_CANCELED); /* call asynchronously */ } isc_result_t @@ -826,10 +764,10 @@ dns_request_getresponse(dns_request_t *request, dns_message_t *message, isc_result_t result; REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); REQUIRE(request->answer != NULL); - req_log(ISC_LOG_DEBUG(3), "dns_request_getresponse: request %p", - request); + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); dns_message_setquerytsig(message, request->tsig); result = dns_message_settsigkey(message, request->tsigkey); @@ -849,6 +787,7 @@ dns_request_getresponse(dns_request_t *request, dns_message_t *message, isc_buffer_t * dns_request_getanswer(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); return (request->answer); } @@ -856,40 +795,22 @@ dns_request_getanswer(dns_request_t *request) { bool dns_request_usedtcp(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); return ((request->flags & DNS_REQUEST_F_TCP) != 0); } void dns_request_destroy(dns_request_t **requestp) { - dns_request_t *request = NULL; - REQUIRE(requestp != NULL && VALID_REQUEST(*requestp)); - request = *requestp; + dns_request_t *request = *requestp; *requestp = NULL; - req_log(ISC_LOG_DEBUG(3), "dns_request_destroy: request %p", request); + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); - LOCK(&request->requestmgr->lock); - LOCK(&request->requestmgr->locks[request->hash]); - ISC_LIST_UNLINK(request->requestmgr->requests, request, link); - UNLOCK(&request->requestmgr->locks[request->hash]); - UNLOCK(&request->requestmgr->lock); - - /* - * These should have been cleaned up before the completion - * event was sent. - */ - INSIST(request->dispentry == NULL); - INSIST(request->dispatch == NULL); - - /* - * if we've called the completion handler, there's - * another ref to detach - */ - if (request->complete) { - dns_request_unref(request); + if (DNS_REQUEST_COMPLETE(request)) { + dns_request_cancel(request); } /* final detach to shut down request */ @@ -897,160 +818,168 @@ dns_request_destroy(dns_request_t **requestp) { } static void -req_connected(isc_result_t eresult, isc_region_t *region, void *arg) { +req_connected(isc_result_t eresult, isc_region_t *region ISC_ATTR_UNUSED, + void *arg) { dns_request_t *request = (dns_request_t *)arg; - UNUSED(region); + REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); + REQUIRE(DNS_REQUEST_CONNECTING(request)); - req_log(ISC_LOG_DEBUG(3), "req_connected: request %p: %s", request, + req_log(ISC_LOG_DEBUG(3), "%s: request %p: %s", __func__, request, isc_result_totext(eresult)); - REQUIRE(VALID_REQUEST(request)); - REQUIRE(DNS_REQUEST_CONNECTING(request) || - DNS_REQUEST_CANCELED(request)); - - LOCK(&request->requestmgr->locks[request->hash]); request->flags &= ~DNS_REQUEST_F_CONNECTING; - if (eresult == ISC_R_TIMEDOUT) { - dns_dispatch_done(&request->dispentry); - dns_dispatch_detach(&request->dispatch); - req_sendevent(request, eresult); - } else if (DNS_REQUEST_CANCELED(request)) { - req_sendevent(request, ISC_R_CANCELED); - } else if (eresult == ISC_R_SUCCESS) { + if (DNS_REQUEST_COMPLETE(request)) { + /* The request callback was already called */ + goto detach; + } + + if (eresult == ISC_R_SUCCESS) { req_send(request); } else { - request_cancel(request); req_sendevent(request, eresult); } - UNLOCK(&request->requestmgr->locks[request->hash]); +detach: /* attached in dns_request_create/_createraw() */ dns_request_unref(request); } static void -req_senddone(isc_result_t eresult, isc_region_t *region, void *arg) { +req_senddone(isc_result_t eresult, isc_region_t *region ISC_ATTR_UNUSED, + void *arg) { dns_request_t *request = (dns_request_t *)arg; REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); REQUIRE(DNS_REQUEST_SENDING(request)); - UNUSED(region); + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); - req_log(ISC_LOG_DEBUG(3), "req_senddone: request %p", request); - - LOCK(&request->requestmgr->locks[request->hash]); request->flags &= ~DNS_REQUEST_F_SENDING; - if (DNS_REQUEST_CANCELED(request)) { - if (eresult == ISC_R_TIMEDOUT) { - req_sendevent(request, eresult); - } else { - req_sendevent(request, ISC_R_CANCELED); - } - } else if (eresult != ISC_R_SUCCESS) { - request_cancel(request); - req_sendevent(request, ISC_R_CANCELED); + if (DNS_REQUEST_COMPLETE(request)) { + /* The request callback was already called */ + goto detach; } - UNLOCK(&request->requestmgr->locks[request->hash]); + if (eresult != ISC_R_SUCCESS) { + req_sendevent(request, eresult); + } +detach: /* attached in req_send() */ - dns_request_detach(&request); + dns_request_unref(request); } static void -req_response(isc_result_t result, isc_region_t *region, void *arg) { +req_response(isc_result_t eresult, isc_region_t *region, void *arg) { dns_request_t *request = (dns_request_t *)arg; - if (result == ISC_R_CANCELED) { + if (eresult == ISC_R_CANCELED) { return; } - req_log(ISC_LOG_DEBUG(3), "req_response: request %p: %s", request, - isc_result_totext(result)); - REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); - if (result == ISC_R_TIMEDOUT) { - LOCK(&request->requestmgr->locks[request->hash]); - if (request->udpcount > 1 && - (request->flags & DNS_REQUEST_F_TCP) == 0) - { + req_log(ISC_LOG_DEBUG(3), "%s: request %p: %s", __func__, request, + isc_result_totext(eresult)); + + if (DNS_REQUEST_COMPLETE(request)) { + /* The request callback was already called */ + return; + } + + switch (eresult) { + case ISC_R_TIMEDOUT: + if (request->udpcount > 1 && !dns_request_usedtcp(request)) { request->udpcount -= 1; dns_dispatch_resume(request->dispentry, request->timeout); if (!DNS_REQUEST_SENDING(request)) { req_send(request); } - UNLOCK(&request->requestmgr->locks[request->hash]); return; } - - /* The lock is unlocked below */ - goto done; + break; + case ISC_R_SUCCESS: + /* Copy region to request. */ + isc_buffer_allocate(request->mctx, &request->answer, + region->length); + eresult = isc_buffer_copyregion(request->answer, region); + if (eresult != ISC_R_SUCCESS) { + isc_buffer_free(&request->answer); + } + break; + default: + break; } - LOCK(&request->requestmgr->locks[request->hash]); + req_sendevent(request, eresult); +} - if (result != ISC_R_SUCCESS) { - goto done; +static void +req_sendevent_cb(void *arg) { + dns_request_t *request = arg; + + request->cb(request); + dns_request_unref(request); +} + +static void +req_cleanup(dns_request_t *request) { + if (ISC_LINK_LINKED(request, link)) { + ISC_LIST_UNLINK(request->requestmgr->requests[request->tid], + request, link); } - - /* - * Copy region to request. - */ - isc_buffer_allocate(request->mctx, &request->answer, region->length); - result = isc_buffer_copyregion(request->answer, region); - if (result != ISC_R_SUCCESS) { - isc_buffer_free(&request->answer); - } - -done: - /* - * Cleanup. - */ if (request->dispentry != NULL) { dns_dispatch_done(&request->dispentry); } - request_cancel(request); - - /* - * Send completion event. - */ - req_sendevent(request, result); - UNLOCK(&request->requestmgr->locks[request->hash]); + if (request->dispatch != NULL) { + dns_dispatch_detach(&request->dispatch); + } } static void req_sendevent(dns_request_t *request, isc_result_t result) { REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); + REQUIRE(!DNS_REQUEST_COMPLETE(request)); - if (request->complete) { - return; - } + request->flags |= DNS_REQUEST_F_COMPLETE; - req_log(ISC_LOG_DEBUG(3), "req_sendevent: request %p", request); + req_cleanup(request); - dns_request_ref(request); + req_log(ISC_LOG_DEBUG(3), "%s: request %p: %s", __func__, request, + isc_result_totext(result)); request->result = result; - request->complete = true; - isc_async_run(request->loop, request->cb, request); + dns_request_ref(request); + isc_async_run(request->loop, req_sendevent_cb, request); } static void req_destroy(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); REQUIRE(!ISC_LINK_LINKED(request, link)); - req_log(ISC_LOG_DEBUG(3), "req_destroy: request %p", request); + req_log(ISC_LOG_DEBUG(3), "%s: request %p", __func__, request); isc_refcount_destroy(&request->references); + /* + * These should have been cleaned up before the + * completion event was sent. + */ + INSIST(!ISC_LINK_LINKED(request, link)); + INSIST(request->dispentry == NULL); + INSIST(request->dispatch == NULL); + request->magic = 0; if (request->query != NULL) { isc_buffer_free(&request->query); @@ -1058,12 +987,6 @@ req_destroy(dns_request_t *request) { if (request->answer != NULL) { isc_buffer_free(&request->answer); } - if (request->dispentry != NULL) { - dns_dispatch_done(&request->dispentry); - } - if (request->dispatch != NULL) { - dns_dispatch_detach(&request->dispatch); - } if (request->tsig != NULL) { isc_buffer_free(&request->tsig); } @@ -1079,6 +1002,7 @@ req_destroy(dns_request_t *request) { void * dns_request_getarg(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); return (request->arg); } @@ -1086,11 +1010,16 @@ dns_request_getarg(dns_request_t *request) { isc_result_t dns_request_getresult(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); + REQUIRE(request->tid == isc_tid()); return (request->result); } +#if DNS_REQUEST_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_request, req_destroy); +#else ISC_REFCOUNT_IMPL(dns_request, req_destroy); +#endif static void req_log(int level, const char *fmt, ...) { diff --git a/lib/dns/view.c b/lib/dns/view.c index d133c805aa..0399ad73f8 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -622,7 +622,7 @@ dns_view_createresolver(dns_view_t *view, isc_loopmgr_t *loopmgr, dns_adb_create(mctx, view, loopmgr, &view->adb); isc_mem_detach(&mctx); - result = dns_requestmgr_create(view->mctx, view->dispatchmgr, + result = dns_requestmgr_create(view->mctx, loopmgr, view->dispatchmgr, dispatchv4, dispatchv6, &view->requestmgr); if (result != ISC_R_SUCCESS) { diff --git a/lib/dns/zone.c b/lib/dns/zone.c index e499444a93..b7dfbc97bf 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -11730,13 +11730,6 @@ checkds_cancel(dns_zone_t *zone) { } } -static void -forward_cancel_cb(void *arg) { - dns_request_t *request = arg; - dns_request_cancel(request); - dns_request_detach(&request); -} - static void forward_cancel(dns_zone_t *zone) { dns_forward_t *forward; @@ -11751,9 +11744,7 @@ forward_cancel(dns_zone_t *zone) { forward = ISC_LIST_NEXT(forward, link)) { if (forward->request != NULL) { - dns_request_t *request = NULL; - dns_request_attach(forward->request, &request); - isc_async_run(zone->loop, forward_cancel_cb, request); + dns_request_cancel(forward->request); } } } @@ -12878,8 +12869,9 @@ cleanup: if (msg != NULL) { dns_message_detach(&msg); } + dns_name_free(&sgr->name, zone->mctx); - dns_request_destroy(&request); + dns_request_destroy(&sgr->request); isc_mem_put(zone->mctx, sgr, sizeof(*sgr)); /* If last request, release all related resources */ @@ -12912,10 +12904,12 @@ stub_request_nameserver_address(struct stub_cb_args *args, bool ipv4, zone = args->stub->zone; sgr = isc_mem_get(zone->mctx, sizeof(*sgr)); - sgr->request = NULL; - sgr->args = args; - sgr->name = (dns_name_t)DNS_NAME_INITEMPTY; - sgr->ipv4 = ipv4; + *sgr = (struct stub_glue_request){ + .args = args, + .name = (dns_name_t)DNS_NAME_INITEMPTY, + .ipv4 = ipv4, + }; + dns_name_dup(name, zone->mctx, &sgr->name); create_query(zone, ipv4 ? dns_rdatatype_a : dns_rdatatype_aaaa, diff --git a/lib/isc/loop.c b/lib/isc/loop.c index 5ee9daa8eb..88c7fd3487 100644 --- a/lib/isc/loop.c +++ b/lib/isc/loop.c @@ -282,6 +282,9 @@ loop_thread(void *arg) { r = uv_run(&loop->loop, UV_RUN_DEFAULT); UV_RUNTIME_CHECK(uv_run, r); + /* Invalidate the loop early */ + loop->magic = 0; + isc_barrier_wait(&loop->loopmgr->stopping); return (NULL);