ensure interlocked netmgr events run on worker[0]

Network manager events that require interlock (pause, resume, listen)
are now always executed in the same worker thread, mgr->workers[0],
to prevent races.

"stoplistening" events no longer require interlock.

diff --git a/bin/named/server.c b/bin/named/server.c
index ea343340ad11abe546bae7f7225f3eb9463e079d..d411647c443776a628a4ea3ab73da6d6414b07ef 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -10201,7 +10201,7 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) {
         * startup and shutdown of the server, as well as all exclusive
         * tasks.
         */
-       CHECKFATAL(isc_task_create(named_g_taskmgr, 0, &server->task),
+       CHECKFATAL(isc_task_create_bound(named_g_taskmgr, 0, &server->task, 0),
                   "creating server task");
        isc_task_setname(server->task, "server", server);
        isc_taskmgr_setexcltask(named_g_taskmgr, server->task);

diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c
index e64a018e5dec5bd3220afbe42588239f1e982040..b76c0b263ba612e3ca81209ebe2c3dc977661ebc 100644 (file)
--- a/lib/isc/netmgr/http.c
+++ b/lib/isc/netmgr/http.c
@@ -2171,7 +2171,7 @@ isc_nm_listenhttp(isc_nm_t *mgr, isc_nmiface_t *iface, int backlog,
 
        sock->nchildren = sock->outer->nchildren;
        sock->result = ISC_R_UNSET;
-       sock->tid = isc_random_uniform(sock->nchildren);
+       sock->tid = 0;
        sock->fd = (uv_os_sock_t)-1;
 
        atomic_store(&sock->listening, true);
@@ -2254,6 +2254,7 @@ isc__nm_http_stoplistening(isc_nmsocket_t *sock) {
                isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
                                       (isc__netievent_t *)ievent);
        } else {
+               REQUIRE(isc_nm_tid() == sock->tid);
                isc__netievent_httpstop_t ievent = { .sock = sock };
                isc__nm_async_httpstop(NULL, (isc__netievent_t *)&ievent);
        }

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 05bbae5b61462783414200d7c1b057c6209f5802..2baa4419f90b57baa9f817cf513f4b1dce6ee457 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -437,6 +437,10 @@ isc_nm_pause(isc_nm_t *mgr) {
 
        isc__nm_acquire_interlocked_force(mgr);
 
+       if (isc__nm_in_netthread()) {
+               REQUIRE(isc_nm_tid() == 0);
+       }
+
        for (int i = 0; i < mgr->nworkers; i++) {
                isc__networker_t *worker = &mgr->workers[i];
                if (i == isc_nm_tid()) {
@@ -447,8 +451,8 @@ isc_nm_pause(isc_nm_t *mgr) {
        }
 
        if (isc__nm_in_netthread()) {
+               atomic_fetch_add(&mgr->workers_paused, 1);
                isc_barrier_wait(&mgr->pausing);
-               drain_priority_queue(&mgr->workers[isc_nm_tid()]);
        }
 
        LOCK(&mgr->lock);
@@ -481,6 +485,11 @@ isc_nm_resume(isc_nm_t *mgr) {
        REQUIRE(VALID_NM(mgr));
        REQUIRE(atomic_load(&mgr->paused));
 
+       if (isc__nm_in_netthread()) {
+               REQUIRE(isc_nm_tid() == 0);
+               drain_priority_queue(&mgr->workers[isc_nm_tid()]);
+       }
+
        for (int i = 0; i < mgr->nworkers; i++) {
                isc__networker_t *worker = &mgr->workers[i];
                if (i == isc_nm_tid()) {

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 052030bf79f8fce9e96697b776d999f49ba69b90..c7030eccccfc67894911a9e2f875acb0efffc8e8 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -409,6 +409,14 @@ start_tcp_child(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nmsocket_t *sock,
                                     (isc__netievent_t *)ievent);
 }
 
+static void
+enqueue_stoplistening(isc_nmsocket_t *sock) {
+       isc__netievent_tcpstop_t *ievent =
+               isc__nm_get_netievent_tcpstop(sock->mgr, sock);
+       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
+                              (isc__netievent_t *)ievent);
+}
+
 isc_result_t
 isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface,
                 isc_nm_accept_cb_t accept_cb, void *accept_cbarg,
@@ -442,11 +450,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface,
        sock->backlog = backlog;
        sock->pquota = quota;
 
-       if (isc__nm_in_netthread()) {
-               sock->tid = isc_nm_tid();
-       } else {
-               sock->tid = isc_random_uniform(sock->nchildren);
-       }
+       sock->tid = 0;
        sock->fd = -1;
 
 #if !HAVE_SO_REUSEPORT_LB && !defined(WIN32)
@@ -485,7 +489,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface,
                *sockp = sock;
        } else {
                atomic_store(&sock->active, false);
-               isc_nm_stoplistening(sock);
+               enqueue_stoplistening(sock);
                isc_nmsocket_close(&sock);
        }
 
@@ -642,14 +646,6 @@ done:
        }
 }
 
-static void
-enqueue_stoplistening(isc_nmsocket_t *sock) {
-       isc__netievent_tcpstop_t *ievent =
-               isc__nm_get_netievent_tcpstop(sock->mgr, sock);
-       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
-                              (isc__netievent_t *)ievent);
-}
-
 void
 isc__nm_tcp_stoplistening(isc_nmsocket_t *sock) {
        REQUIRE(VALID_NMSOCK(sock));
@@ -663,11 +659,8 @@ isc__nm_tcp_stoplistening(isc_nmsocket_t *sock) {
 
        if (!isc__nm_in_netthread()) {
                enqueue_stoplistening(sock);
-       } else if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
        } else {
                stop_tcp_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
        }
 }
 
@@ -686,12 +679,7 @@ isc__nm_async_tcpstop(isc__networker_t *worker, isc__netievent_t *ev0) {
                return;
        }
 
-       if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
-       } else {
-               stop_tcp_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
-       }
+       stop_tcp_parent(sock);
 }
 
 void
