From 16bdced2cafcc5d9a4a15206365a215b9cda4600 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. (cherry picked from commit 7d360bd05ecf74bb106e773a1ae9ab033672ea8a) --- 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 7b68d252ee..ac847e3b8b 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1520,6 +1520,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; @@ -2075,7 +2099,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) }; @@ -2895,6 +2918,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; @@ -3234,14 +3259,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), @@ -4120,7 +4148,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 afefed40c2..43c72777de 100644 --- a/bin/dig/dighost.h +++ b/bin/dig/dighost.h @@ -183,7 +183,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 09e9aabb11a3f6401801c756e37601f3398d5453 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] (cherry picked from commit 3a5793ece2dead9722bb0ecf5f23aa00226178ec) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index f9209830ae..0ade291fef 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 cfdf95d437c529f0625929ccd64e3e5df79e56d2 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`. (cherry picked from commit 4477f718688b47cf46898c07fc56bcfdd254e074) --- bin/dig/dighost.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index ac847e3b8b..2556f25ac8 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2918,8 +2918,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;