From: Ondřej Surý <ondrej@isc.org>
Date: Thu, 3 Nov 2022 16:42:12 +0000 (+0100)
Subject: Propagate the shutdown event to the recursing ns_client(s)
X-Git-Tag: v9.19.8~12^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5111258e7a856a8a9f287061b4756a09724cdaf4;p=thirdparty%2Fbind9.git

Propagate the shutdown event to the recursing ns_client(s)

Send the ns_query_cancel() on the recursing clients when we initiate the
named shutdown for faster shutdown.

When we are shutting down the resolver, we cancel all the outstanding
fetches, and the ISC_R_CANCEL events doesn't propagate to the ns_client
callback.

In the future, the better solution how to fix this would be to look at
the shutdown paths and let them all propagate from bottom (loopmgr) to
top (f.e. ns_client).
---

diff --git a/lib/ns/client.c b/lib/ns/client.c
index 95b2abed403..39ef9acd9f6 100644
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -120,10 +120,6 @@
 
 atomic_uint_fast64_t ns_client_requests = 0;
 
-static void
-clientmgr_attach(ns_clientmgr_t *source, ns_clientmgr_t **targetp);
-static void
-clientmgr_detach(ns_clientmgr_t **mp);
 static void
 clientmgr_destroy_cb(void *arg);
 static void
@@ -1680,7 +1676,7 @@ ns__client_put_cb(void *client0) {
 
 	isc_mem_put(manager->mctx, client, sizeof(*client));
 
-	clientmgr_detach(&manager);
+	ns_clientmgr_detach(&manager);
 }
 
 /*
@@ -2319,7 +2315,7 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
 
 		*client = (ns_client_t){ .magic = 0 };
 
-		clientmgr_attach(mgr, &client->manager);
+		ns_clientmgr_attach(mgr, &client->manager);
 
 		dns_message_create(client->manager->mctx,
 				   DNS_MESSAGE_INTENTPARSE, &client->message);
@@ -2381,7 +2377,7 @@ cleanup:
 	}
 
 	if (client->manager != NULL) {
-		clientmgr_detach(&client->manager);
+		ns_clientmgr_detach(&client->manager);
 	}
 
 	return (result);
@@ -2391,21 +2387,6 @@ cleanup:
  *** Client Manager
  ***/
 
-static void
-clientmgr_attach(ns_clientmgr_t *source, ns_clientmgr_t **targetp) {
-	int32_t oldrefs;
-
-	REQUIRE(VALID_MANAGER(source));
-	REQUIRE(targetp != NULL && *targetp == NULL);
-
-	oldrefs = isc_refcount_increment0(&source->references);
-	isc_log_write(ns_lctx, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT,
-		      ISC_LOG_DEBUG(3), "clientmgr @%p attach: %d", source,
-		      oldrefs + 1);
-
-	*targetp = source;
-}
-
 static void
 clientmgr_destroy_cb(void *arg) {
 	ns_clientmgr_t *manager = (ns_clientmgr_t *)arg;
@@ -2425,23 +2406,13 @@ clientmgr_destroy_cb(void *arg) {
 }
 
 static void
-clientmgr_detach(ns_clientmgr_t **mp) {
-	int32_t oldrefs;
-	ns_clientmgr_t *mgr = *mp;
-	*mp = NULL;
-
-	oldrefs = isc_refcount_decrement(&mgr->references);
-	isc_log_write(ns_lctx, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_CLIENT,
-		      ISC_LOG_DEBUG(3), "clientmgr @%p detach: %d", mgr,
-		      oldrefs - 1);
-	if (oldrefs == 1) {
-		isc_loop_t *loop = isc_loop_get(mgr->loopmgr, mgr->tid);
-
-		/* FIXME: Use isc_loopmgr_teardown() function instead? */
-		isc_async_run(loop, clientmgr_destroy_cb, mgr);
-	}
+clientmgr_destroy(ns_clientmgr_t *mgr) {
+	isc_loop_t *loop = isc_loop_get(mgr->loopmgr, mgr->tid);
+	isc_async_run(loop, clientmgr_destroy_cb, mgr);
 }
 
+ISC_REFCOUNT_IMPL(ns_clientmgr, clientmgr_destroy);
+
 isc_result_t
 ns_clientmgr_create(ns_server_t *sctx, isc_taskmgr_t *taskmgr,
 		    isc_loopmgr_t *loopmgr, dns_aclenv_t *aclenv, int tid,
@@ -2485,13 +2456,20 @@ ns_clientmgr_create(ns_server_t *sctx, isc_taskmgr_t *taskmgr,
 }
 
 void
-ns_clientmgr_destroy(ns_clientmgr_t **managerp) {
-	REQUIRE(managerp != NULL);
-	REQUIRE(VALID_MANAGER(*managerp));
+ns_clientmgr_shutdown(ns_clientmgr_t *manager) {
+	ns_client_t *client;
+
+	REQUIRE(VALID_MANAGER(manager));
 
 	MTRACE("destroy");
 
-	clientmgr_detach(managerp);
+	LOCK(&manager->reclock);
+	for (client = ISC_LIST_HEAD(manager->recursing); client != NULL;
+	     client = ISC_LIST_NEXT(client, rlink))
+	{
+		ns_query_cancel(client);
+	}
+	UNLOCK(&manager->reclock);
 }
 
 isc_sockaddr_t *
diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h
index 47b72685025..1f92486f61a 100644
--- a/lib/ns/include/ns/client.h
+++ b/lib/ns/include/ns/client.h
@@ -329,10 +329,10 @@ ns_clientmgr_create(ns_server_t *sctx, isc_taskmgr_t *taskmgr,
  */
 
 void
-ns_clientmgr_destroy(ns_clientmgr_t **managerp);
+ns_clientmgr_shutdown(ns_clientmgr_t *manager);
 /*%<
- * Destroy a client manager and all ns_client_t objects
- * managed by it.
+ * Shutdown a client manager and all ns_client_t objects
+ * managed by it
  */
 
 isc_sockaddr_t *
@@ -532,6 +532,8 @@ ns_client_findversion(ns_client_t *client, dns_db_t *db);
  * allocated by ns_client_newdbversion().
  */
 
+ISC_REFCOUNT_DECL(ns_clientmgr);
+
 isc_result_t
 ns__client_setup(ns_client_t *client, ns_clientmgr_t *manager, bool new);
 /*%<
diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c
index 4f4253e4881..dc86e2e9753 100644
--- a/lib/ns/interfacemgr.c
+++ b/lib/ns/interfacemgr.c
@@ -382,7 +382,7 @@ ns_interfacemgr_destroy(ns_interfacemgr_t *mgr) {
 	clearlistenon(mgr);
 	isc_mutex_destroy(&mgr->lock);
 	for (size_t i = 0; i < mgr->ncpus; i++) {
-		ns_clientmgr_destroy(&mgr->clientmgrs[i]);
+		ns_clientmgr_detach(&mgr->clientmgrs[i]);
 	}
 	isc_mem_put(mgr->mctx, mgr->clientmgrs,
 		    mgr->ncpus * sizeof(mgr->clientmgrs[0]));
@@ -451,6 +451,10 @@ ns_interfacemgr_shutdown(ns_interfacemgr_t *mgr) {
 	if (mgr->route != NULL) {
 		isc_nm_cancelread(mgr->route);
 	}
+
+	for (size_t i = 0; i < mgr->ncpus; i++) {
+		ns_clientmgr_shutdown(mgr->clientmgrs[i]);
+	}
 }
 
 static void