Run RPZ and catalog zones tasks in exclusive mode

All the heavy RPZ and CATZ work is already running with offloaded
threads, and running the remaining small functions in exclusive mode
offers more synchronization guaranties.

Move the update notify registration code from the offloaded
dns__catz_update_cb() function into dns__catz_done_cb().

After this change, it should be safe to remove the lock/unlock code
from the dns_catz_dbupdate_register() and dns_catz_dbupdate_unregister()
functions, as they were causing a benign TSAN lock-order-inversion
report.

diff --git a/lib/dns/catz.c b/lib/dns/catz.c
index 70b48fdc2c5dde8d9f4b229be3c4a18587a7763c..f790cdc0cf82781e6c59ecfb2f03f92eb843a23b 100644 (file)
--- a/lib/dns/catz.c
+++ b/lib/dns/catz.c
@@ -25,6 +25,7 @@
 #include <isc/print.h>
 #include <isc/result.h>
 #include <isc/task.h>
+#include <isc/thread.h>
 #include <isc/util.h>
 
 #include <dns/catz.h>
@@ -790,7 +791,7 @@ dns_catz_new_zones(isc_mem_t *mctx, isc_taskmgr_t *taskmgr,
                                     .zmm = zmm,
                                     .magic = DNS_CATZ_ZONES_MAGIC };
 
-       result = isc_task_create_bound(taskmgr, 0, &catzs->updater, 0);
+       result = isc_taskmgr_excltask(taskmgr, &catzs->updater);
        if (result != ISC_R_SUCCESS) {
                goto cleanup_task;
        }
@@ -1019,7 +1020,7 @@ dns__catz_zones_destroy(dns_catz_zones_t *catzs) {
        REQUIRE(catzs->zones == NULL);
 
        catzs->magic = 0;
-       isc_task_destroy(&catzs->updater);
+       isc_task_detach(&catzs->updater);
        isc_mutex_destroy(&catzs->lock);
        isc_refcount_destroy(&catzs->references);
 
@@ -2227,9 +2228,7 @@ dns_catz_dbupdate_unregister(dns_db_t *db, dns_catz_zones_t *catzs) {
        REQUIRE(DNS_DB_VALID(db));
        REQUIRE(DNS_CATZ_ZONES_VALID(catzs));
 
-       LOCK(&catzs->lock);
        dns_db_updatenotify_unregister(db, dns_catz_dbupdate_callback, catzs);
-       UNLOCK(&catzs->lock);
 }
 
 void
@@ -2237,9 +2236,7 @@ dns_catz_dbupdate_register(dns_db_t *db, dns_catz_zones_t *catzs) {
        REQUIRE(DNS_DB_VALID(db));
        REQUIRE(DNS_CATZ_ZONES_VALID(catzs));
 
-       LOCK(&catzs->lock);
        dns_db_updatenotify_register(db, dns_catz_dbupdate_callback, catzs);
-       UNLOCK(&catzs->lock);
 }
 
 static bool
@@ -2305,6 +2302,8 @@ dns__catz_update_cb(void *data) {
                goto exit;
        }
 
+       INSIST(catz == oldcatz);
+
        if (!is_active) {
                /* This can happen during a reconfiguration. */
                isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
@@ -2540,22 +2539,6 @@ final:
                      ISC_LOG_DEBUG(3),
                      "catz: update_from_db: new zone merged");
 
-       /*
-        * When we're doing reconfig and setting a new catalog zone
-        * from an existing zone we won't have a chance to set up
-        * update callback in zone_startload or axfr_makedb, but we will
-        * call onupdate() artificially so we can register the callback here.
-        */
-       LOCK(&catzs->lock);
-       if (!oldcatz->db_registered) {
-               result = dns_db_updatenotify_register(
-                       updb, dns_catz_dbupdate_callback, oldcatz->catzs);
-               if (result == ISC_R_SUCCESS) {
-                       oldcatz->db_registered = true;
-               }
-       }
-       UNLOCK(&catzs->lock);
-
 exit:
        catz->updateresult = result;
 }
@@ -2576,6 +2559,21 @@ dns__catz_done_cb(void *data, isc_result_t result) {
 
        dns_name_format(&catz->name, dname, DNS_NAME_FORMATSIZE);
 
+       /*
+        * When we're doing reconfig and setting a new catalog zone
+        * from an existing zone we won't have a chance to set up
+        * update callback in zone_startload or axfr_makedb, but we will
+        * call onupdate() artificially so we can register the callback
+        * here.
+        */
+       if (result == ISC_R_SUCCESS && !catz->db_registered) {
+               result = dns_db_updatenotify_register(
+                       catz->db, dns_catz_dbupdate_callback, catz->catzs);
+               if (result == ISC_R_SUCCESS) {
+                       catz->db_registered = true;
+               }
+       }
+
        /* If there's no update pending, or if shutting down, finish. */
        if (!catz->updatepending || atomic_load(&catz->catzs->shuttingdown)) {
                goto done;

diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c
index a29b6f8e3d568b1fb189d851291f8ce9ab4dad94..62d13d3a6acd52a18130fdbdc9e78e8358154ea3 100644 (file)
--- a/lib/dns/rpz.c
+++ b/lib/dns/rpz.c
@@ -1474,7 +1474,7 @@ dns_rpz_new_zones(isc_mem_t *mctx, isc_taskmgr_t *taskmgr,
                goto cleanup_rbt;
        }
 
-       result = isc_task_create_bound(taskmgr, 0, &rpzs->updater, 0);
+       result = isc_taskmgr_excltask(taskmgr, &rpzs->updater);
        if (result != ISC_R_SUCCESS) {
                goto cleanup_task;
        }
@@ -2120,7 +2120,7 @@ dns__rpz_zones_destroy(dns_rpz_zones_t *rpzs) {
        if (rpzs->rbt != NULL) {
                dns_rbt_destroy(&rpzs->rbt);
        }
-       isc_task_destroy(&rpzs->updater);
+       isc_task_detach(&rpzs->updater);
        isc_mutex_destroy(&rpzs->maint_lock);
        isc_rwlock_destroy(&rpzs->search_lock);
        isc_mem_putanddetach(&rpzs->mctx, rpzs, sizeof(*rpzs));