From 7d360bd05ecf74bb106e773a1ae9ab033672ea8a Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Sun, 20 Mar 2022 13:54:39 +0000 Subject: [PATCH 1/3] Fix "dig +nssearch" indefinitely hanging issue When finishing the NSSEARCH task and there is no more followup lookups to start, dig does not destroy the last lookup, which causes it to hang indefinitely. Rename the unused `first_pass` member of `dig_query_t` to `started` and make it `true` in the first callback after `start_udp()` or `start_tcp()` of the query to indicate that the query has been started. Create a new `check_if_queries_done()` function to check whether all of the queries inside a lookup have been started and finished, or canceled. Use the mentioned function in the TRACE code block in `recv_done()` to check whether the current query is the last one in the lookup and cancel the lookup in that case to free the resources. --- bin/dig/dighost.c | 39 ++++++++++++++++++++++++++++++++++++--- bin/dig/dighost.h | 2 +- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 661ca6e8ab..6d710adb49 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1551,6 +1551,30 @@ check_if_done(void) { } } +/*% + * Check if we're done with all the queries in the lookup, except for + * the `except_q` query (can be NULL if no exception is required). + * Expects `l` to be a valid and locked lookup. + */ +static bool +check_if_queries_done(dig_lookup_t *l, dig_query_t *except_q) { + dig_query_t *q = ISC_LIST_HEAD(l->q); + + debug("check_if_queries_done(%p)", l); + + while (q != NULL) { + if (!q->started || isc_refcount_current(&q->references) > 1) { + if (!q->canceled && q != except_q) { + debug("there is a pending query %p", q); + return (false); + } + } + q = ISC_LIST_NEXT(q, link); + } + + return (true); +} + static void _destroy_lookup(dig_lookup_t *lookup) { dig_server_t *s; @@ -2126,7 +2150,6 @@ _new_query(dig_lookup_t *lookup, char *servname, char *userarg, *query = (dig_query_t){ .sendbuf = lookup->renderbuf, .servname = servname, .userarg = userarg, - .first_pass = true, .warn_id = true, .recvspace = isc_mem_get(mctx, COMMSIZE), .tmpsendspace = isc_mem_get(mctx, COMMSIZE) }; @@ -3074,6 +3097,8 @@ udp_ready(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { dig_query_t *readquery = NULL; int local_timeout = timeout * 1000; + query->started = true; + if (eresult == ISC_R_CANCELED || query->canceled) { dig_lookup_t *l = query->lookup; @@ -3413,14 +3438,17 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { char sockstr[ISC_SOCKADDR_FORMATSIZE]; dig_lookup_t *l = NULL; + REQUIRE(DIG_VALID_QUERY(query)); + REQUIRE(query->handle == NULL); + debug("tcp_connected()"); + query->started = true; + if (atomic_load(&cancel_now)) { return; } - REQUIRE(DIG_VALID_QUERY(query)); - REQUIRE(query->handle == NULL); INSIST(!free_now); debug("tcp_connected(%p, %s, %p)", handle, isc_result_totext(eresult), @@ -4310,7 +4338,12 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, l->trace_root = false; usesearch = false; } else { + /* + * This is a query in the followup lookup + */ dighost_printmessage(query, &b, msg, true); + + docancel = check_if_queries_done(l, query); } } } diff --git a/bin/dig/dighost.h b/bin/dig/dighost.h index 3e3e0a0d8b..d79cc6fc1a 100644 --- a/bin/dig/dighost.h +++ b/bin/dig/dighost.h @@ -194,7 +194,7 @@ struct dig_lookup { struct dig_query { unsigned int magic; dig_lookup_t *lookup; - bool first_pass; + bool started; bool first_soa_rcvd; bool second_rr_rcvd; bool first_repeat_rcvd; From 3a5793ece2dead9722bb0ecf5f23aa00226178ec Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Sun, 20 Mar 2022 14:03:26 +0000 Subject: [PATCH 2/3] Add CHANGES note for [GL #3145] --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 61cf97dab9..8d6ddedf25 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5844. [bug] dig +nssearch was hanging until manually interrupted. + [GL #3145] + 5843. [bug] When an UPDATE targets a zone that is not configured, the requested zone name is now logged in the "not authoritative" error message, so that it is easier to From 4477f718688b47cf46898c07fc56bcfdd254e074 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 29 Mar 2022 20:58:15 +0000 Subject: [PATCH 3/3] Synchronze udp_ready() and tcp_connected() functions entry behavior The `udp_ready()` and `tcp_connected()` functions in dighost.c are used for similar purposes for UDP and TCP respectively. Synchronize the `udp_ready()` function entry code to behave like `tcp_connected()` by adding input validation, debug messages and early exit code when `cancel_now` is `true`. --- bin/dig/dighost.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 6d710adb49..6f99f54064 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3097,8 +3097,22 @@ udp_ready(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { dig_query_t *readquery = NULL; int local_timeout = timeout * 1000; + REQUIRE(DIG_VALID_QUERY(query)); + REQUIRE(query->handle == NULL); + + debug("udp_ready()"); + query->started = true; + if (atomic_load(&cancel_now)) { + return; + } + + INSIST(!free_now); + + debug("udp_ready(%p, %s, %p)", handle, isc_result_totext(eresult), + query); + if (eresult == ISC_R_CANCELED || query->canceled) { dig_lookup_t *l = query->lookup;