fix: dev: Fix data race during rndc dumpdb or zone load

'rndc dumpdb' against a server with zones, and async zone load,
had a timing window where the operation's completion could fire
before the server had finished registering the operation,
occasionally leading to a possible crash.  The completion is now
delivered after the registration is in place.

Closes #5952

Merge branch '5952-fix-masterdump-async-ctx-race' into 'main'

See merge request isc-projects/bind9!11991

lib/dns/master.c

@@ -108,6 +108,8 @@ struct dns_loadctx {
 	dns_loaddonefunc_t done;
 	void *done_arg;
 
+	isc_loop_t *loop;
+
 	/* Common methods */
 	isc_result_t (*openfile)(dns_loadctx_t *lctx, const char *filename);
 	isc_result_t (*load)(dns_loadctx_t *lctx);
@@ -2577,33 +2579,38 @@ cleanup:
 	return result;
 }
 
+/*
+ * The combination of isc_work_enqueue() on the current loop and callback on
+ * lctx->loop ensures the correct ordering:
+ *
+ * 1. dns_master_loadfileasync() calls isc_work_enqueue() on the current loop.
+ * 2. master_load() runs asynchronously and can finish before the entry point
+ *    returns; master_load_done() is queued on the current loop and cannot run
+ *    until the entry point returns.
+ * 3. The entry point publishes *lctxp.
+ * 4. master_load_done() runs on the current loop and hands off to lctx->loop.
+ * 5. lctx->done() runs on lctx->loop asynchronously.
+ */
 static void
-load(void *arg) {
+master_load(void *arg) {
 	dns_loadctx_t *lctx = arg;
 	lctx->result = (lctx->load)(lctx);
 }
 
 static void
-load_done(void *arg) {
+master_load_callback(void *arg) {
 	dns_loadctx_t *lctx = arg;
 
 	(lctx->done)(lctx->done_arg, lctx->result);
+	isc_loop_detach(&lctx->loop);
 	dns_loadctx_detach(&lctx);
 }
 
 static void
-load_enqueue(void *lctx) {
-	isc_work_enqueue(isc_loop(), load, load_done, lctx);
-}
+master_load_done(void *arg) {
+	dns_loadctx_t *lctx = arg;
 
-static void
-dns_loadctx_enqueue(isc_loop_t *loop, dns_loadctx_t *lctx) {
-	dns_loadctx_ref(lctx);
-	if (loop == isc_loop()) {
-		load_enqueue(lctx);
-	} else {
-		isc_async_run(loop, load_enqueue, lctx);
-	}
+	isc_async_run(lctx->loop, master_load_callback, lctx);
 }
 
 isc_result_t
@@ -2634,7 +2641,10 @@ dns_master_loadfileasync(const char *master_file, dns_name_t *top,
 		return result;
 	}
 
-	dns_loadctx_enqueue(loop, lctx);
+	dns_loadctx_ref(lctx);
+	isc_loop_attach(loop, &lctx->loop);
+	isc_work_enqueue(isc_loop(), master_load, master_load_done, lctx);
+
 	*lctxp = lctx;
 
 	return ISC_R_SUCCESS;

lib/dns/masterdump.c

@@ -234,6 +234,7 @@ static char tabs[N_TABS + 1] = "\t\t\t\t\t\t\t\t\t\t";
 struct dns_dumpctx {
 	unsigned int magic;
 	isc_mem_t *mctx;
+	isc_loop_t *loop;
 	isc_mutex_t lock;
 	isc_refcount_t references;
 	atomic_bool canceled;
@@ -1450,10 +1451,20 @@ closeandrename(FILE *f, isc_result_t result, const char *temp,
 }
 
 /*
- * This will run in a libuv threadpool thread.
+ * The combination of isc_work_enqueue() on the current loop and callback on
+ * dctx->loop ensures the correct ordering:
+ *
+ * 1. dns_master_dumptostreamasync() (or dns_master_dumpasync()) calls
+ *    isc_work_enqueue() on the current loop.
+ * 2. master_dump() runs asynchronously and can finish before the entry point
+ *    returns; master_dump_done() is queued on the current loop and cannot run
+ *    until the entry point returns.
+ * 3. The entry point publishes *dctxp.
+ * 4. master_dump_done() runs on the current loop and hands off to dctx->loop.
+ * 5. dctx->done() runs on dctx->loop asynchronously.
  */
 static void
-master_dump_cb(void *data) {
+master_dump(void *data) {
 	isc_result_t result = ISC_R_UNSET;
 	dns_dumpctx_t *dctx = data;
 	REQUIRE(DNS_DCTX_VALID(dctx));
@@ -1478,17 +1489,22 @@ master_dump_cb(void *data) {
 	dctx->result = result;
 }
 
-/*
- * This will run in a loop manager thread when the dump is complete.
- */
 static void
-master_dump_done_cb(void *data) {
+master_dump_callback(void *data) {
 	dns_dumpctx_t *dctx = data;
 
 	(dctx->done)(dctx->done_arg, dctx->result);
+	isc_loop_detach(&dctx->loop);
 	dns_dumpctx_detach(&dctx);
 }
 
+static void
+master_dump_done(void *data) {
+	dns_dumpctx_t *dctx = data;
+
+	isc_async_run(dctx->loop, master_dump_callback, dctx);
+}
+
 static isc_result_t
 dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 	       const dns_master_style_t *style, FILE *f, dns_dumpctx_t **dctxp,
@@ -1707,21 +1723,6 @@ cleanup:
 	return result;
 }
 
-static void
-master_dump_enqueue(void *dctx) {
-	isc_work_enqueue(isc_loop(), master_dump_cb, master_dump_done_cb, dctx);
-}
-
-static void
-dns_dumpctx_enqueue(isc_loop_t *loop, dns_dumpctx_t *dctx) {
-	dns_dumpctx_ref(dctx);
-	if (loop == isc_loop()) {
-		master_dump_enqueue(dctx);
-	} else {
-		isc_async_run(loop, master_dump_enqueue, dctx);
-	}
-}
-
 isc_result_t
 dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db,
 			     dns_dbversion_t *version,
@@ -1739,7 +1740,10 @@ dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db,
 	dctx->done = done;
 	dctx->done_arg = done_arg;
 
-	dns_dumpctx_enqueue(loop, dctx);
+	dns_dumpctx_ref(dctx);
+	isc_loop_attach(loop, &dctx->loop);
+	isc_work_enqueue(isc_loop(), master_dump, master_dump_done, dctx);
+
 	*dctxp = dctx;
 
 	return ISC_R_SUCCESS;
@@ -1828,7 +1832,10 @@ dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 	dctx->file = file;
 	dctx->tmpfile = tempname;
 
-	dns_dumpctx_enqueue(loop, dctx);
+	dns_dumpctx_ref(dctx);
+	isc_loop_attach(loop, &dctx->loop);
+	isc_work_enqueue(isc_loop(), master_dump, master_dump_done, dctx);
+
 	*dctxp = dctx;
 
 	return ISC_R_SUCCESS;