use separate barriers for "stop" and "listen" operations

On some platforms, when a synchronizing barrier is cleared, one
thread can progress while other threads are still in the process
of releasing the barrier. If a barrier is reused by the progressing
thread during this window, it can cause a deadlock. This can occur if,
for example, we stop listening immediately after we start, because the
stop and listen functions both use socket->barrier.  This has been
addressed by using separate barrier objects for stop and listen.

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index e2ac08fe04bc0da00b7a5e2c2728236260ad2c51..f2c1155b994d111ea0a5ebc95d8916f1255e9300 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -844,7 +844,8 @@ struct isc_nmsocket {
        isc__networker_t *worker;
 
        isc_mutex_t lock;
-       isc_barrier_t barrier;
+       isc_barrier_t listen_barrier;
+       isc_barrier_t stop_barrier;
 
        /*% Parent socket for multithreaded listeners */
        isc_nmsocket_t *parent;
@@ -1027,7 +1028,7 @@ struct isc_nmsocket {
 
        atomic_int_fast32_t active_child_connections;
 
-       bool barrier_initialised;
+       bool barriers_initialised;
        bool manual_read_timer;
 #ifdef NETMGR_TRACE
        void *backtrace[TRACE_SIZE];

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index ab48c3cdee7530f76fa9bf00f0db1add10463dbb..68ab0bca1e3a849767d56d828b837d0e5cc0ec5f 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -676,8 +676,9 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) {
 #endif
        isc__nm_streamdns_cleanup_data(sock);
 
-       if (sock->barrier_initialised) {
-               isc_barrier_destroy(&sock->barrier);
+       if (sock->barriers_initialised) {
+               isc_barrier_destroy(&sock->listen_barrier);
+               isc_barrier_destroy(&sock->stop_barrier);
        }
 
        sock->magic = 0;
@@ -2021,8 +2022,9 @@ isc__nmsocket_stop(isc_nmsocket_t *listener) {
 void
 isc__nmsocket_barrier_init(isc_nmsocket_t *listener) {
        REQUIRE(listener->nchildren > 0);
-       isc_barrier_init(&listener->barrier, listener->nchildren);
-       listener->barrier_initialised = true;
+       isc_barrier_init(&listener->listen_barrier, listener->nchildren);
+       isc_barrier_init(&listener->stop_barrier, listener->nchildren);
+       listener->barriers_initialised = true;
 }
 
 void
@@ -2032,7 +2034,7 @@ isc__nm_async_sockstop(isc__networker_t *worker, isc__netievent_t *ev0) {
        UNUSED(worker);
 
        (void)atomic_fetch_sub(&listener->rchildren, 1);
-       isc_barrier_wait(&listener->barrier);
+       isc_barrier_wait(&listener->stop_barrier);
 }
 
 void

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index cd49da70b40eaea379a7cf88d5ff6290dedd4edd..f427279d4c4cf60c65dc53a85393b5f88369e305 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -556,7 +556,7 @@ done:
        UNLOCK(&sock->parent->lock);
 
        REQUIRE(!worker->loop->paused);
-       isc_barrier_wait(&sock->parent->barrier);
+       isc_barrier_wait(&sock->parent->listen_barrier);
 }
 
 static void
@@ -691,7 +691,7 @@ isc__nm_async_tcpstop(isc__networker_t *worker, isc__netievent_t *ev0) {
        (void)atomic_fetch_sub(&sock->parent->rchildren, 1);
 
        REQUIRE(!worker->loop->paused);
-       isc_barrier_wait(&sock->parent->barrier);
+       isc_barrier_wait(&sock->parent->stop_barrier);
 }
 
 void

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 7101a8f56dd854bbe847396847fab2fde33b2988..b06cd2bf6006ce9d2b0adcd6a6c77bfec3bdb262 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -427,7 +427,7 @@ done:
        UNLOCK(&sock->parent->lock);
 
        REQUIRE(!worker->loop->paused);
-       isc_barrier_wait(&sock->parent->barrier);
+       isc_barrier_wait(&sock->parent->listen_barrier);
 }
 
 static void
@@ -496,7 +496,7 @@ isc__nm_async_udpstop(isc__networker_t *worker, isc__netievent_t *ev0) {
        (void)atomic_fetch_sub(&sock->parent->rchildren, 1);
 
        REQUIRE(!worker->loop->paused);
-       isc_barrier_wait(&sock->parent->barrier);
+       isc_barrier_wait(&sock->parent->stop_barrier);
 }
 
 /*