diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 177adec3c6..7dfa8872e6 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -603,6 +603,13 @@ isc__nmsocket_prep_destroy(isc_nmsocket_t *sock); * if there are no remaining references or active handles. */ +bool +isc__nmsocket_active(isc_nmsocket_t *sock); +/*%< + * Determine whether 'sock' is active by checking 'sock->active' + * or, for child sockets, 'sock->parent->active'. + */ + void isc__nm_async_closecb(isc__networker_t *worker, isc__netievent_t *ev0); /*%< diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 6a66bd5fdb..d7cf3a36af 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -681,7 +681,7 @@ isc__nm_enqueue_ievent(isc__networker_t *worker, isc__netievent_t *event) { uv_async_send(&worker->async); } -static bool +bool isc__nmsocket_active(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); if (sock->parent != NULL) { @@ -1158,7 +1158,7 @@ nmhandle_deactivate(isc_nmsocket_t *sock, isc_nmhandle_t *handle) { void isc_nmhandle_unref(isc_nmhandle_t *handle) { - isc_nmsocket_t *sock = NULL; + isc_nmsocket_t *sock = NULL, *tmp = NULL; REQUIRE(VALID_NMHANDLE(handle)); @@ -1199,8 +1199,14 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) { } } + /* + * Temporarily reference the socket to ensure that it can't + * be deleted by another thread while we're deactivating the + * handle. + */ + isc_nmsocket_attach(sock, &tmp); nmhandle_deactivate(sock, handle); - nmsocket_maybe_destroy(sock); + isc_nmsocket_detach(&tmp); } void * diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 17a97cda38..e0c566e176 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -419,7 +419,7 @@ stoplistening(isc_nmsocket_t *sock) { event = isc__nm_get_ievent(sock->mgr, netievent_tcpchildstop); isc_nmsocket_attach(&sock->children[i], &event->sock); - if (i == sock->tid) { + if (isc_nm_tid() == sock->children[i].tid) { isc__nm_async_tcpchildstop(&sock->mgr->workers[i], (isc__netievent_t *) event); isc__nm_put_ievent(sock->mgr, event); diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index a15cb07b96..36f63c8189 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -470,8 +470,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region, REQUIRE(sock->type == isc_nm_tcpdnssocket); if (sock->outer == NULL) { - /* The socket is closed, just issue the callback */ - cb(handle, ISC_R_FAILURE, cbarg); + /* The socket is closed */ return (ISC_R_NOTCONNECTED); } diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 6b79d8e0bc..d4f27a7c36 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -122,6 +122,7 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) { REQUIRE(sock->type == isc_nm_udpsocket); REQUIRE(sock->iface != NULL); REQUIRE(sock->parent != NULL); + REQUIRE(sock->tid == isc_nm_tid()); uv_udp_init(&worker->loop, &sock->uv_handle.udp); uv_handle_set_data(&sock->uv_handle.handle, NULL); @@ -164,7 +165,8 @@ udp_close_cb(uv_handle_t *handle) { static void stop_udp_child(isc_nmsocket_t *sock) { - INSIST(sock->type == isc_nm_udpsocket); + REQUIRE(sock->type == isc_nm_udpsocket); + REQUIRE(sock->tid == isc_nm_tid()); uv_udp_recv_stop(&sock->uv_handle.udp); uv_close((uv_handle_t *) &sock->uv_handle.udp, udp_close_cb); @@ -179,19 +181,25 @@ stop_udp_child(isc_nmsocket_t *sock) { static void stoplistening(isc_nmsocket_t *sock) { + REQUIRE(sock->type == isc_nm_udplistener); + /* * Socket is already closing; there's nothing to do. */ - if (uv_is_closing((uv_handle_t *) &sock->uv_handle.udp)) { + if (!isc__nmsocket_active(sock)) { return; } - INSIST(sock->type == isc_nm_udplistener); + /* + * Mark it inactive now so that all sends will be ignored + * and we won't try to stop listening again. + */ + atomic_store(&sock->active, false); for (int i = 0; i < sock->nchildren; i++) { isc__netievent_udpstop_t *event = NULL; - if (i == sock->tid) { + if (isc_nm_tid() == sock->children[i].tid) { stop_udp_child(&sock->children[i]); continue; } @@ -358,7 +366,11 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, /* * Simulate a firewall blocking UDP packets bigger than - * 'maxudp' bytes. + * 'maxudp' bytes, for testing purposes. + * + * The client would ordinarily have unreferenced the handle + * in the callback, but that won't happen in this case, so + * we need to do so here. */ if (maxudp != 0 && region->length > maxudp) { isc_nmhandle_unref(handle); @@ -371,8 +383,12 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, } else if (sock->type == isc_nm_udplistener) { psock = sock; } else { - isc_nmhandle_unref(handle); - return (ISC_R_UNEXPECTED); + INSIST(0); + ISC_UNREACHABLE(); + } + + if (!isc__nmsocket_active(sock)) { + return (ISC_R_CANCELED); } if (isc__nm_in_netthread()) { @@ -425,7 +441,7 @@ isc__nm_async_udpsend(isc__networker_t *worker, isc__netievent_t *ev0) { REQUIRE(worker->id == ievent->sock->tid); - if (atomic_load(&ievent->sock->active)) { + if (isc__nmsocket_active(ievent->sock)) { udp_send_direct(ievent->sock, ievent->req, &ievent->peer); } else { ievent->req->cb.send(ievent->req->handle, diff --git a/lib/ns/client.c b/lib/ns/client.c index ad99883a23..b60db14c80 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -381,8 +381,9 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) { r.base[1] = client->message->id & 0xff; result = client_sendpkg(client, &buffer); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { return; + } done: if (client->tcpbuf != NULL) { @@ -392,6 +393,7 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) { } ns_client_drop(client, result); + isc_nmhandle_unref(client->handle); } void @@ -596,6 +598,10 @@ ns_client_send(ns_client_t *client) { isc_nmhandle_ref(client->handle); result = client_sendpkg(client, &tcpbuffer); + if (result != ISC_R_SUCCESS) { + /* We won't get a callback to clean it up */ + isc_nmhandle_unref(client->handle); + } switch (isc_sockaddr_pf(&client->peeraddr)) { case AF_INET: @@ -629,6 +635,10 @@ ns_client_send(ns_client_t *client) { isc_nmhandle_ref(client->handle); result = client_sendpkg(client, &buffer); + if (result != ISC_R_SUCCESS) { + /* We won't get a callback to clean it up */ + isc_nmhandle_unref(client->handle); + } switch (isc_sockaddr_pf(&client->peeraddr)) { case AF_INET: @@ -2333,6 +2343,16 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { isc_task_detach(&client->task); } + if (client->manager != NULL) { + clientmgr_detach(&client->manager); + } + if (client->mctx != NULL) { + isc_mem_detach(&client->mctx); + } + if (client->sctx != NULL) { + ns_server_detach(&client->sctx); + } + return (result); } diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index 9c393c63ac..7ad4ef96ea 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -1663,8 +1663,6 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { xfrout_fail(xfr, result, "send"); } else if (xfr->end_of_stream == false) { sendstream(xfr); - /* Return now so we don't unref the handle */ - return; } else { /* End of zone transfer stream. */ uint64_t msecs, persec; @@ -1690,9 +1688,9 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { (unsigned int) persec); xfrout_ctx_destroy(&xfr); + /* We're done, unreference the handle */ + isc_nmhandle_unref(handle); } - - isc_nmhandle_unref(handle); } static void