Use rcu methods to lock access view->zonetable

dns_view_find* may be called after the final call to dns_view_detach
is made which detaches view->zonetable to permit the server to
shutdown.  We need to detect if view->zonetable is NULL during this
stage and appropriately recover.

diff --git a/lib/dns/view.c b/lib/dns/view.c
index ee2a5c9a06e5d2f0fc8b4cb34fd1ef1feb88fbd0..7349ae6f82ed2807d02501a09e3f779717a127e8 100644 (file)
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -30,6 +30,7 @@
 #include <isc/result.h>
 #include <isc/stats.h>
 #include <isc/string.h>
+#include <isc/urcu.h>
 #include <isc/util.h>
 
 #include <dns/acl.h>
@@ -133,7 +134,6 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name,
 
        isc_rwlock_init(&view->sfd_lock);
 
-       view->zonetable = NULL;
        dns_zt_create(mctx, view, &view->zonetable);
 
        result = dns_fwdtable_create(mctx, &view->fwdtable);
@@ -460,7 +460,7 @@ dns_view_detach(dns_view_t **viewp) {
 
        if (isc_refcount_decrement(&view->references) == 1) {
                dns_zone_t *mkzone = NULL, *rdzone = NULL;
-               dns_zt_t *zt = NULL;
+               dns_zt_t *zonetable = NULL;
                dns_resolver_t *resolver = NULL;
                dns_adb_t *adb = NULL;
                dns_requestmgr_t *requestmgr = NULL;
@@ -481,6 +481,15 @@ dns_view_detach(dns_view_t **viewp) {
                /* Swap the pointers under the lock */
                LOCK(&view->lock);
 
+               rcu_read_lock();
+               zonetable = rcu_xchg_pointer(&view->zonetable, NULL);
+               if (zonetable != NULL) {
+                       if (view->flush) {
+                               dns_zt_flush(zonetable);
+                       }
+               }
+               rcu_read_unlock();
+
                if (view->resolver != NULL) {
                        resolver = view->resolver;
                        view->resolver = NULL;
@@ -495,14 +504,6 @@ dns_view_detach(dns_view_t **viewp) {
                        requestmgr = view->requestmgr;
                        view->requestmgr = NULL;
                }
-
-               if (view->zonetable != NULL) {
-                       zt = view->zonetable;
-                       view->zonetable = NULL;
-                       if (view->flush) {
-                               dns_zt_flush(zt);
-                       }
-               }
                if (view->managed_keys != NULL) {
                        mkzone = view->managed_keys;
                        view->managed_keys = NULL;
@@ -537,8 +538,9 @@ dns_view_detach(dns_view_t **viewp) {
                        dns_requestmgr_detach(&requestmgr);
                }
 
-               if (zt != NULL) {
-                       dns_zt_detach(&zt);
+               if (zonetable != NULL) {
+                       synchronize_rcu();
+                       dns_zt_detach(&zonetable);
                }
                if (mkzone != NULL) {
                        dns_zone_detach(&mkzone);
@@ -560,10 +562,16 @@ dialup(dns_zone_t *zone, void *dummy) {
 
 void
 dns_view_dialup(dns_view_t *view) {
+       dns_zt_t *zonetable = NULL;
+
        REQUIRE(DNS_VIEW_VALID(view));
-       REQUIRE(view->zonetable != NULL);
 
-       (void)dns_zt_apply(view->zonetable, false, NULL, dialup, NULL);
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               (void)dns_zt_apply(zonetable, false, NULL, dialup, NULL);
+       }
+       rcu_read_unlock();
 }
 
 void
@@ -762,11 +770,19 @@ dns_view_thaw(dns_view_t *view) {
 isc_result_t
 dns_view_addzone(dns_view_t *view, dns_zone_t *zone) {
        isc_result_t result;
+       dns_zt_t *zonetable = NULL;
 
        REQUIRE(DNS_VIEW_VALID(view));
        REQUIRE(!view->frozen);
 
-       result = dns_zt_mount(view->zonetable, zone);
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               result = dns_zt_mount(zonetable, zone);
+       } else {
+               result = ISC_R_SHUTTINGDOWN;
+       }
+       rcu_read_unlock();
 
        return (result);
 }
@@ -774,9 +790,21 @@ dns_view_addzone(dns_view_t *view, dns_zone_t *zone) {
 isc_result_t
 dns_view_findzone(dns_view_t *view, const dns_name_t *name,
                  dns_zone_t **zonep) {
+       isc_result_t result;
+       dns_zt_t *zonetable = NULL;
+
        REQUIRE(DNS_VIEW_VALID(view));
 
-       return (dns_zt_find(view->zonetable, name, DNS_ZTFIND_EXACT, zonep));
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               result = dns_zt_find(zonetable, name, DNS_ZTFIND_EXACT, zonep);
+       } else {
+               result = ISC_R_NOTFOUND;
+       }
+       rcu_read_unlock();
+
+       return (result);
 }
 
 isc_result_t
@@ -791,6 +819,7 @@ dns_view_find(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type,
        bool is_cache, is_staticstub_zone;
        dns_rdataset_t zrdataset, zsigrdataset;
        dns_zone_t *zone = NULL;
+       dns_zt_t *zonetable = NULL;
 
        /*
         * Find an rdataset whose owner name is 'name', and whose type is
@@ -813,7 +842,14 @@ dns_view_find(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type,
         * Find a database to answer the query.
         */
        is_staticstub_zone = false;
-       result = dns_zt_find(view->zonetable, name, DNS_ZTFIND_MIRROR, &zone);
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               result = dns_zt_find(zonetable, name, DNS_ZTFIND_MIRROR, &zone);
+       } else {
+               result = ISC_R_SHUTTINGDOWN;
+       }
+       rcu_read_unlock();
        if (zone != NULL && dns_zone_gettype(zone) == dns_zone_staticstub &&
            !use_static_stub)
        {
@@ -1067,6 +1103,7 @@ dns_view_findzonecut(dns_view_t *view, const dns_name_t *name,
        bool is_cache, use_zone = false, try_hints = false;
        dns_zone_t *zone = NULL;
        dns_name_t *zfname = NULL;
+       dns_zt_t *zonetable = NULL;
        dns_rdataset_t zrdataset, zsigrdataset;
        dns_fixedname_t zfixedname;
        unsigned int ztoptions = DNS_ZTFIND_MIRROR;
@@ -1087,7 +1124,14 @@ dns_view_findzonecut(dns_view_t *view, const dns_name_t *name,
        if ((options & DNS_DBFIND_NOEXACT) != 0) {
                ztoptions |= DNS_ZTFIND_NOEXACT;
        }
-       result = dns_zt_find(view->zonetable, name, ztoptions, &zone);
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               result = dns_zt_find(zonetable, name, ztoptions, &zone);
+       } else {
+               result = ISC_R_SHUTTINGDOWN;
+       }
+       rcu_read_unlock();
        if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
                result = dns_zone_getdb(zone, &db);
        }
@@ -1287,11 +1331,19 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name,
        for (view = ISC_LIST_HEAD(*list); view != NULL;
             view = ISC_LIST_NEXT(view, link))
        {
+               dns_zt_t *zonetable = NULL;
                if (!allclasses && view->rdclass != rdclass) {
                        continue;
                }
-               result = dns_zt_find(view->zonetable, name, DNS_ZTFIND_EXACT,
-                                    (zone1 == NULL) ? &zone1 : &zone2);
+               rcu_read_lock();
+               zonetable = rcu_dereference(view->zonetable);
+               if (zonetable != NULL) {
+                       result = dns_zt_find(zonetable, name, DNS_ZTFIND_EXACT,
+                                            (zone1 == NULL) ? &zone1 : &zone2);
+               } else {
+                       result = ISC_R_NOTFOUND;
+               }
+               rcu_read_unlock();
                INSIST(result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND);
                if (zone2 != NULL) {
                        dns_zone_detach(&zone1);
@@ -1311,15 +1363,39 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name,
 
 isc_result_t
 dns_view_load(dns_view_t *view, bool stop, bool newonly) {
+       isc_result_t result;
+       dns_zt_t *zonetable = NULL;
+
        REQUIRE(DNS_VIEW_VALID(view));
-       return (dns_zt_load(view->zonetable, stop, newonly));
+
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               result = dns_zt_load(zonetable, stop, newonly);
+       } else {
+               result = ISC_R_SUCCESS;
+       }
+       rcu_read_unlock();
+       return (result);
 }
 
 isc_result_t
 dns_view_asyncload(dns_view_t *view, bool newonly, dns_zt_callback_t *callback,
                   void *arg) {
+       isc_result_t result;
+       dns_zt_t *zonetable = NULL;
+
        REQUIRE(DNS_VIEW_VALID(view));
-       return (dns_zt_asyncload(view->zonetable, newonly, callback, arg));
+
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               result = dns_zt_asyncload(zonetable, newonly, callback, arg);
+       } else {
+               result = ISC_R_SUCCESS;
+       }
+       rcu_read_unlock();
+       return (result);
 }
 
 isc_result_t
@@ -1455,10 +1531,21 @@ dns_view_flushnode(dns_view_t *view, const dns_name_t *name, bool tree) {
 
 isc_result_t
 dns_view_freezezones(dns_view_t *view, bool value) {
+       isc_result_t result;
+       dns_zt_t *zonetable = NULL;
+
        REQUIRE(DNS_VIEW_VALID(view));
-       REQUIRE(view->zonetable != NULL);
 
-       return (dns_zt_freezezones(view->zonetable, view, value));
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               result = dns_zt_freezezones(zonetable, view, value);
+       } else {
+               result = ISC_R_SUCCESS;
+       }
+       rcu_read_unlock();
+
+       return (result);
 }
 
 isc_result_t
@@ -2119,6 +2206,7 @@ cleanup:
 void
 dns_view_setviewcommit(dns_view_t *view) {
        dns_zone_t *redirect = NULL, *managed_keys = NULL;
+       dns_zt_t *zonetable = NULL;
 
        REQUIRE(DNS_VIEW_VALID(view));
 
@@ -2133,9 +2221,13 @@ dns_view_setviewcommit(dns_view_t *view) {
 
        UNLOCK(&view->lock);
 
-       if (view->zonetable != NULL) {
-               dns_zt_setviewcommit(view->zonetable);
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
+       if (zonetable != NULL) {
+               dns_zt_setviewcommit(zonetable);
        }
+       rcu_read_unlock();
+
        if (redirect != NULL) {
                dns_zone_setviewcommit(redirect);
                dns_zone_detach(&redirect);
@@ -2149,7 +2241,7 @@ dns_view_setviewcommit(dns_view_t *view) {
 void
 dns_view_setviewrevert(dns_view_t *view) {
        dns_zone_t *redirect = NULL, *managed_keys = NULL;
-       dns_zt_t *zonetable;
+       dns_zt_t *zonetable = NULL;
 
        REQUIRE(DNS_VIEW_VALID(view));
 
@@ -2164,7 +2256,6 @@ dns_view_setviewrevert(dns_view_t *view) {
        if (view->managed_keys != NULL) {
                dns_zone_attach(view->managed_keys, &managed_keys);
        }
-       zonetable = view->zonetable;
        UNLOCK(&view->lock);
 
        if (redirect != NULL) {
@@ -2175,9 +2266,12 @@ dns_view_setviewrevert(dns_view_t *view) {
                dns_zone_setviewrevert(managed_keys);
                dns_zone_detach(&managed_keys);
        }
+       rcu_read_lock();
+       zonetable = rcu_dereference(view->zonetable);
        if (zonetable != NULL) {
                dns_zt_setviewrevert(zonetable);
        }
+       rcu_read_unlock();
 }
 
 bool

diff --git a/tests/dns/zt_test.c b/tests/dns/zt_test.c
index c5ab32dff92eb763a9547e1031ab1d915711d86a..33232f1edbc5edad5a665b8a026567a6c8e15d46 100644 (file)
--- a/tests/dns/zt_test.c
+++ b/tests/dns/zt_test.c
@@ -28,6 +28,7 @@
 #include <isc/buffer.h>
 #include <isc/loop.h>
 #include <isc/timer.h>
+#include <isc/urcu.h>
 #include <isc/util.h>
 
 #include <dns/db.h>
@@ -56,13 +57,18 @@ count_zone(dns_zone_t *zone, void *uap) {
 ISC_LOOP_TEST_IMPL(apply) {
        isc_result_t result;
        dns_zone_t *zone = NULL;
+       dns_zt_t *zt = NULL;
        int nzones = 0;
 
        result = dns_test_makezone("foo", &zone, NULL, true);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        view = dns_zone_getview(zone);
-       assert_non_null(view->zonetable);
+       rcu_read_lock();
+       zt = rcu_dereference(view->zonetable);
+       rcu_read_unlock();
+
+       assert_non_null(zt);
 
        assert_int_equal(nzones, 0);
        result = dns_zt_apply(view->zonetable, false, NULL, count_zone,
@@ -151,6 +157,7 @@ ISC_LOOP_TEST_IMPL(asyncload_zone) {
        isc_result_t result;
        int n;
        dns_zone_t *zone = NULL;
+       dns_zt_t *zt = NULL;
        char buf[4096];
 
        result = dns_test_makezone("foo", &zone, NULL, true);
@@ -161,7 +168,10 @@ ISC_LOOP_TEST_IMPL(asyncload_zone) {
        assert_int_equal(result, ISC_R_SUCCESS);
 
        view = dns_zone_getview(zone);
-       assert_non_null(view->zonetable);
+       rcu_read_lock();
+       zt = rcu_dereference(view->zonetable);
+       rcu_read_unlock();
+       assert_non_null(zt);
 
        assert_false(dns__zone_loadpending(zone));
        zonefile = fopen("./zone.data", "wb");
@@ -239,7 +249,9 @@ ISC_LOOP_TEST_IMPL(asyncload_zt) {
        dns_zone_setfile(zone3, TESTS_DIR "/testdata/zt/nonexistent.db",
                         dns_masterformat_text, &dns_master_style_default);
 
-       zt = view->zonetable;
+       rcu_read_lock();
+       zt = rcu_dereference(view->zonetable);
+       rcu_read_unlock();
        assert_non_null(zt);
 
        dns_test_setupzonemgr();
@@ -254,7 +266,10 @@ ISC_LOOP_TEST_IMPL(asyncload_zt) {
        assert_false(dns__zone_loadpending(zone2));
        assert_false(atomic_load(&done));
 
+       rcu_read_lock();
+       zt = rcu_dereference(view->zonetable);
        dns_zt_asyncload(zt, false, all_done, NULL);
+       rcu_read_unlock();
 }
 
 ISC_TEST_LIST_START