From 57b4dde9749c88d21d1dc8afd22201224cf83cab 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 control channel connection objects now contain: - readhandle: held while waiting for a read callback - sendhandle: held while waiting for a send callback - cmdhandle: held while an rndc command is running httpd connections contain: - readhandle: held while waiting for a read callback - sendhandle: held while waiting for a send callback --- bin/named/controlconf.c | 149 +++++++++++++----------- bin/named/server.c | 5 +- bin/rndc/rndc.c | 69 +++++++---- bin/tests/system/rndc/ns4/named.conf.in | 1 + lib/isc/httpd.c | 31 ++--- 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/isccc/ccmsg.c | 8 +- lib/isccc/include/isccc/ccmsg.h | 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.am | 5 +- lib/ns/tests/notify_test.c | 8 +- lib/ns/tests/nstest.c | 47 ++++++-- lib/ns/tests/wrap.c | 19 ++- lib/ns/update.c | 35 ++++-- lib/ns/xfrout.c | 52 ++++----- 26 files changed, 416 insertions(+), 309 deletions(-) diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index c69650ace5..6e91d4f782 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -67,9 +67,11 @@ struct controlkey { }; struct controlconnection { - isc_nmhandle_t *handle; + isc_nmhandle_t *readhandle; + isc_nmhandle_t *sendhandle; + isc_nmhandle_t *cmdhandle; isccc_ccmsg_t ccmsg; - bool ccmsg_valid; + bool reading; bool sending; controllistener_t *listener; isccc_sexpr_t *ctrl; @@ -221,10 +223,8 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { conn->sending = false; - isc_nmhandle_unref(handle); - - if (result == ISC_R_CANCELED) { - return; + if (listener->controls->shuttingdown || result == ISC_R_CANCELED) { + goto cleanup_sendhandle; } else if (result != ISC_R_SUCCESS) { char socktext[ISC_SOCKADDR_FORMATSIZE]; @@ -233,16 +233,31 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { NAMED_LOGMODULE_CONTROL, ISC_LOG_WARNING, "error sending command response to %s: %s", socktext, isc_result_totext(result)); - return; + goto cleanup_sendhandle; } + isc_nmhandle_attach(handle, &conn->readhandle); + conn->reading = true; + + isc_nmhandle_detach(&conn->sendhandle); + result = isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn); if (result != ISC_R_SUCCESS) { + isc_nmhandle_detach(&conn->readhandle); + conn->reading = false; + maybe_free_listener(listener); + + return; } listener->listening = true; + + return; + +cleanup_sendhandle: + isc_nmhandle_detach(&conn->sendhandle); } static inline void @@ -257,6 +272,25 @@ log_invalid(isccc_ccmsg_t *ccmsg, isc_result_t result) { isc_result_totext(result)); } +static void +conn_cleanup(controlconnection_t *conn) { + controllistener_t *listener = conn->listener; + + if (conn->response != NULL) { + isccc_sexpr_free(&conn->response); + } + if (conn->request != NULL) { + isccc_sexpr_free(&conn->request); + } + if (conn->secret.rstart != NULL) { + isc_mem_put(listener->mctx, conn->secret.rstart, + REGION_SIZE(conn->secret)); + } + if (conn->text != NULL) { + isc_buffer_free(&conn->text); + } +} + static void control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) { controlconnection_t *conn = (controlconnection_t *)arg; @@ -324,29 +358,21 @@ control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) { r.base = conn->buffer->base; r.length = conn->buffer->used; - isc_nmhandle_ref(handle); + isc_nmhandle_attach(handle, &conn->sendhandle); conn->sending = true; - result = isc_nm_send(handle, &r, control_senddone, conn); + conn_cleanup(conn); + + isc_nmhandle_detach(&conn->cmdhandle); + + result = isc_nm_send(conn->sendhandle, &r, control_senddone, conn); if (result != ISC_R_SUCCESS) { - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&conn->sendhandle); conn->sending = false; } + return; cleanup: - if (conn->response != NULL) { - isccc_sexpr_free(&conn->response); - } - if (conn->request != NULL) { - isccc_sexpr_free(&conn->request); - } - - if (conn->secret.rstart != NULL) { - isc_mem_put(listener->mctx, conn->secret.rstart, - REGION_SIZE(conn->secret)); - } - if (conn->text != NULL) { - isc_buffer_free(&conn->text); - } + conn_cleanup(conn); } static void @@ -356,17 +382,17 @@ control_command(isc_task_t *task, isc_event_t *event) { UNUSED(task); - /* - * An extra ref and two unrefs are needed here to - * ensure the handle isn't cleaned up if we're running - * an "rndc stop" command. - */ - isc_nmhandle_ref(conn->handle); + if (listener->controls->shuttingdown) { + isc_nmhandle_detach(&conn->cmdhandle); + conn_cleanup(conn); + goto done; + } + conn->result = named_control_docommand(conn->request, listener->readonly, &conn->text); - control_respond(conn->handle, conn->result, conn); - isc_nmhandle_unref(conn->handle); - isc_nmhandle_unref(conn->handle); + control_respond(conn->cmdhandle, conn->result, conn); + +done: isc_event_free(&event); } @@ -380,26 +406,21 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_time_t exp; uint32_t nonce; - conn->ccmsg_valid = false; + conn->reading = false; /* Is the server shutting down? */ if (listener->controls->shuttingdown) { - return; + goto cleanup_readhandle; } if (result != ISC_R_SUCCESS) { if (result == ISC_R_CANCELED) { - /* - * Don't bother with any more scheduled command events. - */ listener->controls->shuttingdown = true; - isc_task_purge(named_g_server->task, NULL, - NAMED_EVENT_COMMAND, NULL); } else if (result != ISC_R_EOF) { log_invalid(&conn->ccmsg, result); } - return; + goto cleanup_readhandle; } for (key = ISC_LIST_HEAD(listener->keys); key != NULL; @@ -424,7 +445,7 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { REGION_SIZE(conn->secret)); if (result != ISCCC_R_BADAUTH) { log_invalid(&conn->ccmsg, result); - return; + goto cleanup; } } @@ -496,7 +517,8 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_buffer_allocate(listener->mctx, &conn->text, 2 * 2048); - conn->ccmsg_valid = true; + isc_nmhandle_attach(handle, &conn->cmdhandle); + isc_nmhandle_detach(&conn->readhandle); if (conn->nonce == 0) { /* @@ -513,26 +535,23 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { /* * Trigger the command. */ - isc_nmhandle_ref(handle); + event = isc_event_allocate(listener->mctx, conn, NAMED_EVENT_COMMAND, control_command, conn, sizeof(isc_event_t)); isc_task_send(named_g_server->task, &event); + return; cleanup: - if (conn->response != NULL) { - isccc_sexpr_free(&conn->response); - } - if (conn->request != NULL) { - isccc_sexpr_free(&conn->request); - } + conn_cleanup(conn); - if (conn->secret.rstart != NULL) { - isc_mem_put(listener->mctx, conn->secret.rstart, - REGION_SIZE(conn->secret)); - } - if (conn->text != NULL) { - isc_buffer_free(&conn->text); +cleanup_readhandle: + /* + * readhandle could be NULL if we're shutting down, + * but if not we need to detach it. + */ + if (conn->readhandle != NULL) { + isc_nmhandle_detach(&conn->readhandle); } } @@ -545,7 +564,7 @@ conn_reset(void *arg) { isc_buffer_free(&conn->buffer); } - if (conn->ccmsg_valid) { + if (conn->reading) { isccc_ccmsg_cancelread(&conn->ccmsg); return; } @@ -583,18 +602,11 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3), "allocate new control connection"); - *conn = (controlconnection_t){ .handle = NULL }; - } - - if (conn->handle == NULL) { isc_nmhandle_setdata(handle, conn, conn_reset, conn_put); - } else { - INSIST(conn->handle == handle); } - *conn = (controlconnection_t){ .handle = handle, - .listener = listener, - .ccmsg_valid = true, + *conn = (controlconnection_t){ .listener = listener, + .reading = false, .alg = DST_ALG_UNKNOWN }; isccc_ccmsg_init(listener->mctx, handle, &conn->ccmsg); @@ -604,9 +616,14 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { ISC_LINK_INIT(conn, link); + isc_nmhandle_attach(handle, &conn->readhandle); + conn->reading = true; + result = isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn); if (result != ISC_R_SUCCESS) { + isc_nmhandle_detach(&conn->readhandle); + conn->reading = false; goto cleanup; } diff --git a/bin/named/server.c b/bin/named/server.c index bc6c1e7b3a..af819f3d92 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -14866,14 +14866,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/rndc/rndc.c b/bin/rndc/rndc.c index de6b1850fa..a05b1ee96b 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -89,6 +90,8 @@ static uint32_t serial; static bool quiet = false; static bool showresult = false; static bool shuttingdown = false; +static isc_nmhandle_t *recvdone_handle = NULL; +static isc_nmhandle_t *recvnonce_handle = NULL; static void rndc_startconnect(isc_sockaddr_t *addr); @@ -288,19 +291,21 @@ get_addresses(const char *host, in_port_t port) { static void rndc_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { - UNUSED(arg); + isc_nmhandle_t *sendhandle = (isc_nmhandle_t *)arg; if (result != ISC_R_SUCCESS) { fatal("send failed: %s", isc_result_totext(result)); } + REQUIRE(sendhandle == handle); + isc_nmhandle_detach(&sendhandle); + if (atomic_fetch_sub_release(&sends, 1) == 1 && atomic_load_acquire(&recvs) == 0) { shuttingdown = true; isc_task_shutdown(rndc_task); isc_app_shutdown(); - isc_nmhandle_unref(handle); } } @@ -315,10 +320,12 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { REQUIRE(ccmsg != NULL); - atomic_fetch_sub_release(&recvs, 1); - if (shuttingdown && (result == ISC_R_EOF || result == ISC_R_CANCELED)) { - isc_nmhandle_unref(handle); + atomic_fetch_sub_release(&recvs, 1); + if (handle != NULL) { + REQUIRE(recvdone_handle == handle); + isc_nmhandle_detach(&recvdone_handle); + } return; } else if (result == ISC_R_EOF) { fatal("connection to remote host closed.\n" @@ -376,10 +383,13 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_sexpr_free(&response); + REQUIRE(recvdone_handle == handle); + isc_nmhandle_detach(&recvdone_handle); + if (atomic_load_acquire(&sends) == 0 && - atomic_load_acquire(&recvs) == 0) { + atomic_fetch_sub_release(&recvs, 1) == 1) + { shuttingdown = true; - isc_nmhandle_unref(handle); isc_task_shutdown(rndc_task); isc_app_shutdown(); } @@ -400,10 +410,12 @@ rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { REQUIRE(ccmsg != NULL); - atomic_fetch_sub_release(&recvs, 1); - if (shuttingdown && result == ISC_R_EOF) { - isc_nmhandle_unref(handle); + atomic_fetch_sub_release(&recvs, 1); + if (handle != NULL) { + REQUIRE(recvnonce_handle == handle); + isc_nmhandle_detach(&recvnonce_handle); + } return; } else if (result == ISC_R_EOF) { fatal("connection to remote host closed.\n" @@ -467,12 +479,19 @@ rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { r.base = databuf->base; r.length = databuf->used; + isc_nmhandle_attach(handle, &recvdone_handle); + atomic_fetch_add_relaxed(&recvs, 1); DO("schedule recv", isccc_ccmsg_readmessage(ccmsg, rndc_recvdone, ccmsg)); - atomic_fetch_add_relaxed(&recvs, 1); - DO("send message", isc_nm_send(handle, &r, rndc_senddone, NULL)); + isc_nmhandle_t *sendhandle = NULL; + isc_nmhandle_attach(handle, &sendhandle); atomic_fetch_add_relaxed(&sends, 1); + DO("send message", isc_nm_send(handle, &r, rndc_senddone, sendhandle)); + + REQUIRE(recvnonce_handle == handle); + isc_nmhandle_detach(&recvnonce_handle); + atomic_fetch_sub_release(&recvs, 1); isccc_sexpr_free(&response); isccc_sexpr_free(&request); @@ -488,12 +507,12 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_time_t now; isc_region_t r; isc_buffer_t b; + isc_nmhandle_t *connhandle = NULL; REQUIRE(ccmsg != NULL); - atomic_fetch_sub_release(&connects, 1); - if (result != ISC_R_SUCCESS) { + atomic_fetch_sub_release(&connects, 1); isc_sockaddr_format(&serveraddrs[currentaddr], socktext, sizeof(socktext)); if (++currentaddr < nserveraddrs) { @@ -507,6 +526,8 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_result_totext(result)); } + isc_nmhandle_attach(handle, &connhandle); + isc_stdtime_get(&now); DO("create message", isccc_cc_createmessage(1, NULL, NULL, ++serial, now, now + 60, &request)); @@ -534,14 +555,18 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_ccmsg_init(rndc_mctx, handle, ccmsg); isccc_ccmsg_setmaxsize(ccmsg, 1024 * 1024); - isc_nmhandle_ref(handle); - + isc_nmhandle_attach(handle, &recvnonce_handle); + atomic_fetch_add_relaxed(&recvs, 1); DO("schedule recv", isccc_ccmsg_readmessage(ccmsg, rndc_recvnonce, ccmsg)); - atomic_fetch_add_relaxed(&recvs, 1); - DO("send message", isc_nm_send(handle, &r, rndc_senddone, NULL)); + isc_nmhandle_t *sendhandle = NULL; + isc_nmhandle_attach(handle, &sendhandle); atomic_fetch_add_relaxed(&sends, 1); + DO("send message", isc_nm_send(handle, &r, rndc_senddone, sendhandle)); + + isc_nmhandle_detach(&connhandle); + atomic_fetch_sub_release(&connects, 1); isccc_sexpr_free(&request); } @@ -573,11 +598,11 @@ rndc_startconnect(isc_sockaddr_t *addr) { ISC_UNREACHABLE(); } + atomic_fetch_add_relaxed(&connects, 1); DO("create connection", isc_nm_tcpconnect(netmgr, (isc_nmiface_t *)local, (isc_nmiface_t *)addr, rndc_connected, &rndc_ccmsg, 0)); - atomic_fetch_add_relaxed(&connects, 1); } static void @@ -1068,16 +1093,18 @@ main(int argc, char **argv) { isc_task_detach(&rndc_task); isc_taskmgr_destroy(&taskmgr); - isc_nm_destroy(&netmgr); + isc_nm_closedown(netmgr); /* * Note: when TCP connections are shut down, there will be a final * call to the isccc callback routine with &rndc_ccmsg as its * argument. We therefore need to delay invalidating it until - * after the netmgr is destroyed. + * after the netmgr is closed down. */ isccc_ccmsg_invalidate(&rndc_ccmsg); + isc_nm_destroy(&netmgr); + isc_log_destroy(&log); isc_log_setcontext(NULL); diff --git a/bin/tests/system/rndc/ns4/named.conf.in b/bin/tests/system/rndc/ns4/named.conf.in index 2466e69559..4f009a7c2d 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/httpd.c b/lib/isc/httpd.c index 688c873daa..1b893fce65 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -75,7 +75,10 @@ struct isc_httpd { isc_httpdmgr_t *mgr; /*%< our parent */ ISC_LINK(isc_httpd_t) link; - isc_nmhandle_t *handle; + isc_nmhandle_t *handle; /* Permanent pointer to handle */ + isc_nmhandle_t *readhandle; /* Waiting for a read callback */ + isc_nmhandle_t *sendhandle; /* Waiting for a send callback */ + state_t state; int flags; @@ -636,6 +639,7 @@ new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) { if (httpd->handle == NULL) { isc_nmhandle_setdata(handle, httpd, httpd_reset, httpd_put); + httpd->handle = handle; } else { INSIST(httpd->handle == handle); } @@ -663,10 +667,10 @@ new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) { ISC_LIST_APPEND(httpdmgr->running, httpd, link); UNLOCK(&httpdmgr->lock); - isc_nmhandle_ref(handle); + isc_nmhandle_attach(handle, &httpd->readhandle); result = isc_nm_read(handle, httpd_request, httpdmgr); if (result != ISC_R_SUCCESS) { - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&httpd->readhandle); } return (result); @@ -849,7 +853,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, REQUIRE(httpd->state == RECV); if (eresult != ISC_R_SUCCESS) { - goto done; + goto cleanup_readhandle; } result = process_request(httpd, region); @@ -858,9 +862,9 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, /* don't unref, continue reading */ return; } - goto done; + goto cleanup_readhandle; } else if (result != ISC_R_SUCCESS) { - goto done; + goto cleanup_readhandle; } isc_buffer_initnull(&httpd->bodybuffer); @@ -958,18 +962,18 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_nm_pauseread(handle); httpd->state = SEND; - isc_nmhandle_ref(handle); + isc_nmhandle_attach(handle, &httpd->sendhandle); result = isc_nm_send(handle, &r, httpd_senddone, httpd); if (result != ISC_R_SUCCESS) { - isc_nm_resumeread(handle); - isc_nmhandle_unref(handle); + isc_nmhandle_detach(&httpd->sendhandle); httpd->state = RECV; + isc_nm_resumeread(handle); } return; -done: - isc_nmhandle_unref(handle); +cleanup_readhandle: + isc_nmhandle_detach(&httpd->readhandle); } void @@ -990,7 +994,7 @@ isc_httpdmgr_shutdown(isc_httpdmgr_t **httpdmgrp) { httpd = ISC_LIST_HEAD(httpdmgr->running); while (httpd != NULL) { - isc_nmhandle_unref(httpd->handle); + isc_nm_cancelread(httpd->readhandle); httpd = ISC_LIST_NEXT(httpd, link); } UNLOCK(&httpdmgr->lock); @@ -1137,9 +1141,10 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { return; } + isc_nmhandle_detach(&httpd->sendhandle); + httpd->state = RECV; isc_nm_resumeread(handle); - isc_nmhandle_unref(handle); } isc_result_t diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index b1c92ce348..991e14028d 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 abd4566669..ab8852d8d0 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 114279710c..e4dd3fecec 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 107cde97b6..84bf28d206 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 9a4013f666..d167b91727 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 bc5de5aa8e..8b87058f7d 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -436,15 +436,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/isccc/ccmsg.c b/lib/isccc/ccmsg.c index 5e80cf82da..98b67622ee 100644 --- a/lib/isccc/ccmsg.c +++ b/lib/isccc/ccmsg.c @@ -108,7 +108,6 @@ recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, done: isc_nm_pauseread(handle); ccmsg->cb(handle, ccmsg->result, ccmsg->cbarg); - isc_nmhandle_unref(handle); } void @@ -149,18 +148,14 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg) { ccmsg->result = ISC_R_UNEXPECTED; /* unknown right now */ ccmsg->length_received = false; - isc_nmhandle_ref(ccmsg->handle); if (ccmsg->reading) { result = isc_nm_resumeread(ccmsg->handle); } else { result = isc_nm_read(ccmsg->handle, recv_data, ccmsg); ccmsg->reading = true; } - if (result == ISC_R_CANCELED) { + if (result != ISC_R_SUCCESS) { ccmsg->reading = false; - } else if (result != ISC_R_SUCCESS) { - ccmsg->reading = false; - isc_nmhandle_unref(ccmsg->handle); } return (result); @@ -172,6 +167,7 @@ isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg) { if (ccmsg->reading) { isc_nm_cancelread(ccmsg->handle); + ccmsg->reading = false; } } diff --git a/lib/isccc/include/isccc/ccmsg.h b/lib/isccc/include/isccc/ccmsg.h index baa2c19bb3..31d1490824 100644 --- a/lib/isccc/include/isccc/ccmsg.h +++ b/lib/isccc/include/isccc/ccmsg.h @@ -58,7 +58,9 @@ void isccc_ccmsg_init(isc_mem_t *mctx, isc_nmhandle_t *handle, isccc_ccmsg_t *ccmsg); /*% * Associate a cc message state with a given memory context and - * netmgr handle. + * netmgr handle. (Note that the caller must hold a reference to + * the handle during asynchronous ccmsg operations; the ccmsg code + * does not hold the reference itself.) * * Requires: * diff --git a/lib/ns/client.c b/lib/ns/client.c index 7e1bde9875..db3d40fc4e 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -275,7 +275,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) { @@ -284,7 +284,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 @@ -325,13 +325,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; + REQUIRE(client->sendhandle == NULL); + isc_buffer_usedregion(buffer, &r); - - INSIST(client->handle != NULL); - - return (isc_nm_send(client->handle, &r, client_senddone, client)); + 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 @@ -383,7 +388,6 @@ done: } ns_client_drop(client, result); - isc_nmhandle_unref(client->handle); } void @@ -405,17 +409,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"); @@ -591,12 +591,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: @@ -626,12 +621,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: @@ -1683,6 +1673,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; } @@ -1697,8 +1688,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); @@ -1968,8 +1958,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); } @@ -2174,8 +2163,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"); @@ -2185,14 +2173,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 bf386e869e..539866c0e7 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 555a6f7915..6c39c4344f 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 18e74dec15..a470c0da56 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 e3372c9a08..c86915555c 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 3dafc8cdbf..dec74e8f46 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 6147ff3705..3a64b0fc4e 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 @@ -2475,7 +2475,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 @@ -2518,7 +2518,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, @@ -2527,7 +2527,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); @@ -2732,7 +2732,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, @@ -2740,7 +2740,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); } } @@ -5703,6 +5703,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; @@ -5748,7 +5750,6 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { } dns_resolver_destroyfetch(&fetch); - isc_nmhandle_unref(client->handle); } /*% @@ -5940,14 +5941,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); @@ -11108,7 +11109,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; @@ -11118,6 +11119,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.am b/lib/ns/tests/Makefile.am index 092360cebd..38903266d5 100644 --- a/lib/ns/tests/Makefile.am +++ b/lib/ns/tests/Makefile.am @@ -32,7 +32,8 @@ notify_test_SOURCES = \ notify_test_LDFLAGS = \ $(LDFLAGS) \ - -Wl,--wrap=isc_nmhandle_unref + -Wl,--wrap=isc_nmhandle_attach \ + -Wl,--wrap=isc_nmhandle_detach query_test_SOURCES = \ query_test.c \ @@ -40,7 +41,7 @@ query_test_SOURCES = \ query_test_LDFLAGS = \ $(LDFLAGS) \ - -Wl,--wrap=isc_nmhandle_unref + -Wl,--wrap=isc_nmhandle_detach endif diff --git a/lib/ns/tests/notify_test.c b/lib/ns/tests/notify_test.c index 4e3c5a63d9..05f1133535 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; @@ -130,13 +131,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); } int diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index e3a6990fc9..bc421272aa 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -78,10 +78,32 @@ 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_unref(isc_nmhandle_t *handle) { +__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_detach(isc_nmhandle_t **handlep) { + isc_nmhandle_t *handle = *handlep; ns_client_t *client = (ns_client_t *)handle; int i; @@ -90,7 +112,7 @@ __wrap_isc_nmhandle_unref(isc_nmhandle_t *handle) { break; } } - REQUIRE(i < 32); + INSIST(i < 32); if (atomic_fetch_sub(&client_refs[i], 1) == 1) { dns_view_detach(&client->view); @@ -99,6 +121,8 @@ __wrap_isc_nmhandle_unref(isc_nmhandle_t *handle) { ns__client_put_cb(client); isc_mem_put(mctx, client, sizeof(ns_client_t)); } + + *handlep = NULL; return; } @@ -743,11 +767,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 { @@ -760,6 +786,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); @@ -810,17 +837,19 @@ 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); destroy_query: dns_message_destroy(&client->message); detach_client: - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->handle); return (result); } @@ -842,7 +871,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/tests/wrap.c b/lib/ns/tests/wrap.c index 41b8449fc6..810a1d5d03 100644 --- a/lib/ns/tests/wrap.c +++ b/lib/ns/tests/wrap.c @@ -26,15 +26,22 @@ #include /* - * This overrides calls to isc_nmhandle_unref(), sending them to - * __wrap_isc_nmhandle_unref(), when libtool is in use and LD_WRAP - * can't be used. + * This overrides calls to isc_nmhandle_attach/detach(), sending them to + * __wrap_isc_nmhandle_attach/detach() instead, when libtool is in use + * and LD_WRAP can't be used. */ +void +__wrap_isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp); extern void -__wrap_isc_nmhandle_unref(isc_nmhandle_t *handle); +__wrap_isc_nmhandle_detach(isc_nmhandle_t **handlep); 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 **handlep) { + __wrap_isc_nmhandle_detach(handlep); } diff --git a/lib/ns/update.c b/lib/ns/update.c index 9b780a62b9..d440955acd 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1567,7 +1567,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)); @@ -1585,7 +1585,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: @@ -1594,17 +1593,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. */ @@ -1673,6 +1678,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: @@ -1690,6 +1697,7 @@ failure: if (zone != NULL) { dns_zone_detach(&zone); } + isc_nmhandle_detach(&client->reqhandle); } /*% @@ -3497,6 +3505,7 @@ common: } uev->ev_type = DNS_EVENT_UPDATEDONE; uev->ev_action = updatedone_action; + isc_task_send(client->task, &event); INSIST(ver == NULL); @@ -3510,8 +3519,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) { @@ -3528,16 +3538,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; @@ -3548,7 +3560,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 @@ -3568,6 +3580,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); } @@ -3584,7 +3597,7 @@ forward_done(isc_task_t *task, isc_event_t *event) { ns_client_sendraw(client, uev->answer); dns_message_destroy(&uev->answer); isc_event_free(&event); - isc_nmhandle_unref(client->handle); + isc_nmhandle_detach(&client->updatehandle); } static void @@ -3636,7 +3649,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 b6e46578b5..951720bcea 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 @@ -1192,7 +1190,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); } } @@ -1276,11 +1274,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 @@ -1572,15 +1565,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; } @@ -1623,6 +1618,10 @@ failure: return; } + if (xfr->client->sendhandle != NULL) { + isc_nmhandle_detach(&xfr->client->sendhandle); + } + xfrout_fail(xfr, result, "sending zone data"); } @@ -1675,6 +1674,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. @@ -1684,10 +1685,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) { @@ -1716,9 +1713,12 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { (unsigned int)(msecs % 1000), (unsigned int)persec, xfr->end_serial); + /* + * 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); } } @@ -1732,23 +1732,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