From: Evan Hunt Date: Wed, 10 Sep 2025 21:15:57 +0000 (-0700) Subject: rename NS_QUERY_RESET to NS_QUERY_CLEANUP X-Git-Tag: v9.21.14~50^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0cdcc8a8f444160ec991bdeee5b1e647cb09e51d;p=thirdparty%2Fbind9.git 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. --- diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index e8507397304..d5b4bb69aa4 100644 --- a/bin/plugins/filter-a.c +++ b/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); } /** diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index 3a0dc1a98c6..4bdff94116a 100644 --- a/bin/plugins/filter-aaaa.c +++ b/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); } /** diff --git a/bin/tests/system/hooks/driver/test-async.c b/bin/tests/system/hooks/driver/test-async.c index 0e7158be856..e412fdcbe72 100644 --- a/bin/tests/system/hooks/driver/test-async.c +++ b/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 diff --git a/lib/ns/client.c b/lib/ns/client.c index f3a3dc44b3c..0bd85a7c278 100644 --- a/lib/ns/client.c +++ b/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); diff --git a/lib/ns/include/ns/hooks.h b/lib/ns/include/ns/hooks.h index 3b1b4b60573..0680eecfce3 100644 --- a/lib/ns/include/ns/hooks.h +++ b/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, diff --git a/lib/ns/query.c b/lib/ns/query.c index 5a8d646e4ae..f5f8d847095 100644 --- a/lib/ns/query.c +++ b/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. + * + * 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 + * 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); } diff --git a/tests/ns/netmgr_wrap.c b/tests/ns/netmgr_wrap.c index f2b265c329f..c2e62352f93 100644 --- a/tests/ns/netmgr_wrap.c +++ b/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);