Merge branch '3183-send-shutdown-to-ns_client-on-shutdown' into 'main'

Propagate the shutdown event to the recursing ns_client(s)

Closes #3183

See merge request isc-projects/bind9!7027

CHANGES

@@ -1,3 +1,7 @@
+6040.	[bug]		Speed up the named shutdown time by explicitly
+			canceling all recursing ns_client objects for
+			each ns_clientmgr. [GL #3183]
+
 6039.	[bug]		Removing a catalog zone from catalog-zones without
 			also removing the referenced zone could leave a
 			dangling pointer. [GL #3683]

doc/notes/notes-current.rst

@@ -75,6 +75,9 @@ Bug Fixes
   cases a dangling pointer could cause a :iscman:`named` process
   crash. This has been fixed. :gl:`#3683`
 
+- The ``named`` would wait for some outstanding recursing queries
+  to finish before shutting down.  This has been fixed.  :gl:`#3183`
+
 Known Issues
 ~~~~~~~~~~~~
 

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 *

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);
 /*%<

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