Use a reference counter for zt

WARNING: ThreadSanitizer: data race
  Write of size 8 at 0x000000000001 by thread T1 (mutexes: write M1):
    #0 memset <null>
    #1 mem_put lib/isc/mem.c:819
    #2 isc___mem_free lib/isc/mem.c:1662
    #3 isc__mem_free lib/isc/mem.c:3078
    #4 isc___mem_putanddetach lib/isc/mem.c:1221
    #5 isc__mem_putanddetach lib/isc/mem.c:3033
    #6 zt_destroy lib/dns/zt.c:214
    #7 doneloading lib/dns/zt.c:591
    #8 zone_asyncload lib/dns/zone.c:2243
    #9 dispatch lib/isc/task.c:1157
    #10 run lib/isc/task.c:1331
    #11 <null> <null>

  Previous atomic read of size 8 at 0x000000000001 by thread T2:
    #0 __tsan_atomic64_load <null>
    #1 isc_rwlock_unlock lib/isc/rwlock.c:612
    #2 doneloading lib/dns/zt.c:585
    #3 zone_asyncload lib/dns/zone.c:2243
    #4 dispatch lib/isc/task.c:1157
    #5 run lib/isc/task.c:1331
    #6 <null> <null>

  Location is heap block of size 273 at 0x000000000015 allocated by thread T3:
    #0 malloc <null>
    #1 internal_memalloc lib/isc/mem.c:887
    #2 mem_get lib/isc/mem.c:792
    #3 mem_allocateunlocked lib/isc/mem.c:1545
    #4 isc___mem_allocate lib/isc/mem.c:1566
    #5 isc__mem_allocate lib/isc/mem.c:3048
    #6 isc___mem_get lib/isc/mem.c:1304
    #7 isc__mem_get lib/isc/mem.c:3012
    #8 dns_zt_create lib/dns/zt.c:85
    #9 dns_view_create lib/dns/view.c:126
    #10 create_view server.c:5312
    #11 load_configuration server.c:8101
    #12 loadconfig server.c:9428
    #13 ns_server_reconfigcommand server.c:9763
    #14 ns_control_docommand bin/named/control.c:243
    #15 control_recvmessage bin/named/controlconf.c:465
    #16 dispatch lib/isc/task.c:1157
    #17 run lib/isc/task.c:1331
    #18 <null> <null>

diff --git a/lib/dns/zt.c b/lib/dns/zt.c
index 8098b90d1fa143c48dbce62cdbed11ef25957d01..14d8bcd6be3471e304a8a75f43c3d12b8432aa62 100644 (file)
--- a/lib/dns/zt.c
+++ b/lib/dns/zt.c
@@ -41,9 +41,10 @@ struct dns_zt {
        isc_rwlock_t            rwlock;
        dns_zt_allloaded_t      loaddone;
        void *                  loaddone_arg;
+       isc_refcount_t          references;
+
        /* Locked by lock. */
-       bool            flush;
-       uint32_t                references;
+       bool                    flush;
        unsigned int            loads_pending;
        dns_rbt_t               *table;
 };
