Fix the data race when shutting down dns_adb

When dns_adb is shutting down, first the adb->shutting_down flag is set
and then task is created that runs shutdown_stage2() that sets the
shutdown flag on names and entries.  However, when dns_adb_createfind()
is called, only the individual shutdown flags are being checked, and the
global adb->shutting_down flag was not checked.  Because of that it was
possible for a different thread to slip in and create new find between
the dns_adb_shutdown() and dns_adb_detach(), but before the
shutdown_stage2() task is complete.  This was detected by
ThreadSanitizer as data race because the zonetable might have been
already detached by dns_view shutdown process and simultaneously
accessed by dns_adb_createfind().

This commit converts the adb->shutting_down to atomic_bool to prevent
the global adb lock when creating the find.

diff --git a/lib/dns/adb.c b/lib/dns/adb.c
index 27bce060b865d6f0d2c2f164be352204de1c16b2..0bd8c2f7ac49e937efe72bdde11efa097461d873 100644 (file)
--- a/lib/dns/adb.c
+++ b/lib/dns/adb.c
@@ -145,7 +145,7 @@ struct dns_adb {
 
        isc_event_t cevent;
        bool cevent_out;
-       bool shutting_down;
+       atomic_bool shutting_down;
        isc_eventlist_t whenshutdown;
        isc_event_t growentries;
        bool growentries_sent;
@@ -1567,7 +1567,7 @@ check_exit(dns_adb_t *adb) {
        /*
         * The caller must be holding the adb lock.
         */
-       if (adb->shutting_down) {
+       if (atomic_load(&adb->shutting_down)) {
                /*
                 * If there aren't any external references either, we're
                 * done.  Send the control event to initiate shutdown.
@@ -2542,7 +2542,7 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr,
        ISC_EVENT_INIT(&adb->cevent, sizeof(adb->cevent), 0, NULL, 0, NULL,
                       NULL, NULL, NULL, NULL);
        adb->cevent_out = false;
-       adb->shutting_down = false;
+       atomic_init(&adb->shutting_down, false);
        ISC_LIST_INIT(adb->whenshutdown);
 
        adb->nentries = nbuckets[0];
@@ -2757,7 +2757,7 @@ dns_adb_detach(dns_adb_t **adbx) {
 
        if (need_exit_check) {
                LOCK(&adb->lock);
-               INSIST(adb->shutting_down);
+               INSIST(atomic_load(&adb->shutting_down));
                check_exit(adb);
                UNLOCK(&adb->lock);
        }
@@ -2784,7 +2784,7 @@ dns_adb_whenshutdown(dns_adb_t *adb, isc_task_t *task, isc_event_t **eventp) {
 
        zeroirefcnt = (adb->irefcnt == 0);
 
-       if (adb->shutting_down && zeroirefcnt &&
+       if (atomic_load(&adb->shutting_down) && zeroirefcnt &&
            isc_refcount_current(&adb->ahrefcnt) == 0)
        {
                /*
@@ -2813,7 +2813,7 @@ shutdown_stage2(isc_task_t *task, isc_event_t *event) {
        INSIST(DNS_ADB_VALID(adb));
 
        LOCK(&adb->lock);
-       INSIST(adb->shutting_down);
+       INSIST(atomic_load(&adb->shutting_down));
        adb->cevent_out = false;
        (void)shutdown_names(adb);
        (void)shutdown_entries(adb);
@@ -2833,8 +2833,8 @@ dns_adb_shutdown(dns_adb_t *adb) {
 
        LOCK(&adb->lock);
 
-       if (!adb->shutting_down) {
-               adb->shutting_down = true;
+       if (atomic_compare_exchange_strong(&adb->shutting_down,
+                                          &(bool){ false }, true)) {
                isc_mem_clearwater(adb->mctx);
                /*
                 * Isolate shutdown_names and shutdown_entries calls.
@@ -2885,6 +2885,13 @@ dns_adb_createfind(dns_adb_t *adb, isc_task_t *task, isc_taskaction_t action,
        result = ISC_R_UNEXPECTED;
        POST(result);
 
+       if (atomic_load(&adb->shutting_down)) {
+               DP(DEF_LEVEL, "dns_adb_createfind: returning "
+                             "ISC_R_SHUTTINGDOWN");
+
+               return (ISC_R_SHUTTINGDOWN);
+       }
+
        if (now == 0) {
                isc_stdtime_get(&now);
        }