From: Ondřej Surý Date: Thu, 4 Jul 2019 13:45:06 +0000 (+0200) Subject: Make lib/ns Thread Sanitizer clean X-Git-Tag: v9.15.6~3^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e95af30b231b289a1913b735d8955fdb93bb1ad9;p=thirdparty%2Fbind9.git Make lib/ns Thread Sanitizer clean --- diff --git a/bin/named/server.c b/bin/named/server.c index 1e15368e3ab..e2115df9aef 100644 --- 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 69400141a1e..e360ba1422c 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -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 613a91f822c..c808064665c 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -57,6 +57,7 @@ #include #include +#include #include #include #include @@ -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 0d932857bb6..93a59dd9eb5 100644 --- 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 b65629da112..4259cea7967 100644 --- 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 1d0354ff19c..927bcab5d3a 100644 --- 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);