@@ -97,7 +98,7 @@ dns_zt_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, dns_zt_t **ztp) {
 
        zt->mctx = NULL;
        isc_mem_attach(mctx, &zt->mctx);
-       zt->references = 1;
+       isc_refcount_init(&zt->references, 1);
        zt->flush = false;
        zt->rdclass = rdclass;
        zt->magic = ZTMAGIC;
@@ -187,13 +188,7 @@ dns_zt_attach(dns_zt_t *zt, dns_zt_t **ztp) {
        REQUIRE(VALID_ZT(zt));
        REQUIRE(ztp != NULL && *ztp == NULL);
 
-       RWLOCK(&zt->rwlock, isc_rwlocktype_write);
-
-       INSIST(zt->references > 0);
-       zt->references++;
-       INSIST(zt->references != 0);
-
-       RWUNLOCK(&zt->rwlock, isc_rwlocktype_write);
+       isc_refcount_increment(&zt->references, NULL);
 
        *ztp = zt;
 }
@@ -206,8 +201,10 @@ flush(dns_zone_t *zone, void *uap) {
 
 static void
 zt_destroy(dns_zt_t *zt) {
-       if (zt->flush)
+       if (zt->flush) {
                (void)dns_zt_apply(zt, false, flush, NULL);
+       }
+       isc_refcount_destroy(&zt->references);
        dns_rbt_destroy(&zt->table);
        isc_rwlock_destroy(&zt->rwlock);
        zt->magic = 0;
@@ -216,28 +213,24 @@ zt_destroy(dns_zt_t *zt) {
 
 static void
 zt_flushanddetach(dns_zt_t **ztp, bool need_flush) {
-       bool destroy = false;
+       unsigned int refs;
        dns_zt_t *zt;
 
        REQUIRE(ztp != NULL && VALID_ZT(*ztp));
 
        zt = *ztp;
+       *ztp = NULL;
 
-       RWLOCK(&zt->rwlock, isc_rwlocktype_write);
-
-       INSIST(zt->references > 0);
-       zt->references--;
-       if (zt->references == 0)
-               destroy = true;
-       if (need_flush)
+       if (need_flush) {
+               RWLOCK(&zt->rwlock, isc_rwlocktype_write);
                zt->flush = true;
+               RWUNLOCK(&zt->rwlock, isc_rwlocktype_write);
+       }
 
-       RWUNLOCK(&zt->rwlock, isc_rwlocktype_write);
-
-       if (destroy)
+       isc_refcount_decrement(&zt->references, &refs);
+       if (refs == 0) {
                zt_destroy(zt);
-
-       *ztp = NULL;
+       }
 }
 
 void
@@ -301,13 +294,11 @@ dns_zt_asyncload2(dns_zt_t *zt, dns_zt_allloaded_t alldone, void *arg,
         */
        zt->loads_pending++;
        result = dns_zt_apply2(zt, false, NULL, asyncload, &params);
-
        pending = --zt->loads_pending;
        if (pending != 0) {
                zt->loaddone = alldone;
                zt->loaddone_arg = arg;
        }
-
        RWUNLOCK(&zt->rwlock, isc_rwlocktype_write);
 
        if (pending == 0)
@@ -329,18 +320,18 @@ asyncload(dns_zone_t *zone, void *paramsv) {
        dns_zt_t *zt;
 
        REQUIRE(zone != NULL);
-       zt = dns_zone_getview(zone)->zonetable;
+       zt = params->zt;
        INSIST(VALID_ZT(zt));
 
-       INSIST(zt->references > 0);
-       zt->references++;
+       isc_refcount_increment(&zt->references, NULL);
        zt->loads_pending++;
 
        result = dns_zone_asyncload2(zone, *params->dl, zt, params->newonly);
        if (result != ISC_R_SUCCESS) {
-               zt->references--;
+               unsigned int refs;
                zt->loads_pending--;
-               INSIST(zt->references > 0);
+               isc_refcount_decrement(&zt->references, &refs);
+               INSIST(refs > 0);
        }
        return (ISC_R_SUCCESS);
 }
@@ -560,7 +551,7 @@ dns_zt_apply2(dns_zt_t *zt, bool stop, isc_result_t *sub,
  */
 static isc_result_t
 doneloading(dns_zt_t *zt, dns_zone_t *zone, isc_task_t *task) {
-       bool destroy = false;
+       unsigned int refs;
        dns_zt_allloaded_t alldone = NULL;
        void *arg = NULL;
 
@@ -571,10 +562,6 @@ doneloading(dns_zt_t *zt, dns_zone_t *zone, isc_task_t *task) {
 
        RWLOCK(&zt->rwlock, isc_rwlocktype_write);
        INSIST(zt->loads_pending != 0);
-       INSIST(zt->references != 0);
-       zt->references--;
-       if (zt->references == 0)
-               destroy = true;
        zt->loads_pending--;
        if (zt->loads_pending == 0) {
                alldone = zt->loaddone;
@@ -587,8 +574,10 @@ doneloading(dns_zt_t *zt, dns_zone_t *zone, isc_task_t *task) {
        if (alldone != NULL)
                alldone(arg);
 
-       if (destroy)
+       isc_refcount_decrement(&zt->references, &refs);
+       if (refs == 0) {
                zt_destroy(zt);
+       }
 
        return (ISC_R_SUCCESS);
 }