@@ -1248,7 +1236,9 @@ stop_tcp_child(isc_nmsocket_t *sock) {
 static void
 stop_tcp_parent(isc_nmsocket_t *sock) {
        isc_nmsocket_t *csock = NULL;
+
        REQUIRE(VALID_NMSOCK(sock));
+       REQUIRE(sock->tid == isc_nm_tid());
        REQUIRE(sock->type == isc_nm_tcplistener);
 
        isc_barrier_init(&sock->stoplistening, sock->nchildren);

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 142d8b30faba7abc33f2ccd433afa14f5902b64b..0d9bd85f99cb755403b886b4ea582755c2f3c1ff 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -343,6 +343,14 @@ isc__nm_tcpdns_lb_socket(sa_family_t sa_family) {
        return (sock);
 }
 
+static void
+enqueue_stoplistening(isc_nmsocket_t *sock) {
+       isc__netievent_tcpdnsstop_t *ievent =
+               isc__nm_get_netievent_tcpdnsstop(sock->mgr, sock);
+       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
+                              (isc__netievent_t *)ievent);
+}
+
 static void
 start_tcpdns_child(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nmsocket_t *sock,
                   uv_os_sock_t fd, int tid) {
@@ -412,11 +420,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
        sock->backlog = backlog;
        sock->pquota = quota;
 
-       if (isc__nm_in_netthread()) {
-               sock->tid = isc_nm_tid();
-       } else {
-               sock->tid = isc_random_uniform(sock->nchildren);
-       }
+       sock->tid = 0;
        sock->fd = -1;
 
 #if !HAVE_SO_REUSEPORT_LB && !defined(WIN32)
@@ -455,7 +459,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface,
                *sockp = sock;
        } else {
                atomic_store(&sock->active, false);
-               isc_nm_stoplistening(sock);
+               enqueue_stoplistening(sock);
                isc_nmsocket_close(&sock);
        }
 
@@ -612,14 +616,6 @@ done:
        }
 }
 
