Make lib/ns Thread Sanitizer clean

diff --git a/bin/named/server.c b/bin/named/server.c
index 1e15368e3ab0524859b8f9599380f388c56031c4..e2115df9aef36512480acb2a0db1a039aaa0f793 100644 (file)
--- a/bin/named/server.c
+++ b/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);

diff --git a/lib/ns/client.c b/lib/ns/client.c
index 69400141a1e836c3067af1bc3a45b2a1c14ad7a4..e360ba1422c477445ed85621ea391feebf917370 100644 (file)
--- a/lib/ns/client.c
+++ b/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);

diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h
index 613a91f822c27724739e651c967fcabed6238765..c808064665ca806719b2e745f0b58e884b9bb17c 100644 (file)
--- a/lib/ns/include/ns/client.h
+++ b/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

diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c
index 0d932857bb644ef60b1ff8348a1e0da61b01d30a..93a59dd9eb5518ca071d983e1359231a8499ab7e 100644 (file)
--- a/lib/ns/interfacemgr.c
+++ b/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)) {
-               if (isc_sockaddr_equal(&ifp->addr, addr))
+            ifp = ISC_LIST_NEXT(ifp, link))
+       {
+               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);
-       return (false);
+       {
+               if (isc_sockaddr_equal(old, addr)) {
+                       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));
-
-       return (ISC_LIST_HEAD(mgr->interfaces));
+       LOCK(&mgr->lock);
+       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);
 }

diff --git a/lib/ns/server.c b/lib/ns/server.c
index b65629da11225d556b4a62ffa5c4b363c6ad186b..4259cea7967a436b8d442eb092f91353fbc1110f 100644 (file)
--- a/lib/ns/server.c
+++ b/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));
        }

diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c
index 1d0354ff19cca1466190d340694a0b8008fef1b4..927bcab5d3a9de6dd418bf4d6d804b63f36202db 100644 (file)
--- a/lib/ns/tests/nstest.c
+++ b/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;
-       run_managers = false;
+       atomic_store(&shutdown_done, true);
+       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);