rename NS_QUERY_RESET to NS_QUERY_CLEANUP

query_reset() is called during query initialization, but the only
time the NS_QUERY_SETUP hook runs is when it's called from
query_cleanup().  it makes more sense to move the hook point to
there and rename it to NS_QUERY_CLEANUP.

this change caused a crash in the unit tests due to the view being
unnecessarily detached before ns__client_reset_cb() was called.
this has also been fixed.

bin/plugins/filter-a.c

@@ -164,7 +164,7 @@ install_hooks(ns_hooktable_t *hooktable, isc_mem_t *mctx,
 	ns_hook_add(hooktable, mctx, NS_QUERY_PREP_RESPONSE_BEGIN,
 		    &filter_prepresp);
 	ns_hook_add(hooktable, mctx, NS_QUERY_DONE_SEND, &filter_donesend);
-	ns_hook_add(hooktable, mctx, NS_QUERY_RESET, &filter_reset);
+	ns_hook_add(hooktable, mctx, NS_QUERY_CLEANUP, &filter_reset);
 }
 
 /**

bin/plugins/filter-aaaa.c

@@ -164,7 +164,7 @@ install_hooks(ns_hooktable_t *hooktable, isc_mem_t *mctx,
 	ns_hook_add(hooktable, mctx, NS_QUERY_PREP_RESPONSE_BEGIN,
 		    &filter_prepresp);
 	ns_hook_add(hooktable, mctx, NS_QUERY_DONE_SEND, &filter_donesend);
-	ns_hook_add(hooktable, mctx, NS_QUERY_RESET, &filter_reset);
+	ns_hook_add(hooktable, mctx, NS_QUERY_CLEANUP, &filter_reset);
 }
 
 /**

bin/tests/system/hooks/driver/test-async.c

@@ -96,7 +96,7 @@ install_hooks(ns_hooktable_t *hooktable, isc_mem_t *mctx,
 
 	ns_hook_add(hooktable, mctx, NS_QUERY_SETUP, &async_setup);
 	ns_hook_add(hooktable, mctx, NS_QUERY_DONE_BEGIN, &async_donebegin);
-	ns_hook_add(hooktable, mctx, NS_QUERY_RESET, &async_reset);
+	ns_hook_add(hooktable, mctx, NS_QUERY_CLEANUP, &async_reset);
 }
 
 static void

lib/ns/client.c

@@ -257,7 +257,7 @@ ns_client_endrequest(ns_client_t *client) {
 #ifdef ENABLE_AFL
 		if (client->manager->sctx->fuzztype == isc_fuzz_resolver) {
 			dns_adb_t *adb = NULL;
-			dns_view_getadb(client->view, &adb);
+			dns_view_getadb(client->inner.view, &adb);
 			if (adb != NULL) {
 				dns_adb_flush(adb);
 				dns_adb_detach(&adb);
@@ -774,7 +774,7 @@ renderend:
 #ifdef HAVE_DNSTAP
 		/*
 		 * Log dnstap data first, because client_sendpkg() may
-		 * leave client->view set to NULL.
+		 * leave client->inner.view set to NULL.
 		 */
 		if (client->inner.view != NULL) {
 			dns_dt_send(client->inner.view, dtmsgtype,
@@ -1843,7 +1843,7 @@ ns_client_setup_view(ns_client_t *client, isc_netaddr_t *netaddr) {
 	/*
 	 * matchingview() returning anything other than DNS_R_WAIT means it's
 	 * not running in async mode, in which case 'result' must be equal to
-	 * 'client->viewmatchresult'.
+	 * 'client->inner.viewmatchresult'.
 	 */
 	INSIST(result == client->inner.viewmatchresult);
 

lib/ns/include/ns/hooks.h

@@ -385,7 +385,7 @@
 typedef enum {
 	/* hookpoints from query.c */
 	NS_QUERY_SETUP,
-	NS_QUERY_RESET,
+	NS_QUERY_CLEANUP,
 	NS_QUERY_START_BEGIN,
 	NS_QUERY_LOOKUP_BEGIN,
 	NS_QUERY_RESUME_BEGIN,

lib/ns/query.c

@@ -304,14 +304,17 @@ ns__query_callhook_noreturn(uint8_t id, query_ctx_t *qctx,
 }
 
 /*
- * Call the specified hook function in every configured module that implements
- * that function. If any hook function returns NS_HOOK_RETURN, we
- * set 'result' and terminate processing by jumping to the 'cleanup' tag.
+ * Call the specified hook function in every configured module that
+ * implements that function. If any hook function returns NS_HOOK_RETURN,
+ * we set 'result' and terminate processing by jumping to the 'cleanup' tag.
  *
- * (Note that a hook function may set the 'result' to ISC_R_SUCCESS but
+ * Before any hook point is reached, qctx->view must be initialized to
+ * point to a valid view.
+ *
+ * Note: that a hook function may set the 'result' to ISC_R_SUCCESS but
  * still terminate processing within the calling function. That's why this
  * is a macro instead of a static function; it needs to be able to use
- * 'goto cleanup' regardless of the return value.)
+ * 'goto cleanup' regardless of the return value.
  */
 #define CALL_HOOK(_id, _qctx)                                               \
 	if ((_qctx)->zhooks != NULL &&                                      \
@@ -339,8 +342,11 @@ ns__query_callhook_noreturn(uint8_t id, query_ctx_t *qctx,
  * codes are ignored. This is intended for use with initialization and
  * destruction calls which *must* run in every configured module.
  *
- * (This could be implemented as a static void function, but is left as a
- * macro for symmetry with CALL_HOOK above.)
+ * Before any hook point is reached, qctx->view must be initialized to
+ * point to a valid view.
+ *
+ * This could be implemented as a static void function, but is left as a
+ * macro for symmetry with CALL_HOOK above.
  */
 #define CALL_HOOK_NORETURN(_id, _qctx)                                    \
 	if ((_qctx)->zhooks != NULL) {                                    \
@@ -806,27 +812,6 @@ query_reset(ns_client_t *client, bool everything) {
 
 	CTRACE(ISC_LOG_DEBUG(3), "query_reset");
 
-	/*
-	 * Set up a transient qctx so we can call the NS_QUERY_RESET hook;
-	 * this will free resources being held by plugins for this
-	 * query, if any were configured.
-	 *
-	 * Note that NS_QUERY_RESET hook is called only if the view is not
-	 * NULL at this point. Otherwise, it means the client has been
-	 * reset even before the query starts, so we should not call this
-	 * hook as no other hook has been called before.
-	 */
-	if (client->inner.view != NULL) {
-		query_ctx_t qctx = { .view = client->inner.view,
-				     .client = client };
-		if (client->query.authzone != NULL) {
-			qctx.zhooks =
-				dns_zone_gethooktable(client->query.authzone);
-		}
-
-		CALL_HOOK_NORETURN(NS_QUERY_RESET, &qctx);
-	}
-
 	/*
 	 * Cancel the fetch if it's running.
 	 */
@@ -920,6 +905,18 @@ query_reset(ns_client_t *client, bool everything) {
 
 static void
 query_cleanup(ns_client_t *client) {
+	/*
+	 * Set up a transient qctx so we can call the NS_QUERY_CLEANUP hook;
+	 * this will free resources being held by plugins for this
+	 * query, if any were configured.
+	 */
+	query_ctx_t qctx = { .view = client->inner.view, .client = client };
+	if (client->query.authzone != NULL) {
+		qctx.zhooks = dns_zone_gethooktable(client->query.authzone);
+	}
+
+	CALL_HOOK_NORETURN(NS_QUERY_CLEANUP, &qctx);
+
 	query_reset(client, false);
 }
 

tests/ns/netmgr_wrap.c

@@ -84,7 +84,6 @@ isc_nmhandle_detach(isc_nmhandle_t **handlep) {
 	INSIST(i < 32);
 
 	if (atomic_fetch_sub(&client_refs[i], 1) == 1) {
-		dns_view_detach(&client->inner.view);
 		client->inner.state = 4;
 		ns__client_reset_cb(client);
 		ns__client_put_cb(client);