From: Ondřej Surý Date: Thu, 6 May 2021 14:11:43 +0000 (+0200) Subject: ensure interlocked netmgr events run on worker[0] X-Git-Tag: v9.17.13~8^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=365c6a9851fd6f37cf16129a3d5d624541f2f6a6;p=thirdparty%2Fbind9.git 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 ea343340ad1..d411647c443 100644 --- 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 e64a018e5de..b76c0b263ba 100644 --- 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 05bbae5b614..2baa4419f90 100644 --- 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 052030bf79f..c7030eccccf 100644 --- 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 142d8b30fab..0d9bd85f99c 100644 --- 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 3f68c7f5139..212d81539c1 100644 --- 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 ca4900c5ea6..ba61418ce08 100644 --- 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 49e2c4fcc6f..9849faeafc4 100644 --- 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 89ef18c5c72..6975501ef7b 100644 --- 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 4edcfc902f0..b47770bfff2 100644 --- 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 b291335afa6..6cf7ac91a9a 100644 --- 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));