Pin dns_request to the associated loop

When dns_request was canceled via dns_requestmgr_shutdown() the cancel
event would be propagated on different loop (loop 0) than the loop where
request was created on.  In turn this would propagate down to isc_netmgr
where we require all the events to be called from the matching isc_loop.

Pin the dns_requests to the loops and ensure that all the events are
called on the associated loop.  This in turn allows us to remove the
hashed locks on the requests and change the single .requests list to be
a per-loop list for the request accounting.

Additionally, do some extra cleanup because some race condititions are
now not possible as all events on the dns_request are serialized.

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 });

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");
 

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,
+	RUNCHECK(dns_requestmgr_create(mctx, loopmgr, dispatchmgr, dispatchv4,
-				       &requestmgr));
+				       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);

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);

lib/dns/include/dns/request.h

@@ -36,10 +36,14 @@
 
 #include <stdbool.h>
 
+#include <isc/job.h>
 #include <isc/lang.h>
+#include <isc/tls.h>
 
 #include <dns/types.h>
 
+#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
+#if DNS_REQUEST_TRACE
-dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp);
+#define dns_requestmgr_ref(ptr) \
-/*%<
+	dns_requestmgr__ref(ptr, __func__, __FILE__, __LINE__)
- *	Attach to the request manager.  dns_requestmgr_shutdown() must not
+#define dns_requestmgr_unref(ptr) \
- *	have been called on 'source' prior to calling dns_requestmgr_attach().
+	dns_requestmgr__unref(ptr, __func__, __FILE__, __LINE__)
- *
+#define dns_requestmgr_attach(ptr, ptrp) \
- * Requires:
+	dns_requestmgr__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
- *
+#define dns_requestmgr_detach(ptrp) \
- *\li	'source' is a valid requestmgr.
+	dns_requestmgr__detach(ptrp, __func__, __FILE__, __LINE__)
- *
+ISC_REFCOUNT_TRACE_DECL(dns_requestmgr);
- *\li	'targetp' to be non NULL and '*targetp' to be NULL.
+#else
- */
+ISC_REFCOUNT_DECL(dns_requestmgr);
-
+#endif
-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.
- */
 
 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

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) {

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_cancel(forward->request);
-			dns_request_attach(forward->request, &request);
-			isc_async_run(zone->loop, forward_cancel_cb, 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 = (struct stub_glue_request){
-	sgr->args = args;
+		.args = args,
-	sgr->name = (dns_name_t)DNS_NAME_INITEMPTY;
+		.name = (dns_name_t)DNS_NAME_INITEMPTY,
-	sgr->ipv4 = ipv4;
+		.ipv4 = ipv4,
+	};
+
 	dns_name_dup(name, zone->mctx, &sgr->name);
 
 	create_query(zone, ipv4 ? dns_rdatatype_a : dns_rdatatype_aaaa,

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);