-static void
-enqueue_stoplistening(isc_nmsocket_t *sock) {
-       isc__netievent_tcpdnsstop_t *ievent =
-               isc__nm_get_netievent_tcpdnsstop(sock->mgr, sock);
-       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
-                              (isc__netievent_t *)ievent);
-}
-
 void
 isc__nm_tcpdns_stoplistening(isc_nmsocket_t *sock) {
        REQUIRE(VALID_NMSOCK(sock));
@@ -633,11 +629,8 @@ isc__nm_tcpdns_stoplistening(isc_nmsocket_t *sock) {
 
        if (!isc__nm_in_netthread()) {
                enqueue_stoplistening(sock);
-       } else if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
        } else {
                stop_tcpdns_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
        }
 }
 
@@ -657,15 +650,7 @@ isc__nm_async_tcpdnsstop(isc__networker_t *worker, isc__netievent_t *ev0) {
                return;
        }
 
-       /*
-        * If network manager is paused, re-enqueue the event for later.
-        */
-       if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
-       } else {
-               stop_tcpdns_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
-       }
+       stop_tcpdns_parent(sock);
 }
 
 void
@@ -1283,6 +1268,7 @@ stop_tcpdns_parent(isc_nmsocket_t *sock) {
        isc_nmsocket_t *csock = NULL;
 
        REQUIRE(VALID_NMSOCK(sock));
+       REQUIRE(sock->tid == isc_nm_tid());
        REQUIRE(sock->type == isc_nm_tcpdnslistener);
 
        isc_barrier_init(&sock->stoplistening, sock->nchildren);

diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index 3f68c7f51399ba8313d935914ca50883ff5caf39..212d81539c1f8a5f14a347e88d38d4ff070f411f 100644 (file)
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -445,6 +445,14 @@ start_tlsdns_child(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nmsocket_t *sock,
                                     (isc__netievent_t *)ievent);
 }
 
+static void
+enqueue_stoplistening(isc_nmsocket_t *sock) {
+       isc__netievent_tlsdnsstop_t *ievent =
+               isc__nm_get_netievent_tlsdnsstop(sock->mgr, sock);
+       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
+                              (isc__netievent_t *)ievent);
+}
+
 isc_result_t
 isc_nm_listentlsdns(isc_nm_t *mgr, isc_nmiface_t *iface,
                    isc_nm_recv_cb_t recv_cb, void *recv_cbarg,
@@ -480,13 +488,9 @@ isc_nm_listentlsdns(isc_nm_t *mgr, isc_nmiface_t *iface,
        sock->backlog = backlog;
        sock->pquota = quota;
 
-       if (isc__nm_in_netthread()) {
-               sock->tid = isc_nm_tid();
-       } else {
-               sock->tid = isc_random_uniform(sock->nchildren);
-       }
-
        sock->tls.ctx = sslctx;
+
+       sock->tid = 0;
        sock->fd = -1;
 
 #if !HAVE_SO_REUSEPORT_LB && !defined(WIN32)
@@ -525,7 +529,7 @@ isc_nm_listentlsdns(isc_nm_t *mgr, isc_nmiface_t *iface,
                *sockp = sock;
        } else {
                atomic_store(&sock->active, false);
-               isc_nm_stoplistening(sock);
+               enqueue_stoplistening(sock);
                isc_nmsocket_close(&sock);
        }
 
@@ -683,14 +687,6 @@ done:
        }
 }
 
-static void
-enqueue_stoplistening(isc_nmsocket_t *sock) {
-       isc__netievent_tlsdnsstop_t *ievent =
-               isc__nm_get_netievent_tlsdnsstop(sock->mgr, sock);
-       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
-                              (isc__netievent_t *)ievent);
-}
-
 void
 isc__nm_tlsdns_stoplistening(isc_nmsocket_t *sock) {
        REQUIRE(VALID_NMSOCK(sock));
@@ -704,11 +700,8 @@ isc__nm_tlsdns_stoplistening(isc_nmsocket_t *sock) {
 
        if (!isc__nm_in_netthread()) {
                enqueue_stoplistening(sock);
-       } else if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
        } else {
                stop_tlsdns_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
        }
 }
 
