Make lib/ns Thread Sanitizer clean

bin/named/server.c

@@ -7040,7 +7040,7 @@ tat_timer_tick(isc_task_t *task, isc_event_t *event) {
 static void
 pps_timer_tick(isc_task_t *task, isc_event_t *event) {
 	static unsigned int oldrequests = 0;
-	unsigned int requests = ns_client_requests;
+	unsigned int requests = atomic_load_relaxed(&ns_client_requests);
 
 	UNUSED(task);
 	isc_event_free(&event);

lib/ns/client.c

@@ -13,6 +13,7 @@
 #include <stdbool.h>
 
 #include <isc/aes.h>
+#include <isc/atomic.h>
 #include <isc/formatcheck.h>
 #include <isc/fuzz.h>
 #include <isc/hmac.h>
@@ -115,7 +116,11 @@
 #define NS_CLIENT_DROPPORT 1
 #endif
 
-LIBNS_EXTERNAL_DATA unsigned int ns_client_requests;
+#if defined(_WIN32) && !defined(_WIN64)
+LIBNS_EXTERNAL_DATA atomic_uint_fast32_t ns_client_requests;
+#else
+LIBNS_EXTERNAL_DATA atomic_uint_fast64_t ns_client_requests;
+#endif
 
 static void clientmgr_attach(ns_clientmgr_t *source, ns_clientmgr_t **targetp);
 static void clientmgr_detach(ns_clientmgr_t **mp);
@@ -1658,7 +1663,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 
 	INSIST(client->state == NS_CLIENTSTATE_READY);
 
-	ns_client_requests++;
+	(void)atomic_fetch_add_relaxed(&ns_client_requests, 1);
 
 	isc_buffer_init(&tbuffer, region->base, region->length);
 	isc_buffer_add(&tbuffer, region->length);

lib/ns/include/ns/client.h

@@ -57,6 +57,7 @@
 #include <inttypes.h>
 #include <stdbool.h>
 
+#include <isc/atomic.h>
 #include <isc/buffer.h>
 #include <isc/magic.h>
 #include <isc/netmgr.h>
@@ -294,7 +295,11 @@ struct ns_client {
  */
 #define NS_FAILCACHE_CD		0x01
 
-LIBNS_EXTERNAL_DATA extern unsigned int ns_client_requests;
+#if defined(_WIN32) && !defined(_WIN64)
+LIBNS_EXTERNAL_DATA extern atomic_uint_fast32_t ns_client_requests;
+#else
+LIBNS_EXTERNAL_DATA extern atomic_uint_fast64_t ns_client_requests;
+#endif
 
 /***
  *** Functions

lib/ns/interfacemgr.c

@@ -410,7 +410,9 @@ ns_interface_create(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr,
 	ISC_LINK_INIT(ifp, link);
 
 	ns_interfacemgr_attach(mgr, &ifp->mgr);
+	LOCK(&mgr->lock);
 	ISC_LIST_APPEND(mgr->interfaces, ifp, link);
+	UNLOCK(&mgr->lock);
 
 	isc_refcount_init(&ifp->references, 1);
 	ifp->magic = IFACE_MAGIC;
@@ -527,7 +529,9 @@ ns_interface_setup(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr,
 	return (result);
 
  cleanup_interface:
+	LOCK(&ifp->mgr->lock);
 	ISC_LIST_UNLINK(ifp->mgr->interfaces, ifp, link);
+	UNLOCK(&ifp->mgr->lock);
 	ns_interface_detach(&ifp);
 	return (result);
 }
@@ -549,24 +553,23 @@ ns_interface_shutdown(ns_interface_t *ifp) {
 
 static void
 ns_interface_destroy(ns_interface_t *ifp) {
-	isc_mem_t *mctx;
-	int disp;
-
 	REQUIRE(NS_INTERFACE_VALID(ifp));
 
-	mctx = ifp->mgr->mctx;
+	isc_mem_t *mctx = ifp->mgr->mctx;
 
 	ns_interface_shutdown(ifp);
 
-	for (disp = 0; disp < ifp->nudpdispatch; disp++)
+	for (int disp = 0; disp < ifp->nudpdispatch; disp++) {
 		if (ifp->udpdispatch[disp] != NULL) {
 			dns_dispatch_changeattributes(ifp->udpdispatch[disp], 0,
 						    DNS_DISPATCHATTR_NOLISTEN);
 			dns_dispatch_detach(&(ifp->udpdispatch[disp]));
 		}
+	}
 
-	if (ifp->tcpsocket != NULL)
+	if (ifp->tcpsocket != NULL) {
 		isc_socket_detach(&ifp->tcpsocket);
+	}
 
 	isc_mutex_destroy(&ifp->lock);
 
@@ -576,6 +579,7 @@ ns_interface_destroy(ns_interface_t *ifp) {
 	isc_refcount_destroy(&ifp->ntcpaccepting);
 
 	ifp->magic = 0;
+
 	isc_mem_put(mctx, ifp, sizeof(*ifp));
 }
 
@@ -604,11 +608,15 @@ ns_interface_detach(ns_interface_t **targetp) {
 static ns_interface_t *
 find_matching_interface(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr) {
 	ns_interface_t *ifp;
+	LOCK(&mgr->lock);
 	for (ifp = ISC_LIST_HEAD(mgr->interfaces); ifp != NULL;
-	     ifp = ISC_LIST_NEXT(ifp, link)) {
+	     ifp = ISC_LIST_NEXT(ifp, link))
-		if (isc_sockaddr_equal(&ifp->addr, addr))
+	{
+		if (isc_sockaddr_equal(&ifp->addr, addr)) {
 			break;
+		}
 	}
+	UNLOCK(&mgr->lock);
 	return (ifp);
 }
 
@@ -618,6 +626,7 @@ find_matching_interface(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr) {
 static void
 purge_old_interfaces(ns_interfacemgr_t *mgr) {
 	ns_interface_t *ifp, *next;
+	LOCK(&mgr->lock);
 	for (ifp = ISC_LIST_HEAD(mgr->interfaces); ifp != NULL; ifp = next) {
 		INSIST(NS_INTERFACE_VALID(ifp));
 		next = ISC_LIST_NEXT(ifp, link);
@@ -632,6 +641,7 @@ purge_old_interfaces(ns_interfacemgr_t *mgr) {
 			ns_interface_detach(&ifp);
 		}
 	}
+	UNLOCK(&mgr->lock);
 }
 
 static isc_result_t
@@ -712,28 +722,36 @@ setup_listenon(ns_interfacemgr_t *mgr, isc_interface_t *interface,
 
 	isc_sockaddr_fromnetaddr(addr, &interface->address, port);
 
+	LOCK(&mgr->lock);
 	for (old = ISC_LIST_HEAD(mgr->listenon);
 	     old != NULL;
 	     old = ISC_LIST_NEXT(old, link))
-		if (isc_sockaddr_equal(addr, old))
+	{
+		if (isc_sockaddr_equal(addr, old)) {
 			break;
+		}
+	}
 
-	if (old != NULL)
+	if (old != NULL) {
 		isc_mem_put(mgr->mctx, addr, sizeof(*addr));
-	else
+	} else {
 		ISC_LIST_APPEND(mgr->listenon, addr, link);
+	}
+	UNLOCK(&mgr->lock);
 }
 
 static void
 clearlistenon(ns_interfacemgr_t *mgr) {
 	isc_sockaddr_t *old;
 
+	LOCK(&mgr->lock);
 	old = ISC_LIST_HEAD(mgr->listenon);
 	while (old != NULL) {
 		ISC_LIST_UNLINK(mgr->listenon, old, link);
 		isc_mem_put(mgr->mctx, old, sizeof(*old));
 		old = ISC_LIST_HEAD(mgr->listenon);
 	}
+	UNLOCK(&mgr->lock);
 }
 
 static isc_result_t
@@ -1209,25 +1227,40 @@ ns_interfacemgr_listeningon(ns_interfacemgr_t *mgr,
 			    const isc_sockaddr_t *addr)
 {
 	isc_sockaddr_t *old;
+	bool result = false;
 
 	REQUIRE(NS_INTERFACEMGR_VALID(mgr));
 
+	LOCK(&mgr->lock);
 	for (old = ISC_LIST_HEAD(mgr->listenon);
 	     old != NULL;
 	     old = ISC_LIST_NEXT(old, link))
-		if (isc_sockaddr_equal(old, addr))
+	{
-			return (true);
+		if (isc_sockaddr_equal(old, addr)) {
-	return (false);
+			result = true;
+			break;
+		}
+	}
+	UNLOCK(&mgr->lock);
+
+	return (result);
 }
 
 ns_interface_t *
 ns__interfacemgr_getif(ns_interfacemgr_t *mgr) {
+	ns_interface_t *head;
 	REQUIRE(NS_INTERFACEMGR_VALID(mgr));
-
+	LOCK(&mgr->lock);
-	return (ISC_LIST_HEAD(mgr->interfaces));
+	head = ISC_LIST_HEAD(mgr->interfaces);
+	UNLOCK(&mgr->lock);
+	return (head);
 }
 
 ns_interface_t *
 ns__interfacemgr_nextif(ns_interface_t *ifp) {
-	return (ISC_LIST_NEXT(ifp, link));
+	ns_interface_t *next;
+	LOCK(&ifp->lock);
+	next = ISC_LIST_NEXT(ifp, link);
+	UNLOCK(&ifp->lock);
+	return (next);
 }

lib/ns/server.c

@@ -131,8 +131,6 @@ ns_server_detach(ns_server_t **sctxp) {
 	if (isc_refcount_decrement(&sctx->references) == 1) {
 		ns_altsecret_t *altsecret;
 
-		sctx->magic = 0;
-
 		while ((altsecret = ISC_LIST_HEAD(sctx->altsecrets)) != NULL) {
 			ISC_LIST_UNLINK(sctx->altsecrets, altsecret, link);
 			isc_mem_put(sctx->mctx, altsecret, sizeof(*altsecret));
@@ -142,43 +140,61 @@ ns_server_detach(ns_server_t **sctxp) {
 		isc_quota_destroy(&sctx->tcpquota);
 		isc_quota_destroy(&sctx->xfroutquota);
 
-		if (sctx->server_id != NULL)
+		if (sctx->server_id != NULL) {
 			isc_mem_free(sctx->mctx, sctx->server_id);
+		}
 
-		if (sctx->blackholeacl != NULL)
+		if (sctx->blackholeacl != NULL) {
 			dns_acl_detach(&sctx->blackholeacl);
-		if (sctx->keepresporder != NULL)
+		}
+		if (sctx->keepresporder != NULL) {
 			dns_acl_detach(&sctx->keepresporder);
-		if (sctx->tkeyctx != NULL)
+		}
+		if (sctx->tkeyctx != NULL) {
 			dns_tkeyctx_destroy(&sctx->tkeyctx);
+		}
 
-		if (sctx->nsstats != NULL)
+		if (sctx->nsstats != NULL) {
 			ns_stats_detach(&sctx->nsstats);
+		}
 
-		if (sctx->rcvquerystats != NULL)
+		if (sctx->rcvquerystats != NULL) {
 			dns_stats_detach(&sctx->rcvquerystats);
-		if (sctx->opcodestats != NULL)
+		}
+		if (sctx->opcodestats != NULL) {
 			dns_stats_detach(&sctx->opcodestats);
-		if (sctx->rcodestats != NULL)
+		}
+		if (sctx->rcodestats != NULL) {
 			dns_stats_detach(&sctx->rcodestats);
+		}
 
-		if (sctx->udpinstats4 != NULL)
+		if (sctx->udpinstats4 != NULL) {
 			isc_stats_detach(&sctx->udpinstats4);
-		if (sctx->tcpinstats4 != NULL)
+		}
+		if (sctx->tcpinstats4 != NULL) {
 			isc_stats_detach(&sctx->tcpinstats4);
-		if (sctx->udpoutstats4 != NULL)
+		}
+		if (sctx->udpoutstats4 != NULL) {
 			isc_stats_detach(&sctx->udpoutstats4);
-		if (sctx->tcpoutstats4 != NULL)
+		}
+		if (sctx->tcpoutstats4 != NULL) {
 			isc_stats_detach(&sctx->tcpoutstats4);
+		}
 
-		if (sctx->udpinstats6 != NULL)
+		if (sctx->udpinstats6 != NULL) {
 			isc_stats_detach(&sctx->udpinstats6);
-		if (sctx->tcpinstats6 != NULL)
+		}
+		if (sctx->tcpinstats6 != NULL) {
 			isc_stats_detach(&sctx->tcpinstats6);
-		if (sctx->udpoutstats6 != NULL)
+		}
+		if (sctx->udpoutstats6 != NULL) {
 			isc_stats_detach(&sctx->udpoutstats6);
-		if (sctx->tcpoutstats6 != NULL)
+		}
+		if (sctx->tcpoutstats6 != NULL) {
 			isc_stats_detach(&sctx->tcpoutstats6);
+		}
+
+		sctx->magic = 0;
 
 		isc_mem_putanddetach(&sctx->mctx, sctx, sizeof(*sctx));
 	}

lib/ns/tests/nstest.c

@@ -65,7 +65,7 @@ ns_server_t *sctx = NULL;
 bool app_running = false;
 int ncpus;
 bool debug_mem_record = true;
-bool run_managers = false;
+static atomic_bool run_managers = ATOMIC_VAR_INIT(false);
 
 static bool dst_active = false;
 static bool test_running = false;
@@ -136,7 +136,7 @@ matchview(isc_netaddr_t *srcaddr, isc_netaddr_t *destaddr,
 /*
  * These need to be shut down from a running task.
  */
-bool shutdown_done = false;
+static atomic_bool shutdown_done = ATOMIC_VAR_INIT(false);
 static void
 shutdown_managers(isc_task_t *task, isc_event_t *event) {
 	UNUSED(task);
@@ -150,22 +150,22 @@ shutdown_managers(isc_task_t *task, isc_event_t *event) {
 		dns_dispatchmgr_destroy(&dispatchmgr);
 	}
 
-	shutdown_done = true;
+	atomic_store(&shutdown_done, true);
-	run_managers = false;
+	atomic_store(&run_managers, false);
 
 	isc_event_free(&event);
 }
 
 static void
 cleanup_managers(void) {
-	shutdown_done = false;
+	atomic_store(&shutdown_done, false);
 
 	if (maintask != NULL) {
 		isc_task_shutdown(maintask);
 		isc_task_destroy(&maintask);
 	}
 
-	while (run_managers && !shutdown_done) {
+	while (atomic_load(&run_managers) && !atomic_load(&shutdown_done)) {
 		/*
 		 * There's no straightforward way to determine
 		 * whether all the clients have shut down, so
@@ -256,7 +256,7 @@ create_managers(void) {
 	ns_interface_t *ifp = ns__interfacemgr_getif(interfacemgr);
 	clientmgr = ifp->clientmgr;
 
-	run_managers = true;
+	atomic_store(&run_managers, true);
 
 	return (ISC_R_SUCCESS);