From: Mark Andrews <marka@isc.org>
Date: Thu, 27 Aug 2020 08:26:57 +0000 (+1000)
Subject: Don't grab all the adb locks at once
X-Git-Tag: v9.11.24~30^2~11
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=9e8e886fd6dc6eef92cc5f66672195ace405b219;p=thirdparty%2Fbind9.git

Don't grab all the adb locks at once

FATAL: ThreadSanitizer CHECK failed: /build/llvm-toolchain-10-10.0.1~++20200708124224+ef32c611aa2/compiler-rt/lib/sanitizer_common/sanitizer_deadlock_detector.h:67 "((n_all_locks_)) < (((sizeof(all_locks_with_contexts_)/sizeof((all_locks_with_contexts_)[0]))))" (0x40, 0x40)
    #0 __tsan::TsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) <null> (named+0x4ad775)
    #1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) <null> (named+0x4c429f)
    #2 __sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::addLock(unsigned long, unsigned long, unsigned int) <null> (named+0x4b8a3c)
    #3 __sanitizer::DD::MutexAfterLock(__sanitizer::DDCallback*, __sanitizer::DDMutex*, bool, bool) <null> (named+0x4b7fae)
    #4 __tsan::MutexPostLock(__tsan::ThreadState*, unsigned long, unsigned long, unsigned int, int) <null> (named+0x4abc58)
    #5 pthread_mutex_lock <null> (named+0x4643a8)
    #6 dump_adb /builds/isc-projects/bind9/lib/dns/adb.c:3461:3 (libdns.so.1110+0x48be6)
    #7 dns_adb_dump /builds/isc-projects/bind9/lib/dns/adb.c:3434:2 (libdns.so.1110+0x48738)
    #8 dumpdone /builds/isc-projects/bind9/bin/named/./server.c:10149:4 (named+0x512022)
    #9 dump_quantum /builds/isc-projects/bind9/lib/dns/masterdump.c:1424:2 (libdns.so.1110+0x9f253)
    #10 dispatch /builds/isc-projects/bind9/lib/isc/task.c:1157:7 (libisc.so.1107+0x50845)
    #11 run /builds/isc-projects/bind9/lib/isc/task.c:1331:2 (libisc.so.1107+0x4d799)
    #12 __tsan_thread_start_func <null> (named+0x446e3c)
    #13 start_thread /build/glibc-vjB4T1/glibc-2.28/nptl/pthread_create.c:486:8 (libpthread.so.0+0x7fa2)
    #14 clone /build/glibc-vjB4T1/glibc-2.28/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (libc.so.6+0xf94ce)
---

diff --git a/lib/dns/adb.c b/lib/dns/adb.c
index d6f56fa6d39..982aba4cbf9 100644
--- a/lib/dns/adb.c
+++ b/lib/dns/adb.c
@@ -3457,18 +3457,16 @@ dump_adb(dns_adb_t *adb, FILE *f, bool debug, isc_stdtime_t now) {
 			adb, adb->erefcnt, adb->irefcnt,
 			isc_mempool_getallocated(adb->nhmp));
 
-	for (i = 0; i < adb->nnames; i++)
-		LOCK(&adb->namelocks[i]);
-	for (i = 0; i < adb->nentries; i++)
-		LOCK(&adb->entrylocks[i]);
-
 	/*
 	 * Dump the names
 	 */
 	for (i = 0; i < adb->nnames; i++) {
+		LOCK(&adb->namelocks[i]);
 		name = ISC_LIST_HEAD(adb->names[i]);
-		if (name == NULL)
+		if (name == NULL) {
+			UNLOCK(&adb->namelocks[i]);
 			continue;
+		}
 		if (debug)
 			fprintf(f, "; bucket %u\n", i);
 		for (;
@@ -3506,26 +3504,21 @@ dump_adb(dns_adb_t *adb, FILE *f, bool debug, isc_stdtime_t now) {
 				print_find_list(f, name);
 			}
 		}
+		UNLOCK(&adb->namelocks[i]);
 	}
 
 	fprintf(f, ";\n; Unassociated entries\n;\n");
 
 	for (i = 0; i < adb->nentries; i++) {
+		LOCK(&adb->entrylocks[i]);
 		entry = ISC_LIST_HEAD(adb->entries[i]);
 		while (entry != NULL) {
 			if (entry->nh == 0)
 				dump_entry(f, adb, entry, debug, now);
 			entry = ISC_LIST_NEXT(entry, plink);
 		}
-	}
-
-	/*
-	 * Unlock everything
-	 */
-	for (i = 0; i < adb->nentries; i++)
 		UNLOCK(&adb->entrylocks[i]);
-	for (i = 0; i < adb->nnames; i++)
-		UNLOCK(&adb->namelocks[i]);
+	}
 }
 
 static void
@@ -3644,6 +3637,7 @@ print_namehook_list(FILE *f, const char *legend,
 		    dns_adb_t *adb, dns_adbnamehooklist_t *list,
 		    bool debug, isc_stdtime_t now)
 {
+	int addr_bucket = DNS_ADB_INVALIDBUCKET;
 	dns_adbnamehook_t *nh;
 
 	for (nh = ISC_LIST_HEAD(*list);
@@ -3652,8 +3646,19 @@ print_namehook_list(FILE *f, const char *legend,
 	{
 		if (debug)
 			fprintf(f, ";\tHook(%s) %p\n", legend, nh);
+		if (addr_bucket != nh->entry->lock_bucket) {
+			if (addr_bucket != DNS_ADB_INVALIDBUCKET) {
+				UNLOCK(&adb->entrylocks[addr_bucket]);
+			}
+			addr_bucket = nh->entry->lock_bucket;
+			INSIST(addr_bucket != DNS_ADB_INVALIDBUCKET);
+			LOCK(&adb->entrylocks[addr_bucket]);
+		}
 		dump_entry(f, adb, nh->entry, debug, now);
 	}
+	if (addr_bucket != DNS_ADB_INVALIDBUCKET) {
+		UNLOCK(&adb->entrylocks[addr_bucket]);
+	}
 }
 
 static inline void