@@ -803,15 +796,7 @@ isc__nm_async_tlsdnsstop(isc__networker_t *worker, isc__netievent_t *ev0) {
                return;
        }
 
-       /*
-        * If network manager is paused, re-enqueue the event for later.
-        */
-       if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
-       } else {
-               stop_tlsdns_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
-       }
+       stop_tlsdns_parent(sock);
 }
 
 void
@@ -1831,6 +1816,7 @@ stop_tlsdns_parent(isc_nmsocket_t *sock) {
        isc_nmsocket_t *csock = NULL;
 
        REQUIRE(VALID_NMSOCK(sock));
+       REQUIRE(sock->tid == isc_nm_tid());
        REQUIRE(sock->type == isc_nm_tlsdnslistener);
 
        isc_barrier_init(&sock->stoplistening, sock->nchildren);

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index ca4900c5ea62b6b09c952c2fdd949a1d3c5ffbdf..ba61418ce08471992d4f563cb938893e7907912e 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -108,6 +108,14 @@ start_udp_child(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nmsocket_t *sock,
                                     (isc__netievent_t *)ievent);
 }
 
+static void
+enqueue_stoplistening(isc_nmsocket_t *sock) {
+       isc__netievent_udpstop_t *ievent =
+               isc__nm_get_netievent_udpstop(sock->mgr, sock);
+       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
+                              (isc__netievent_t *)ievent);
+}
+
 isc_result_t
 isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
                 void *cbarg, size_t extrahandlesize, isc_nmsocket_t **sockp) {
@@ -139,11 +147,8 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
        sock->recv_cbarg = cbarg;
        sock->extrahandlesize = extrahandlesize;
        sock->result = ISC_R_UNSET;
-       if (isc__nm_in_netthread()) {
-               sock->tid = isc_nm_tid();
-       } else {
-               sock->tid = isc_random_uniform(sock->nchildren);
-       }
+
+       sock->tid = 0;
        sock->fd = -1;
 
 #if !HAVE_SO_REUSEPORT_LB && !defined(WIN32)
@@ -182,7 +187,7 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
                *sockp = sock;
        } else {
                atomic_store(&sock->active, false);
-               isc_nm_stoplistening(sock);
+               enqueue_stoplistening(sock);
                isc_nmsocket_close(&sock);
        }
 
@@ -298,14 +303,6 @@ done:
        isc_barrier_wait(&sock->parent->startlistening);
 }
 
-static void
-enqueue_stoplistening(isc_nmsocket_t *sock) {
-       isc__netievent_udpstop_t *ievent =
-               isc__nm_get_netievent_udpstop(sock->mgr, sock);
-       isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
-                              (isc__netievent_t *)ievent);
-}
-
 void
 isc__nm_udp_stoplistening(isc_nmsocket_t *sock) {
        REQUIRE(VALID_NMSOCK(sock));
@@ -319,11 +316,8 @@ isc__nm_udp_stoplistening(isc_nmsocket_t *sock) {
 
        if (!isc__nm_in_netthread()) {
                enqueue_stoplistening(sock);
-       } else if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
        } else {
                stop_udp_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
        }
 }
 
@@ -345,15 +339,7 @@ isc__nm_async_udpstop(isc__networker_t *worker, isc__netievent_t *ev0) {
                return;
        }
 
