From: Aram Sargsyan <aram@isc.org>
Date: Tue, 26 Nov 2024 12:06:03 +0000 (+0000)
Subject: xfrin: refactor and fix the ISC_R_CANCELED case handling
X-Git-Tag: v9.21.3~12^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3262ebd0f3fb53a471c7d736ea32710f26dcd509;p=thirdparty%2Fbind9.git

xfrin: refactor and fix the ISC_R_CANCELED case handling

Previously a ISC_R_CANCELED result code switch-case has been added to
the zone.c:zone_xfrdone() function, which did two things:

1. Schedule a new zone transfer if there's a scheduled force reload of
   the zone.

2. Reset the primaries list.

This proved to be not a well-thought change and causes problems,
because the ISC_R_CANCELED code is used not only when the whole transfer
is canceled, but also when, for example, a particular primary server is
unreachable, and named still needs to continue the transfer process by
trying the next server, which it now no longer does in some cases. To
solve this issue, three changes are made:

1. Make sure dns_zone_refresh() runs on the zone's loop, so that the
   sequential calls of dns_zone_stopxfr() and dns_zone_forcexfr()
   functions (like done in 'rndc retransfer -force') run in intended
   order and don't race with each other.

2. Since starting the new transfer is now guaranteed to run after the
   previous transfer is shut down (see the previous change), remove the
   special handling of the ISC_R_CANCELED case, and let the default
   handler to handle it like before. This will bring back the ability to
   try the next primary if the current one was interrupted with a
   ISC_R_CANCELED result code.

3. Change the xfrin.c:xfrin_shutdown() function to pass the
   ISC_R_SHUTTINGDOWN result code instead of ISC_R_CANCELED, as it makes
   more sense.
---

diff --git a/bin/tests/system/xfer/tests.sh b/bin/tests/system/xfer/tests.sh
index 5818870b603..cb67e405478 100755
--- a/bin/tests/system/xfer/tests.sh
+++ b/bin/tests/system/xfer/tests.sh
@@ -677,7 +677,7 @@ msg="'axfr-rndc-retransfer-force/IN' from 10.53.0.1#${PORT}: received"
 retry_quiet 5 wait_for_message "$msg" || tmp=1
 # Issue a retransfer-force command which should cancel the ongoing transfer and start a new one
 $RNDCCMD 10.53.0.6 retransfer -force axfr-rndc-retransfer-force 2>&1 | sed 's/^/ns6 /' | cat_i
-msg="'axfr-rndc-retransfer-force/IN' from 10.53.0.1#${PORT}: Transfer status: operation canceled"
+msg="'axfr-rndc-retransfer-force/IN' from 10.53.0.1#${PORT}: Transfer status: shutting down"
 retry_quiet 5 wait_for_message "$msg" || tmp=1
 # Wait for the new transfer to complete successfully
 msg="'axfr-rndc-retransfer-force/IN' from 10.53.0.1#${PORT}: Transfer status: success"
diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c
index d9e3a2733ce..47f58aed5f3 100644
--- a/lib/dns/xfrin.c
+++ b/lib/dns/xfrin.c
@@ -1082,7 +1082,7 @@ xfrin_shutdown(void *arg) {
 
 	REQUIRE(VALID_XFRIN(xfr));
 
-	xfrin_fail(xfr, ISC_R_CANCELED, "shut down");
+	xfrin_fail(xfr, ISC_R_SHUTTINGDOWN, "shut down");
 	dns_xfrin_detach(&xfr);
 }
 
@@ -1094,7 +1094,7 @@ dns_xfrin_shutdown(dns_xfrin_t *xfr) {
 		dns_xfrin_ref(xfr);
 		isc_async_run(xfr->loop, xfrin_shutdown, xfr);
 	} else {
-		xfrin_fail(xfr, ISC_R_CANCELED, "shut down");
+		xfrin_fail(xfr, ISC_R_SHUTTINGDOWN, "shut down");
 	}
 }
 
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 928183348ee..7889c259525 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -11522,11 +11522,23 @@ zone_refresh(dns_zone_t *zone) {
 	queue_soa_query(zone);
 }
 
-void
-dns_zone_refresh(dns_zone_t *zone) {
+static void
+zone_refresh_async(void *arg) {
+	dns_zone_t *zone = arg;
+
 	LOCK_ZONE(zone);
 	zone_refresh(zone);
 	UNLOCK_ZONE(zone);
+
+	dns_zone_detach(&zone);
+}
+
+void
+dns_zone_refresh(dns_zone_t *zone) {
+	REQUIRE(DNS_ZONE_VALID(zone));
+
+	dns_zone_ref(zone);
+	isc_async_run(zone->loop, zone_refresh_async, zone);
 }
 
 static isc_result_t
@@ -17971,17 +17983,6 @@ again:
 		inc_stats(zone, dns_zonestatscounter_xfrfail);
 		break;
 
-	case ISC_R_CANCELED:
-		/*
-		 * A new "retransfer" command with a "-force" argument could
-		 * have canceled the current transfer in which case we should
-		 * make sure to try again from the beginning.
-		 */
-		if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_FORCEXFER)) {
-			DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_REFRESH);
-			again = true;
-		}
-		FALLTHROUGH;
 	case ISC_R_SHUTTINGDOWN:
 		dns_remote_reset(&zone->primaries, true);
 		break;