From ba2e9dfb99ac50970874ba2744f930fc4c291bd8 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 3 Sep 2020 13:31:27 -0700 Subject: [PATCH] change from isc_nmhandle_ref/unref to isc_nmhandle attach/detach Attaching and detaching handle pointers will make it easier to determine where and why reference counting errors have occurred. A handle needs to be referenced more than once when multiple asynchronous operations are in flight, so callers must now maintain multiple handle pointers for each pending operation. For example, ns_client objects now contain: - reqhandle: held while waiting for a request callback (query, notify, update) - sendhandle: held while waiting for a send callback - fetchhandle: held while waiting for a recursive fetch to complete - updatehandle: held while waiting for an update-forwarding task to complete (cherry picked from commit 57b4dde9749c88d21d1dc8afd22201224cf83cab) --- bin/named/server.c | 5 +- bin/tests/system/rndc/ns4/named.conf.in | 1 + lib/isc/include/isc/netmgr.h | 4 +- lib/isc/netmgr/netmgr.c | 27 +++++++-- lib/isc/netmgr/tcp.c | 19 +++--- lib/isc/netmgr/tcpdns.c | 81 ++++++++++++++----------- lib/isc/netmgr/udp.c | 36 ++++------- lib/isc/win32/libisc.def.in | 4 +- lib/ns/client.c | 52 ++++++---------- lib/ns/include/ns/client.h | 21 ++++--- lib/ns/include/ns/notify.h | 2 +- lib/ns/include/ns/query.h | 2 +- lib/ns/include/ns/update.h | 3 +- lib/ns/notify.c | 11 +++- lib/ns/query.c | 30 +++++---- lib/ns/tests/Makefile.in | 2 +- lib/ns/tests/notify_test.c | 8 ++- lib/ns/tests/nstest.c | 56 +++++++++++++---- lib/ns/update.c | 35 +++++++---- lib/ns/xfrout.c | 52 ++++++---------- 20 files changed, 254 insertions(+), 197 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index ee0886ad0b1..56961ea7530 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -14944,14 +14944,15 @@ named_server_zonestatus(named_server_t *server, isc_lex_t *lex, /* Serial number */ result = dns_zone_getserial(mayberaw, &serial); - /* XXXWPK TODO this is to mirror old behavior with dns_zone_getserial */ + + /* This is to mirror old behavior with dns_zone_getserial */ if (result != ISC_R_SUCCESS) { serial = 0; } + snprintf(serbuf, sizeof(serbuf), "%u", serial); if (hasraw) { result = dns_zone_getserial(zone, &signed_serial); - /* XXXWPK TODO ut supra */ if (result != ISC_R_SUCCESS) { serial = 0; } diff --git a/bin/tests/system/rndc/ns4/named.conf.in b/bin/tests/system/rndc/ns4/named.conf.in index 2466e695591..4f009a7c2db 100644 --- a/bin/tests/system/rndc/ns4/named.conf.in +++ b/bin/tests/system/rndc/ns4/named.conf.in @@ -15,6 +15,7 @@ options { listen-on { 10.53.0.4; }; listen-on-v6 { none; }; recursion yes; + dnssec-validation yes; }; view normal { diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 61ef9abee7e..dc4f977e420 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -110,9 +110,9 @@ isc_nmsocket_close(isc_nmsocket_t **sockp); */ void -isc_nmhandle_ref(isc_nmhandle_t *handle); +isc_nmhandle_attach(isc_nmhandle_t *handle, isc_nmhandle_t **dest); void -isc_nmhandle_unref(isc_nmhandle_t *handle); +isc_nmhandle_detach(isc_nmhandle_t **handlep); /*%< * Increment/decrement the reference counter in a netmgr handle, * but (unlike the attach/detach functions) do not change the pointer diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index a88f8f11e26..bebb68eabd4 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -746,8 +746,7 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree) { sock->statichandle = NULL; if (sock->outerhandle != NULL) { - isc_nmhandle_unref(sock->outerhandle); - sock->outerhandle = NULL; + isc_nmhandle_detach(&sock->outerhandle); } if (sock->outer != NULL) { @@ -1107,6 +1106,13 @@ isc__nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer, (sock->type == isc_nm_udpsocket && atomic_load(&sock->client))) { INSIST(sock->statichandle == NULL); + + /* + * statichandle must be assigned, not attached; + * otherwise, if a handle was detached elsewhere + * it could never reach 0 references, and the + * handle and socket would never be freed. + */ sock->statichandle = handle; } @@ -1114,10 +1120,12 @@ isc__nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer, } void -isc_nmhandle_ref(isc_nmhandle_t *handle) { +isc_nmhandle_attach(isc_nmhandle_t *handle, isc_nmhandle_t **handlep) { REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(handlep != NULL && *handlep == NULL); isc_refcount_increment(&handle->references); + *handlep = handle; } bool @@ -1173,14 +1181,20 @@ nmhandle_deactivate(isc_nmsocket_t *sock, isc_nmhandle_t *handle) { } void -isc_nmhandle_unref(isc_nmhandle_t *handle) { +isc_nmhandle_detach(isc_nmhandle_t **handlep) { isc_nmsocket_t *sock = NULL; + isc_nmhandle_t *handle = NULL; - REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(handlep != NULL); + REQUIRE(VALID_NMHANDLE(*handlep)); + + handle = *handlep; + *handlep = NULL; if (isc_refcount_decrement(&handle->references) > 1) { return; } + /* We need an acquire memory barrier here */ (void)isc_refcount_current(&handle->references); @@ -1215,6 +1229,7 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) { } if (handle == sock->statichandle) { + /* statichandle is assigned, not attached. */ sock->statichandle = NULL; } @@ -1319,7 +1334,7 @@ isc__nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock) { } if (handle != NULL) { - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&handle); } isc__nmsocket_detach(&sock); diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 7fd2a87ef3f..71e041b1fba 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -183,10 +183,10 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) { isc__nmsocket_detach(&sock); /* - * If the connect callback wants to hold on to the handle, - * it needs to attach to it. + * The connect callback should have attached to the handle. + * If it didn't, the socket will be closed now. */ - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&handle); } isc_result_t @@ -498,10 +498,10 @@ isc__nm_async_tcpchildaccept(isc__networker_t *worker, isc__netievent_t *ev0) { isc__nmsocket_detach(&csock); /* - * If the accept callback wants to hold on to the handle, - * it needs to attach to it. + * The accept callback should have attached to the handle. + * If it didn't, the socket will be closed now. */ - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&handle); return; error: @@ -959,8 +959,7 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, uvreq = isc__nm_uvreq_get(sock->mgr, sock); uvreq->uvbuf.base = (char *)region->base; uvreq->uvbuf.len = region->length; - uvreq->handle = handle; - isc_nmhandle_ref(uvreq->handle); + isc_nmhandle_attach(handle, &uvreq->handle); uvreq->cb.send = cb; uvreq->cbarg = cbarg; @@ -1003,7 +1002,6 @@ tcp_send_cb(uv_write_t *req, int status) { uvreq->cb.send(uvreq->handle, result, uvreq->cbarg); sock = uvreq->handle->sock; - isc_nmhandle_unref(uvreq->handle); isc__nm_uvreq_put(&uvreq, sock); } @@ -1035,8 +1033,6 @@ tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(sock->type == isc_nm_tcpsocket); - - isc_nmhandle_ref(req->handle); r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, &req->uvbuf, 1, tcp_send_cb); if (r < 0) { @@ -1143,6 +1139,7 @@ isc__nm_tcp_cancelread(isc_nmhandle_t *handle) { sock = handle->sock; + REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->type == isc_nm_tcpsocket); if (atomic_load(&sock->client) && sock->rcb.recv != NULL) { diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index 5fa1d3bbfb5..13dac785bab 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -98,8 +98,7 @@ dnstcp_readtimeout(uv_timer_t *timer) { REQUIRE(sock->tid == isc_nm_tid()); /* Close the TCP connection; its closure should fire ours. */ - isc_nmhandle_unref(sock->outerhandle); - sock->outerhandle = NULL; + isc_nmhandle_detach(&sock->outerhandle); } /* @@ -109,6 +108,7 @@ static isc_result_t dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc_nmsocket_t *dnslistensock = (isc_nmsocket_t *)cbarg; isc_nmsocket_t *dnssock = NULL; + isc_nmhandle_t *readhandle = NULL; REQUIRE(VALID_NMSOCK(dnslistensock)); REQUIRE(dnslistensock->type == isc_nm_tcpdnslistener); @@ -135,8 +135,7 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc__nmsocket_attach(dnssock, &dnssock->self); - dnssock->outerhandle = handle; - isc_nmhandle_ref(dnssock->outerhandle); + isc_nmhandle_attach(handle, &dnssock->outerhandle); dnssock->peer = handle->sock->peer; dnssock->read_timeout = handle->sock->mgr->init; @@ -150,10 +149,15 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { uv_timer_start(&dnssock->timer, dnstcp_readtimeout, dnssock->read_timeout, 0); - isc_nmhandle_ref(handle); - result = isc_nm_read(handle, dnslisten_readcb, dnssock); + /* + * Add a reference to handle to keep it from being freed by + * the caller. It will be detached in dnslisted_readcb() when + * the connection is closed or there is no more data to be read. + */ + isc_nmhandle_attach(handle, &readhandle); + result = isc_nm_read(readhandle, dnslisten_readcb, dnssock); if (result != ISC_R_SUCCESS) { - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&readhandle); } isc__nmsocket_detach(&dnssock); @@ -190,17 +194,17 @@ processbuffer(isc_nmsocket_t *dnssock, isc_nmhandle_t **handlep) { */ len = dnslen(dnssock->buf); if (len <= dnssock->buf_len - 2) { - isc_nmhandle_t *dnshandle; + isc_nmhandle_t *dnshandle = NULL; + isc_nmsocket_t *listener = NULL; + if (atomic_load(&dnssock->client) && dnssock->statichandle != NULL) { - dnshandle = dnssock->statichandle; - isc_nmhandle_ref(dnshandle); + isc_nmhandle_attach(dnssock->statichandle, &dnshandle); } else { dnshandle = isc__nmhandle_get(dnssock, NULL, NULL); } - isc_nmsocket_t *listener = dnssock->listener; - + listener = dnssock->listener; if (listener != NULL && listener->rcb.recv != NULL) { listener->rcb.recv( dnshandle, ISC_R_SUCCESS, @@ -238,8 +242,8 @@ processbuffer(isc_nmsocket_t *dnssock, isc_nmhandle_t **handlep) { } /* - * We've got a read on our underlying socket, need to check if we have - * a complete DNS packet and, if so - call the callback + * We've got a read on our underlying socket. Check whether + * we have a complete DNS packet and, if so, call the callback. */ static void dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult, @@ -254,18 +258,15 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult, if (region == NULL || eresult != ISC_R_SUCCESS) { /* Connection closed */ - if (eresult != ISC_R_CANCELED) { - isc_nmhandle_unref(handle); - } dnssock->result = eresult; if (dnssock->self != NULL) { isc__nmsocket_detach(&dnssock->self); } isc__nmsocket_clearcb(dnssock); if (dnssock->outerhandle != NULL) { - isc_nmhandle_unref(dnssock->outerhandle); - dnssock->outerhandle = NULL; + isc_nmhandle_detach(&dnssock->outerhandle); } + isc_nmhandle_detach(&handle); return; } @@ -305,11 +306,11 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult, if (atomic_load(&dnssock->sequential) || dnssock->rcb.recv == NULL) { /* - * Two reasons we might want to pause here: - * - If we're in sequential mode and we've received + * There are two reasons we might want to pause here: + * - We're in sequential mode and we've received * a whole packet, so we're done until it's been - * processed; - * - If we no longer have a read callback. + * processed; or + * - We no longer have a read callback. */ isc_nm_pauseread(dnssock->outerhandle); done = true; @@ -328,7 +329,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_result_t eresult, } } - isc_nmhandle_unref(dnshandle); + isc_nmhandle_detach(&dnshandle); } while (!done); } @@ -460,12 +461,11 @@ resume_processing(void *arg) { if (sock->timer_initialized) { uv_timer_stop(&sock->timer); } - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&handle); } else if (sock->outerhandle != NULL) { result = isc_nm_resumeread(sock->outerhandle); if (result != ISC_R_SUCCESS) { - isc_nmhandle_unref(sock->outerhandle); - sock->outerhandle = NULL; + isc_nmhandle_detach(&sock->outerhandle); } } @@ -495,7 +495,7 @@ resume_processing(void *arg) { uv_timer_stop(&sock->timer); } atomic_store(&sock->outerhandle->sock->processing, true); - isc_nmhandle_unref(dnshandle); + isc_nmhandle_detach(&dnshandle); } while (atomic_load(&sock->ah) < TCPDNS_CLIENTS_PER_CONN); } @@ -508,6 +508,7 @@ tcpdnssend_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { req->cb.send(req->handle, result, req->cbarg); isc_mem_put(req->sock->mgr->mctx, req->uvbuf.base, req->uvbuf.len); isc__nm_uvreq_put(&req, req->handle->sock); + isc_nmhandle_detach(&handle); } void @@ -522,11 +523,16 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) { result = ISC_R_NOTCONNECTED; if (atomic_load(&sock->active) && sock->outerhandle != NULL) { + isc_nmhandle_t *sendhandle = NULL; isc_region_t r; r.base = (unsigned char *)req->uvbuf.base; r.length = req->uvbuf.len; - result = isc_nm_send(sock->outerhandle, &r, tcpdnssend_cb, req); + isc_nmhandle_attach(sock->outerhandle, &sendhandle); + result = isc_nm_send(sendhandle, &r, tcpdnssend_cb, req); + if (result != ISC_R_SUCCESS) { + isc_nmhandle_detach(&sendhandle); + } } if (result != ISC_R_SUCCESS) { @@ -552,8 +558,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region, REQUIRE(sock->type == isc_nm_tcpdnssocket); uvreq = isc__nm_uvreq_get(sock->mgr, sock); - uvreq->handle = handle; - isc_nmhandle_ref(uvreq->handle); + isc_nmhandle_attach(handle, &uvreq->handle); uvreq->cb.send = cb; uvreq->cbarg = cbarg; @@ -563,13 +568,20 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region, memmove(uvreq->uvbuf.base + 2, region->base, region->length); if (sock->tid == isc_nm_tid()) { + isc_result_t result; + isc_nmhandle_t *sendhandle = NULL; isc_region_t r; r.base = (unsigned char *)uvreq->uvbuf.base; r.length = uvreq->uvbuf.len; - return (isc_nm_send(sock->outerhandle, &r, tcpdnssend_cb, - uvreq)); + isc_nmhandle_attach(sock->outerhandle, &sendhandle); + result = isc_nm_send(sock->outerhandle, &r, tcpdnssend_cb, + uvreq); + if (result != ISC_R_SUCCESS) { + isc_nmhandle_detach(&sendhandle); + } + return (result); } else { isc__netievent_tcpdnssend_t *ievent = NULL; @@ -609,8 +621,7 @@ tcpdns_close_direct(isc_nmsocket_t *sock) { */ if (sock->outerhandle != NULL) { isc__nmsocket_clearcb(sock->outerhandle->sock); - isc_nmhandle_unref(sock->outerhandle); - sock->outerhandle = NULL; + isc_nmhandle_detach(&sock->outerhandle); } if (sock->listener != NULL) { isc__nmsocket_detach(&sock->listener); diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 4ac562cc6cd..84e0b818d83 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -159,7 +159,7 @@ udp_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) { } /* - * handle 'udplisten' async call - start listening on a socket. + * Asynchronous 'udplisten' call handler: start listening on a UDP socket. */ void isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) { @@ -311,7 +311,7 @@ isc__nm_udp_stoplistening(isc_nmsocket_t *sock) { } /* - * handle 'udpstop' async call - stop listening on a socket. + * Asynchronous 'udpstop' call handler: stop listening on a UDP socket. */ void isc__nm_async_udpstop(isc__networker_t *worker, isc__netievent_t *ev0) { @@ -375,9 +375,9 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf, #endif /* - * Three reasons to return now without processing: - * - If addr == NULL that's the end of stream - we can - * free the buffer and bail. + * Three possible reasons to return now without processing: + * - If addr == NULL, in which case it's the end of stream; + * we can free the buffer and bail. * - If we're simulating a firewall blocking UDP packets * bigger than 'maxudp' bytes for testing purposes. * - If the socket is no longer active. @@ -395,11 +395,7 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf, result = isc_sockaddr_fromsockaddr(&sockaddr, addr); RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (!atomic_load(&sock->connected)) { - nmhandle = isc__nmhandle_get(sock, &sockaddr, NULL); - } else { - nmhandle = sock->statichandle; - } + nmhandle = isc__nmhandle_get(sock, &sockaddr, NULL); region.base = (unsigned char *)buf->base; region.length = nrecv; @@ -418,13 +414,13 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf, * If the recv callback wants to hold on to the handle, * it needs to attach to it. */ - isc_nmhandle_unref(nmhandle); + isc_nmhandle_detach(&nmhandle); } /* - * isc__nm_udp_send sends buf to a peer on a socket. - * It tries to find a proper sibling/child socket so that we won't have - * to jump to another thread. + * Send the data in 'region' to a peer via a UDP socket. We try to find + * a proper sibling/child socket so that we won't have to jump to another + * thread. */ isc_result_t isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, @@ -446,7 +442,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, * we need to do so here. */ if (maxudp != 0 && region->length > maxudp) { - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&handle); return (ISC_R_SUCCESS); } @@ -486,8 +482,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, uvreq->uvbuf.base = (char *)region->base; uvreq->uvbuf.len = region->length; - uvreq->handle = handle; - isc_nmhandle_ref(uvreq->handle); + isc_nmhandle_attach(handle, &uvreq->handle); uvreq->cb.send = cb; uvreq->cbarg = cbarg; @@ -514,7 +509,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, } /* - * handle 'udpsend' async event - send a packet on the socket + * Asynchronous 'udpsend' event handler: send a packet on a UDP socket. */ void isc__nm_async_udpsend(isc__networker_t *worker, isc__netievent_t *ev0) { @@ -531,9 +526,6 @@ isc__nm_async_udpsend(isc__networker_t *worker, isc__netievent_t *ev0) { } } -/* - * udp_send_cb - callback - */ static void udp_send_cb(uv_udp_send_t *req, int status) { isc_result_t result = ISC_R_SUCCESS; @@ -549,7 +541,6 @@ udp_send_cb(uv_udp_send_t *req, int status) { } uvreq->cb.send(uvreq->handle, result, uvreq->cbarg); - isc_nmhandle_unref(uvreq->handle); isc__nm_uvreq_put(&uvreq, uvreq->sock); } @@ -570,7 +561,6 @@ udp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req, return (ISC_R_CANCELED); } - isc_nmhandle_ref(req->handle); sa = atomic_load(&sock->connected) ? NULL : &peer->type.sa; rv = uv_udp_send(&req->uv_req.udp_send, &sock->uv_handle.udp, &req->uvbuf, 1, sa, udp_send_cb); diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 22ec9abfe26..fd67ae8e495 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -444,15 +444,15 @@ isc_netaddr_setzone isc_netaddr_totext isc_netaddr_unspec isc_netscope_pton +isc_nmhandle_attach +isc_nmhandle_detach isc_nmhandle_getdata isc_nmhandle_getextra isc_nmhandle_is_stream isc_nmhandle_netmgr isc_nmhandle_localaddr isc_nmhandle_peeraddr -isc_nmhandle_ref isc_nmhandle_setdata -isc_nmhandle_unref isc_nm_cancelread isc_nm_closedown isc_nm_destroy diff --git a/lib/ns/client.c b/lib/ns/client.c index 498e78afee6..cc28ffd0ef1 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -274,7 +274,7 @@ static void client_senddone(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { ns_client_t *client = cbarg; - REQUIRE(client->handle == handle); + REQUIRE(client->sendhandle == handle); CTRACE("senddone"); if (result != ISC_R_SUCCESS) { @@ -283,7 +283,7 @@ client_senddone(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { "send failed: %s", isc_result_totext(result)); } - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&client->sendhandle); } static void @@ -324,13 +324,18 @@ client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer, static isc_result_t client_sendpkg(ns_client_t *client, isc_buffer_t *buffer) { + isc_result_t result; isc_region_t r; - isc_buffer_usedregion(buffer, &r); - - INSIST(client->handle != NULL); + REQUIRE(client->sendhandle == NULL); - return (isc_nm_send(client->handle, &r, client_senddone, client)); + isc_buffer_usedregion(buffer, &r); + isc_nmhandle_attach(client->handle, &client->sendhandle); + result = isc_nm_send(client->handle, &r, client_senddone, client); + if (result != ISC_R_SUCCESS) { + isc_nmhandle_detach(&client->sendhandle); + } + return (result); } void @@ -382,7 +387,6 @@ done: } ns_client_drop(client, result); - isc_nmhandle_unref(client->handle); } void @@ -404,17 +408,13 @@ ns_client_send(ns_client_t *client) { isc_region_t zr; #endif /* HAVE_DNSTAP */ + REQUIRE(NS_CLIENT_VALID(client)); + /* * XXXWPK TODO * Delay the response according to the -T delay option */ - REQUIRE(NS_CLIENT_VALID(client)); - /* - * We need to do it to make sure the client and handle - * won't disappear from under us with client_senddone. - */ - env = ns_interfacemgr_getaclenv(client->manager->interface->mgr); CTRACE("send"); @@ -590,12 +590,7 @@ renderend: respsize = isc_buffer_usedlength(&buffer); - 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: @@ -625,12 +620,7 @@ renderend: respsize = isc_buffer_usedlength(&buffer); - 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: @@ -1676,6 +1666,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, ns__client_put_cb); client->handle = handle; } + if (isc_nmhandle_is_stream(handle)) { client->attributes |= NS_CLIENTATTR_TCP; } @@ -1690,8 +1681,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_buffer_add(&tbuffer, region->length); buffer = &tbuffer; - client->peeraddr = isc_nmhandle_peeraddr(client->handle); - + client->peeraddr = isc_nmhandle_peeraddr(handle); client->peeraddr_valid = true; reqsize = isc_buffer_usedlength(buffer); @@ -1955,8 +1945,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_netaddr_fromsockaddr(&client->destaddr, &client->manager->interface->addr); } else { - isc_sockaddr_t sockaddr = - isc_nmhandle_localaddr(client->handle); + isc_sockaddr_t sockaddr = isc_nmhandle_localaddr(handle); isc_netaddr_fromsockaddr(&client->destaddr, &sockaddr); } @@ -2161,8 +2150,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, &client->requesttime, NULL, buffer); #endif /* HAVE_DNSTAP */ - isc_nmhandle_ref(client->handle); - ns_query_start(client); + ns_query_start(client, handle); break; case dns_opcode_update: CTRACE("update"); @@ -2172,14 +2160,12 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, &client->requesttime, NULL, buffer); #endif /* HAVE_DNSTAP */ ns_client_settimeout(client, 60); - isc_nmhandle_ref(client->handle); - ns_update_start(client, sigresult); + ns_update_start(client, handle, sigresult); break; case dns_opcode_notify: CTRACE("notify"); ns_client_settimeout(client, 60); - isc_nmhandle_ref(client->handle); - ns_notify_start(client); + ns_notify_start(client, handle); break; case dns_opcode_iquery: CTRACE("iquery"); diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index f45ca8707c8..33eff5938eb 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -183,14 +183,19 @@ struct ns_client { isc_task_t * task; dns_view_t * view; dns_dispatch_t * dispatch; - isc_nmhandle_t * handle; - unsigned char * tcpbuf; - dns_message_t * message; - unsigned char * sendbuf; - dns_rdataset_t * opt; - uint16_t udpsize; - uint16_t extflags; - int16_t ednsversion; /* -1 noedns */ + isc_nmhandle_t * handle; /* Permanent pointer to handle */ + isc_nmhandle_t * sendhandle; /* Waiting for send callback */ + isc_nmhandle_t * reqhandle; /* Waiting for request callback + (query, update, notify) */ + isc_nmhandle_t *fetchhandle; /* Waiting for recursive fetch */ + isc_nmhandle_t *updatehandle; /* Waiting for update callback */ + unsigned char * tcpbuf; + dns_message_t * message; + unsigned char * sendbuf; + dns_rdataset_t *opt; + uint16_t udpsize; + uint16_t extflags; + int16_t ednsversion; /* -1 noedns */ void (*cleanup)(ns_client_t *); void (*shutdown)(void *arg, isc_result_t result); void * shutdown_arg; diff --git a/lib/ns/include/ns/notify.h b/lib/ns/include/ns/notify.h index db2efb6fede..e36bdabbeed 100644 --- a/lib/ns/include/ns/notify.h +++ b/lib/ns/include/ns/notify.h @@ -29,7 +29,7 @@ ***/ void -ns_notify_start(ns_client_t *client); +ns_notify_start(ns_client_t *client, isc_nmhandle_t *handle); /*%< * Examines the incoming message to determine appropriate zone. diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h index e6ab1ca9cdc..734f1baf1f4 100644 --- a/lib/ns/include/ns/query.h +++ b/lib/ns/include/ns/query.h @@ -209,7 +209,7 @@ void ns_query_free(ns_client_t *client); void -ns_query_start(ns_client_t *client); +ns_query_start(ns_client_t *client, isc_nmhandle_t *handle); void ns_query_cancel(ns_client_t *client); diff --git a/lib/ns/include/ns/update.h b/lib/ns/include/ns/update.h index a2f53086f0c..6eeeaec9dd7 100644 --- a/lib/ns/include/ns/update.h +++ b/lib/ns/include/ns/update.h @@ -37,6 +37,7 @@ ***/ void -ns_update_start(ns_client_t *client, isc_result_t sigresult); +ns_update_start(ns_client_t *client, isc_nmhandle_t *handle, + isc_result_t sigresult); #endif /* NS_UPDATE_H */ diff --git a/lib/ns/notify.c b/lib/ns/notify.c index 6ca91caee30..f7f7d00e73b 100644 --- a/lib/ns/notify.c +++ b/lib/ns/notify.c @@ -54,7 +54,7 @@ respond(ns_client_t *client, isc_result_t result) { } if (msg_result != ISC_R_SUCCESS) { ns_client_drop(client, msg_result); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->reqhandle); return; } message->rcode = rcode; @@ -65,11 +65,11 @@ respond(ns_client_t *client, isc_result_t result) { } ns_client_send(client); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->reqhandle); } void -ns_notify_start(ns_client_t *client) { +ns_notify_start(ns_client_t *client, isc_nmhandle_t *handle) { dns_message_t *request = client->message; isc_result_t result; dns_name_t *zonename; @@ -79,6 +79,11 @@ ns_notify_start(ns_client_t *client) { char tsigbuf[DNS_NAME_FORMATSIZE * 2 + sizeof(": TSIG '' ()")]; dns_tsigkey_t *tsigkey; + /* + * Attach to the request handle + */ + isc_nmhandle_attach(handle, &client->reqhandle); + /* * Interpret the question section. */ diff --git a/lib/ns/query.c b/lib/ns/query.c index fe24e1b9fd9..b16cb527b81 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -550,7 +550,7 @@ query_send(ns_client_t *client) { inc_stats(client, counter); ns_client_send(client); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->reqhandle); } static void @@ -577,7 +577,7 @@ query_error(ns_client_t *client, isc_result_t result, int line) { log_queryerror(client, result, line, loglevel); ns_client_error(client, result); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->reqhandle); } static void @@ -590,7 +590,7 @@ query_next(ns_client_t *client, isc_result_t result) { inc_stats(client, ns_statscounter_failure); } ns_client_drop(client, result); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->reqhandle); } static inline void @@ -2471,7 +2471,7 @@ prefetch_done(isc_task_t *task, isc_event_t *event) { } free_devent(client, &event, &devent); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->fetchhandle); } static void @@ -2514,7 +2514,7 @@ query_prefetch(ns_client_t *client, dns_name_t *qname, peeraddr = NULL; } - isc_nmhandle_ref(client->handle); + isc_nmhandle_attach(client->handle, &client->fetchhandle); options = client->query.fetchoptions | DNS_FETCHOPT_PREFETCH; result = dns_resolver_createfetch( client->view->resolver, qname, rdataset->type, NULL, NULL, NULL, @@ -2523,7 +2523,7 @@ query_prefetch(ns_client_t *client, dns_name_t *qname, &client->query.prefetch); if (result != ISC_R_SUCCESS) { ns_client_putrdataset(client, &tmprdataset); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->fetchhandle); } dns_rdataset_clearprefetch(rdataset); @@ -2728,7 +2728,7 @@ query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) { } options = client->query.fetchoptions; - isc_nmhandle_ref(client->handle); + isc_nmhandle_attach(client->handle, &client->fetchhandle); result = dns_resolver_createfetch( client->view->resolver, qname, type, NULL, NULL, NULL, peeraddr, client->message->id, options, 0, NULL, client->task, @@ -2736,7 +2736,7 @@ query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) { &client->query.prefetch); if (result != ISC_R_SUCCESS) { ns_client_putrdataset(client, &tmprdataset); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->fetchhandle); } } @@ -5702,6 +5702,8 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { } UNLOCK(&client->manager->reclock); + isc_nmhandle_detach(&client->fetchhandle); + client->query.attributes &= ~NS_QUERYATTR_RECURSING; client->state = NS_CLIENTSTATE_WORKING; @@ -5763,7 +5765,6 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { } dns_resolver_destroyfetch(&fetch); - isc_nmhandle_unref(client->handle); } /*% @@ -5956,14 +5957,14 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname, peeraddr = &client->peeraddr; } - isc_nmhandle_ref(client->handle); + isc_nmhandle_attach(client->handle, &client->fetchhandle); result = dns_resolver_createfetch( client->view->resolver, qname, qtype, qdomain, nameservers, NULL, peeraddr, client->message->id, client->query.fetchoptions, 0, NULL, client->task, fetch_callback, client, rdataset, sigrdataset, &client->query.fetch); if (result != ISC_R_SUCCESS) { - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->fetchhandle); ns_client_putrdataset(client, &rdataset); if (sigrdataset != NULL) { ns_client_putrdataset(client, &sigrdataset); @@ -11124,7 +11125,7 @@ log_queryerror(ns_client_t *client, isc_result_t result, int line, int level) { } void -ns_query_start(ns_client_t *client) { +ns_query_start(ns_client_t *client, isc_nmhandle_t *handle) { isc_result_t result; dns_message_t *message; dns_rdataset_t *rdataset; @@ -11134,6 +11135,11 @@ ns_query_start(ns_client_t *client) { REQUIRE(NS_CLIENT_VALID(client)); + /* + * Attach to the request handle + */ + isc_nmhandle_attach(handle, &client->reqhandle); + message = client->message; saved_extflags = client->extflags; saved_flags = client->message->flags; diff --git a/lib/ns/tests/Makefile.in b/lib/ns/tests/Makefile.in index 651b20033c7..ef1c1a6385d 100644 --- a/lib/ns/tests/Makefile.in +++ b/lib/ns/tests/Makefile.in @@ -15,7 +15,7 @@ VERSION=@BIND9_VERSION@ @BIND9_MAKE_INCLUDES@ -WRAP_OPTIONS = -Wl,--wrap=isc_nmhandle_unref +WRAP_OPTIONS = -Wl,--wrap=isc_nmhandle_detach -Wl,--wrap=isc_nmhandle_attach CINCLUDES = -I. -Iinclude ${NS_INCLUDES} ${DNS_INCLUDES} ${ISC_INCLUDES} \ ${OPENSSL_CFLAGS} \ diff --git a/lib/ns/tests/notify_test.c b/lib/ns/tests/notify_test.c index fef7ad98382..f3dbc7029e1 100644 --- a/lib/ns/tests/notify_test.c +++ b/lib/ns/tests/notify_test.c @@ -86,6 +86,7 @@ static void notify_start(void **state) { isc_result_t result; ns_client_t *client = NULL; + isc_nmhandle_t *handle = NULL; dns_message_t *nmsg = NULL; unsigned char ndata[4096]; isc_buffer_t nbuf; @@ -129,13 +130,16 @@ notify_start(void **state) { client->message = nmsg; nmsg = NULL; client->sendcb = check_response; - ns_notify_start(client); + ns_notify_start(client, client->handle); /* * Clean up */ ns_test_cleanup_zone(); - isc_nmhandle_unref(client->handle); + + handle = client->handle; + isc_nmhandle_detach(&client->handle); + isc_nmhandle_detach(&handle); } #endif /* if defined(USE_LIBTOOL) || LD_WRAP */ diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 6e58b9377ca..36a62796682 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -78,19 +78,43 @@ atomic_uint_fast32_t client_refs[32]; atomic_uintptr_t client_addrs[32]; void -__wrap_isc_nmhandle_unref(isc_nmhandle_t *handle); +__wrap_isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp); +void +__wrap_isc_nmhandle_detach(isc_nmhandle_t **handlep); + +void +__wrap_isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) { + ns_client_t *client = (ns_client_t *)source; + int i; + + for (i = 0; i < 32; i++) { + if (atomic_load(&client_addrs[i]) == (uintptr_t)client) { + break; + } + } + INSIST(i < 32); + INSIST(atomic_load(&client_refs[i]) > 0); + + atomic_fetch_add(&client_refs[i], 1); + + *targetp = source; + return; +} void -__wrap_isc_nmhandle_unref(isc_nmhandle_t *handle) { +__wrap_isc_nmhandle_detach(isc_nmhandle_t **handlep) { + isc_nmhandle_t *handle = *handlep; ns_client_t *client = (ns_client_t *)handle; int i; + *handlep = NULL; + for (i = 0; i < 32; i++) { if (atomic_load(&client_addrs[i]) == (uintptr_t)client) { break; } } - REQUIRE(i < 32); + INSIST(i < 32); if (atomic_fetch_sub(&client_refs[i], 1) == 1) { dns_view_detach(&client->view); @@ -99,13 +123,18 @@ __wrap_isc_nmhandle_unref(isc_nmhandle_t *handle) { ns__client_put_cb(client); isc_mem_put(mctx, client, sizeof(ns_client_t)); } + return; } #ifdef USE_LIBTOOL void -isc_nmhandle_unref(isc_nmhandle_t *handle) { - __wrap_isc_nmhandle_unref(handle); +isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) { + __wrap_isc_nmhandle_attach(source, targetp); +} +void +isc_nmhandle_detach(isc_nmhandle_t **handle) { + __wrap_isc_nmhandle_detach(handle); } #endif /* USE_LIBTOOL */ @@ -747,11 +776,13 @@ create_qctx_for_client(ns_client_t *client, query_ctx_t **qctxp) { saved_hook_table = ns__hook_table; ns__hook_table = query_hooks; - ns_query_start(client); + ns_query_start(client, client->handle); ns__hook_table = saved_hook_table; ns_hooktable_free(mctx, (void **)&query_hooks); + isc_nmhandle_detach(&client->reqhandle); + if (*qctxp == NULL) { return (ISC_R_NOMEMORY); } else { @@ -764,6 +795,7 @@ ns_test_qctx_create(const ns_test_qctx_create_params_t *params, query_ctx_t **qctxp) { ns_client_t *client = NULL; isc_result_t result; + isc_nmhandle_t *handle = NULL; REQUIRE(params != NULL); REQUIRE(params->qname != NULL); @@ -814,10 +846,12 @@ ns_test_qctx_create(const ns_test_qctx_create_params_t *params, } /* - * Reference count for "client" is now at 2, so decrement it in order - * for it to drop to zero when "qctx" gets destroyed. + * The reference count for "client" is now at 2, so we need to + * decrement it in order for it to drop to zero when "qctx" gets + * destroyed. */ - isc_nmhandle_unref(client->handle); + handle = client->handle; + isc_nmhandle_detach(&handle); return (ISC_R_SUCCESS); @@ -826,7 +860,7 @@ detach_query: detach_view: dns_view_detach(&client->view); detach_client: - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->handle); return (result); } @@ -848,7 +882,7 @@ ns_test_qctx_destroy(query_ctx_t **qctxp) { dns_db_detach(&qctx->db); } if (qctx->client != NULL) { - isc_nmhandle_unref(qctx->client->handle); + isc_nmhandle_detach(&qctx->client->handle); } isc_mem_put(mctx, qctx, sizeof(*qctx)); diff --git a/lib/ns/update.c b/lib/ns/update.c index a30292f76c5..cb0ff5cd84b 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1554,7 +1554,7 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { client->nupdates++; event->ev_arg = client; - isc_nmhandle_ref(client->handle); + isc_nmhandle_attach(client->handle, &client->updatehandle); dns_zone_gettask(zone, &zonetask); isc_task_send(zonetask, ISC_EVENT_PTR(&event)); @@ -1572,7 +1572,6 @@ respond(ns_client_t *client, isc_result_t result) { client->message->rcode = dns_result_torcode(result); ns_client_send(client); - isc_nmhandle_unref(client->handle); return; msg_failure: @@ -1581,17 +1580,23 @@ msg_failure: "could not create update response message: %s", isc_result_totext(msg_result)); ns_client_drop(client, msg_result); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->reqhandle); } void -ns_update_start(ns_client_t *client, isc_result_t sigresult) { +ns_update_start(ns_client_t *client, isc_nmhandle_t *handle, + isc_result_t sigresult) { dns_message_t *request = client->message; isc_result_t result; dns_name_t *zonename; dns_rdataset_t *zone_rdataset; dns_zone_t *zone = NULL, *raw = NULL; + /* + * Attach to the request handle + */ + isc_nmhandle_attach(handle, &client->reqhandle); + /* * Interpret the zone section. */ @@ -1661,6 +1666,8 @@ ns_update_start(ns_client_t *client, isc_result_t sigresult) { default: FAILC(DNS_R_NOTAUTH, "not authoritative for update zone"); } + + isc_nmhandle_detach(&client->reqhandle); return; failure: @@ -1678,6 +1685,7 @@ failure: if (zone != NULL) { dns_zone_detach(&zone); } + isc_nmhandle_detach(&client->reqhandle); } /*% @@ -3446,6 +3454,7 @@ common: } uev->ev_type = DNS_EVENT_UPDATEDONE; uev->ev_action = updatedone_action; + isc_task_send(client->task, &event); INSIST(ver == NULL); @@ -3459,8 +3468,9 @@ updatedone_action(isc_task_t *task, isc_event_t *event) { UNUSED(task); - INSIST(event->ev_type == DNS_EVENT_UPDATEDONE); - INSIST(task == client->task); + REQUIRE(event->ev_type == DNS_EVENT_UPDATEDONE); + REQUIRE(task == client->task); + REQUIRE(client->updatehandle == client->handle); INSIST(client->nupdates > 0); switch (uev->result) { @@ -3477,16 +3487,18 @@ updatedone_action(isc_task_t *task, isc_event_t *event) { if (uev->zone != NULL) { dns_zone_detach(&uev->zone); } + client->nupdates--; + respond(client, uev->result); + isc_event_free(&event); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->updatehandle); } /*% * Update forwarding support. */ - static void forward_fail(isc_task_t *task, isc_event_t *event) { ns_client_t *client = (ns_client_t *)event->ev_arg; @@ -3497,7 +3509,7 @@ forward_fail(isc_task_t *task, isc_event_t *event) { client->nupdates--; respond(client, DNS_R_SERVFAIL); isc_event_free(&event); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->updatehandle); } static void @@ -3517,6 +3529,7 @@ forward_callback(void *arg, isc_result_t result, dns_message_t *answer) { uev->answer = answer; inc_stats(client, zone, ns_statscounter_updaterespfwd); } + isc_task_send(client->task, ISC_EVENT_PTR(&uev)); dns_zone_detach(&zone); } @@ -3533,7 +3546,7 @@ forward_done(isc_task_t *task, isc_event_t *event) { ns_client_sendraw(client, uev->answer); dns_message_detach(&uev->answer); isc_event_free(&event); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->updatehandle); } static void @@ -3585,7 +3598,7 @@ send_forward_event(ns_client_t *client, dns_zone_t *zone) { namebuf, classbuf); dns_zone_gettask(zone, &zonetask); - isc_nmhandle_ref(client->handle); + isc_nmhandle_attach(client->handle, &client->updatehandle); isc_task_send(zonetask, ISC_EVENT_PTR(&event)); if (event != NULL) { diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index 363e25aa0aa..5d97aff94b3 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include @@ -34,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -1160,7 +1158,7 @@ failure: NS_LOGMODULE_XFER_OUT, ISC_LOG_DEBUG(3), "zone transfer setup failed"); ns_client_error(client, result); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->reqhandle); } } @@ -1244,11 +1242,6 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, unsigned int id, xfr->txmem = mem; xfr->txmemlen = len; -#if 0 - CHECK(dns_timer_setidle(xfr->client->timer, - maxtime,idletime,false)); -#endif /* if 0 */ - /* * Register a shutdown callback with the client, so that we * can stop the transfer immediately when the client task @@ -1540,15 +1533,17 @@ sendstream(xfrout_ctx_t *xfr) { xfrout_log(xfr, ISC_LOG_DEBUG(8), "sending TCP message of %d bytes", used.length); - CHECK(isc_nm_send(xfr->client->handle, &used, xfrout_senddone, - xfr)); + isc_nmhandle_attach(xfr->client->handle, + &xfr->client->sendhandle); + CHECK(isc_nm_send(xfr->client->sendhandle, &used, + xfrout_senddone, xfr)); xfr->sends++; xfr->cbytes = used.length; } else { xfrout_log(xfr, ISC_LOG_DEBUG(8), "sending IXFR UDP response"); ns_client_send(xfr->client); xfr->stream->methods->pause(xfr->stream); - isc_nmhandle_unref(xfr->client->handle); + isc_nmhandle_detach(&xfr->client->reqhandle); xfrout_ctx_destroy(&xfr); return; } @@ -1591,6 +1586,10 @@ failure: return; } + if (xfr->client->sendhandle != NULL) { + isc_nmhandle_detach(&xfr->client->sendhandle); + } + xfrout_fail(xfr, result, "sending zone data"); } @@ -1643,6 +1642,8 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { xfr->sends--; INSIST(xfr->sends == 0); + isc_nmhandle_detach(&xfr->client->sendhandle); + /* * Update transfer statistics if sending succeeded, accounting for the * two-byte TCP length prefix included in the number of bytes sent. @@ -1652,10 +1653,6 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { xfr->stats.nbytes += xfr->cbytes; } -#if 0 - (void)isc_timer_touch(xfr->client->timer); -#endif /* if 0 */ - if (xfr->shuttingdown) { xfrout_maybe_destroy(xfr); } else if (result != ISC_R_SUCCESS) { @@ -1683,9 +1680,12 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { xfr->stats.nbytes, (unsigned int)(msecs / 1000), (unsigned int)(msecs % 1000), (unsigned int)persec); + /* + * We're done, unreference the handle and destroy the xfr + * context. + */ + isc_nmhandle_detach(&xfr->client->reqhandle); xfrout_ctx_destroy(&xfr); - /* We're done, unreference the handle */ - isc_nmhandle_unref(handle); } } @@ -1699,23 +1699,11 @@ xfrout_fail(xfrout_ctx_t *xfr, isc_result_t result, const char *msg) { static void xfrout_maybe_destroy(xfrout_ctx_t *xfr) { - INSIST(xfr->shuttingdown); -#if 0 - if (xfr->sends > 0) { - /* - * If we are currently sending, cancel it and wait for - * cancel event before destroying the context. - */ - isc_socket_cancel(xfr->client->tcpsocket,xfr->client->task, - ISC_SOCKCANCEL_SEND); - } else { -#endif /* if 0 */ + REQUIRE(xfr->shuttingdown); + ns_client_drop(xfr->client, ISC_R_CANCELED); - isc_nmhandle_unref(xfr->client->handle); + isc_nmhandle_detach(&xfr->client->reqhandle); xfrout_ctx_destroy(&xfr); -#if 0 -} -#endif /* if 0 */ } static void -- 2.47.3