-       /*
-        * If network manager is paused, re-enqueue the event for later.
-        */
-       if (!isc__nm_acquire_interlocked(sock->mgr)) {
-               enqueue_stoplistening(sock);
-       } else {
-               stop_udp_parent(sock);
-               isc__nm_drop_interlocked(sock->mgr);
-       }
+       stop_udp_parent(sock);
 }
 
 /*
@@ -1016,6 +1002,7 @@ stop_udp_parent(isc_nmsocket_t *sock) {
        isc_nmsocket_t *csock = NULL;
 
        REQUIRE(VALID_NMSOCK(sock));
+       REQUIRE(sock->tid == isc_nm_tid());
        REQUIRE(sock->type == isc_nm_udplistener);
 
        isc_barrier_init(&sock->stoplistening, sock->nchildren);

diff --git a/lib/isc/task.c b/lib/isc/task.c
index 49e2c4fcc6f4d223f1ac068c870fad6f52d8e6f1..9849faeafc457cd475866a5d8583f6c809945925 100644 (file)
--- a/lib/isc/task.c
+++ b/lib/isc/task.c
@@ -1095,6 +1095,8 @@ void
 isc_taskmgr_setexcltask(isc_taskmgr_t *mgr, isc_task_t *task) {
        REQUIRE(VALID_MANAGER(mgr));
        REQUIRE(VALID_TASK(task));
+       REQUIRE(task->threadid == 0);
+
        LOCK(&mgr->excl_lock);
        if (mgr->excl != NULL) {
                isc_task_detach(&mgr->excl);

diff --git a/lib/isc/tests/isctest.c b/lib/isc/tests/isctest.c
index 89ef18c5c7278c71f995044894ede8361ed0bdb8..6975501ef7b9b1e695fec899c1e2cb749959efb5 100644 (file)
--- a/lib/isc/tests/isctest.c
+++ b/lib/isc/tests/isctest.c
@@ -83,7 +83,7 @@ create_managers(unsigned int workers) {
        isc_managers_create(test_mctx, workers, 0, 0, &netmgr, &taskmgr,
                            &timermgr, &socketmgr);
 
-       CHECK(isc_task_create(taskmgr, 0, &maintask));
+       CHECK(isc_task_create_bound(taskmgr, 0, &maintask, 0));
        isc_taskmgr_setexcltask(taskmgr, maintask);
 
        return (ISC_R_SUCCESS);

diff --git a/lib/isc/tests/task_test.c b/lib/isc/tests/task_test.c
index 4edcfc902f00ad984ebe6a49e8454847acf8f2cd..b47770bfff2314a48d6061fecc5f9c82446ead0b 100644 (file)
--- a/lib/isc/tests/task_test.c
+++ b/lib/isc/tests/task_test.c
@@ -708,12 +708,16 @@ task_exclusive(void **state) {
 
                tasks[i] = NULL;
 
-               result = isc_task_create(taskmgr, 0, &tasks[i]);
-               assert_int_equal(result, ISC_R_SUCCESS);
-
-               /* task chosen from the middle of the range */
                if (i == 6) {
+                       /* task chosen from the middle of the range */
+                       result = isc_task_create_bound(taskmgr, 0, &tasks[i],
+                                                      0);
+                       assert_int_equal(result, ISC_R_SUCCESS);
+
                        isc_taskmgr_setexcltask(taskmgr, tasks[6]);
+               } else {
+                       result = isc_task_create(taskmgr, 0, &tasks[i]);
+                       assert_int_equal(result, ISC_R_SUCCESS);
                }
 
                v = isc_mem_get(test_mctx, sizeof *v);

diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c
index b291335afa63dc53c7193c4eb2542f5ad8b95a34..6cf7ac91a9a95812f722a47877e03600ba5e4e1e 100644 (file)
--- a/lib/ns/tests/nstest.c
+++ b/lib/ns/tests/nstest.c
@@ -227,7 +227,7 @@ create_managers(void) {
 
        isc_managers_create(mctx, ncpus, 0, 0, &netmgr, &taskmgr, &timermgr,
                            &socketmgr);
-       CHECK(isc_task_create(taskmgr, 0, &maintask));
+       CHECK(isc_task_create_bound(taskmgr, 0, &maintask, 0));
        isc_taskmgr_setexcltask(taskmgr, maintask);
        CHECK(isc_task_onshutdown(maintask, shutdown_managers, NULL));