From 7db778854e35f2dcea2a08207110dfed9366e8a5 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 31 Aug 2020 19:28:38 +1000 Subject: [PATCH] Use a reference counter for zt WARNING: ThreadSanitizer: data race Write of size 8 at 0x000000000001 by thread T1 (mutexes: write M1): #0 memset #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 Previous atomic read of size 8 at 0x000000000001 by thread T2: #0 __tsan_atomic64_load #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 Location is heap block of size 273 at 0x000000000015 allocated by thread T3: #0 malloc #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 Conflict: NA Reference: https://gitlab.isc.org/isc-projects/bind9/-/commit/7db778854e35f2dcea2a08207110dfed9366e8a5 --- lib/dns/zt.c | 63 ++++++++++++++++++++++------------------------------ 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/lib/dns/zt.c b/lib/dns/zt.c index 8098b90d1f..14d8bcd6be 100644 --- 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, ¶ms); - 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); } -- 2.23.0