diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index 41751ed890..c702656431 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -1288,11 +1288,11 @@ dns_dispatch_detach(dns_dispatch_t **dispp) { } isc_result_t -dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, - unsigned int timeout, const isc_sockaddr_t *dest, - dispatch_cb_t connected, dispatch_cb_t sent, - dispatch_cb_t response, void *arg, - dns_messageid_t *idp, dns_dispentry_t **resp) { +dns_dispatch_add(dns_dispatch_t *disp, unsigned int options, + unsigned int timeout, const isc_sockaddr_t *dest, + dispatch_cb_t connected, dispatch_cb_t sent, + dispatch_cb_t response, void *arg, dns_messageid_t *idp, + dns_dispentry_t **resp) { dns_dispentry_t *res = NULL; dns_qid_t *qid = NULL; in_port_t localport = 0; @@ -1476,7 +1476,65 @@ dns_dispatch_getnext(dns_dispentry_t *resp) { } void -dns_dispatch_removeresponse(dns_dispentry_t **respp) { +dns_dispatch_cancel(dns_dispentry_t **respp) { + dns_dispentry_t *resp = NULL; + + REQUIRE(respp != NULL); + + resp = *respp; + + REQUIRE(VALID_RESPONSE(resp)); + + resp->canceled = true; + + /* Connected UDP. */ + if (resp->handle != NULL) { + isc_nm_cancelread(resp->handle); + goto done; + } + + /* TCP pending connection. */ + if (ISC_LINK_LINKED(resp, plink)) { + dns_dispentry_t *copy = resp; + + ISC_LIST_UNLINK(resp->disp->pending, resp, plink); + if (resp->connected != NULL) { + resp->connected(ISC_R_CANCELED, NULL, resp->arg); + } + + /* + * We need to detach twice if we were pending + * connection - once to take the place of the + * detach in tcp_connected() or udp_connected() + * that we won't reach, and again later in + * dns_dispatch_done(). + */ + dispentry_detach(©); + goto done; + } + + /* + * Connected TCP, or unconnected UDP. + * + * If TCP, we don't want to cancel the dispatch + * unless this is the last resp waiting. + */ + if (ISC_LINK_LINKED(resp, alink)) { + ISC_LIST_UNLINK(resp->disp->active, resp, alink); + if (ISC_LIST_EMPTY(resp->disp->active) && + resp->disp->handle != NULL) { + isc_nm_cancelread(resp->disp->handle); + } else if (resp->response != NULL) { + resp->response(ISC_R_CANCELED, NULL, resp->arg); + } + } + +done: + dns_dispatch_done(respp); +} + +void +dns_dispatch_done(dns_dispentry_t **respp) { dns_dispatchmgr_t *mgr = NULL; dns_dispatch_t *disp = NULL; dns_dispentry_t *resp = NULL; @@ -1762,45 +1820,6 @@ dns_dispatch_send(dns_dispentry_t *resp, isc_region_t *r, isc_dscp_t dscp) { isc_nm_send(handle, r, send_done, resp); } -void -dns_dispatch_cancel(dns_dispentry_t *resp) { - REQUIRE(VALID_RESPONSE(resp)); - - resp->canceled = true; - - /* Connected UDP */ - if (resp->handle != NULL) { - isc_nm_cancelread(resp->handle); - return; - } - - /* TCP pending connection. */ - if (ISC_LINK_LINKED(resp, plink)) { - ISC_LIST_UNLINK(resp->disp->pending, resp, plink); - if (resp->connected != NULL) { - resp->connected(ISC_R_CANCELED, NULL, resp->arg); - dispentry_detach(&resp); - } - return; - } - - /* - * Connected TCP, or unconnected UDP. - * - * If TCP, we don't want to cancel the dispatch - * unless this is the last resp waiting. - */ - if (ISC_LINK_LINKED(resp, alink)) { - ISC_LIST_UNLINK(resp->disp->active, resp, alink); - if (ISC_LIST_EMPTY(resp->disp->active) && - resp->disp->handle != NULL) { - isc_nm_cancelread(resp->disp->handle); - } else if (resp->response != NULL) { - resp->response(ISC_R_CANCELED, NULL, resp->arg); - } - } -} - isc_result_t dns_dispatch_getlocaladdress(dns_dispatch_t *disp, isc_sockaddr_t *addrp) { REQUIRE(VALID_DISPATCH(disp)); diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index 93f676f5f9..572fa95d19 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -25,7 +25,7 @@ * MP: * *\li All locking is performed internally to each dispatch. - * Restrictions apply to dns_dispatch_removeresponse(). + * Restrictions apply to dns_dispatch_done(). * * Reliability: * @@ -225,17 +225,7 @@ isc_result_t dns_dispatch_connect(dns_dispentry_t *resp); /*%< * Connect to the remote server configured in 'resp' and run the - * connect callback that was set up via dns_dispatch_addresponse(). - * - * Requires: - *\li 'resp' is valid. - */ - -void -dns_dispatch_cancel(dns_dispentry_t *resp); -/*%< - * Cancel pending connects in 'resp', by setting a flag so that - * a read is not started when the connect handler runs. + * connect callback that was set up via dns_dispatch_add(). * * Requires: *\li 'resp' is valid. @@ -274,11 +264,11 @@ typedef void (*dispatch_cb_t)(isc_result_t eresult, isc_region_t *region, void *cbarg); isc_result_t -dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, - unsigned int timeout, const isc_sockaddr_t *dest, - dispatch_cb_t connected, dispatch_cb_t sent, - dispatch_cb_t response, void *arg, - dns_messageid_t *idp, dns_dispentry_t **resp); +dns_dispatch_add(dns_dispatch_t *disp, unsigned int options, + unsigned int timeout, const isc_sockaddr_t *dest, + dispatch_cb_t connected, dispatch_cb_t sent, + dispatch_cb_t response, void *arg, dns_messageid_t *idp, + dns_dispentry_t **resp); /*%< * Add a response entry for this dispatch. * @@ -316,13 +306,27 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, */ void -dns_dispatch_removeresponse(dns_dispentry_t **resp); +dns_dispatch_done(dns_dispentry_t **respp); /*%< - * Stops the flow of responses for the provided id and destination. + * Disconnects a dispatch response entry from its dispatch and shuts it + * down. This is called when the dispatch is complete; use + * dns_dispatch_cancel() if it is still pending. * * Requires: *\li "resp" != NULL and "*resp" contain a value previously allocated - * by dns_dispatch_addresponse(); + * by dns_dispatch_add(); + */ + +void +dns_dispatch_cancel(dns_dispentry_t **respp); +/*%< + * Cancel all pending connects and reads in a dispatch entry, + * then call dns_dispatch_done(). This is used when the caller + * cancels a dispatch response before it has completed. + * + * Requires: + *\li "resp" != NULL and "*resp" contain a value previously allocated + * by dns_dispatch_add(); */ isc_result_t diff --git a/lib/dns/request.c b/lib/dns/request.c index 2085575661..0e3619d93a 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -559,10 +559,10 @@ again: } req_attach(request, &rclone); - result = dns_dispatch_addresponse( - request->dispatch, dispopt, request->timeout, destaddr, - req_connected, req_senddone, req_response, request, &id, - &request->dispentry); + result = dns_dispatch_add(request->dispatch, dispopt, request->timeout, + destaddr, req_connected, req_senddone, + req_response, request, &id, + &request->dispentry); if (result != ISC_R_SUCCESS) { if ((options & DNS_REQUESTOPT_FIXEDID) != 0 && !newtcp) { newtcp = true; @@ -722,7 +722,7 @@ use_tcp: } req_attach(request, &rclone); - result = dns_dispatch_addresponse( + result = dns_dispatch_add( request->dispatch, 0, request->timeout, destaddr, req_connected, req_senddone, req_response, request, &id, &request->dispentry); if (result != ISC_R_SUCCESS) { @@ -737,7 +737,7 @@ use_tcp: */ req_detach(&rclone); dns_message_renderreset(message); - dns_dispatch_removeresponse(&request->dispentry); + dns_dispatch_done(&request->dispentry); dns_dispatch_detach(&request->dispatch); options |= DNS_REQUESTOPT_TCP; tcp = true; @@ -901,8 +901,7 @@ request_cancel(dns_request_t *request) { request->flags &= ~DNS_REQUEST_F_CONNECTING; if (request->dispentry != NULL) { - dns_dispatch_cancel(request->dispentry); - dns_dispatch_removeresponse(&request->dispentry); + dns_dispatch_cancel(&request->dispentry); } dns_dispatch_detach(&request->dispatch); @@ -977,8 +976,6 @@ dns_request_destroy(dns_request_t **requestp) { LOCK(&request->requestmgr->lock); LOCK(&request->requestmgr->locks[request->hash]); ISC_LIST_UNLINK(request->requestmgr->requests, request, link); - INSIST(!DNS_REQUEST_CONNECTING(request)); - INSIST(!DNS_REQUEST_SENDING(request)); UNLOCK(&request->requestmgr->locks[request->hash]); UNLOCK(&request->requestmgr->lock); @@ -986,7 +983,6 @@ dns_request_destroy(dns_request_t **requestp) { * These should have been cleaned up before the completion * event was sent. */ - INSIST(!ISC_LINK_LINKED(request, link)); INSIST(request->dispentry == NULL); INSIST(request->dispatch == NULL); @@ -1010,7 +1006,7 @@ req_connected(isc_result_t eresult, isc_region_t *region, void *arg) { request->flags &= ~DNS_REQUEST_F_CONNECTING; if (eresult == ISC_R_TIMEDOUT) { - dns_dispatch_removeresponse(&request->dispentry); + dns_dispatch_done(&request->dispentry); dns_dispatch_detach(&request->dispatch); req_sendevent(request, eresult); } else if (DNS_REQUEST_CANCELED(request)) { @@ -1103,7 +1099,7 @@ done: * Cleanup. */ if (request->dispentry != NULL) { - dns_dispatch_removeresponse(&request->dispentry); + dns_dispatch_done(&request->dispentry); } request_cancel(request); @@ -1192,7 +1188,7 @@ req_destroy(dns_request_t *request) { isc_event_free((isc_event_t **)&request->event); } if (request->dispentry != NULL) { - dns_dispatch_removeresponse(&request->dispentry); + dns_dispatch_done(&request->dispentry); } if (request->dispatch != NULL) { dns_dispatch_detach(&request->dispatch); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4ca5d8ab54..02c09aad34 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1142,7 +1142,7 @@ resquery_destroy(resquery_t *query) { } if (query->dispentry != NULL) { - dns_dispatch_removeresponse(&query->dispentry); + dns_dispatch_done(&query->dispentry); } if (query->dispatch != NULL) { @@ -1229,12 +1229,12 @@ fctx_cancelquery(resquery_t **queryp, isc_time_t *finish, bool no_response, query = *queryp; fctx = query->fctx; - FCTXTRACE("cancelquery"); - if (RESQUERY_CANCELED(query)) { return; } + FCTXTRACE("cancelquery"); + query->attributes |= RESQUERY_ATTR_CANCELED; /* @@ -1397,8 +1397,7 @@ fctx_cancelquery(resquery_t **queryp, isc_time_t *finish, bool no_response, * exist, cancel them. */ if (query->dispentry != NULL) { - dns_dispatch_cancel(query->dispentry); - dns_dispatch_removeresponse(&query->dispentry); + dns_dispatch_cancel(&query->dispentry); } if (ISC_LINK_LINKED(query, link)) { @@ -2126,7 +2125,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, UNLOCK(&res->buckets[fctx->bucketnum].lock); /* Set up the dispatch and set the query ID */ - result = dns_dispatch_addresponse( + result = dns_dispatch_add( query->dispatch, 0, isc_interval_ms(&fctx->interval), &query->addrinfo->sockaddr, resquery_connected, resquery_senddone, resquery_response, query, &query->id, @@ -2733,7 +2732,7 @@ cleanup_message: /* * Stop the dispatcher from listening. */ - dns_dispatch_removeresponse(&query->dispentry); + dns_dispatch_done(&query->dispentry); cleanup_temps: if (qname != NULL) { @@ -4390,7 +4389,7 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) { fctx_cleanup(fctx); LOCK(&res->buckets[bucketnum].lock); - fctx_decreference(fctx); + RUNTIME_CHECK(!fctx_decreference(fctx)); FCTX_ATTR_SET(fctx, FCTX_ATTR_SHUTTINGDOWN); diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c index 8b64c82ce0..93eac9f5ae 100644 --- a/lib/dns/tests/dispatch_test.c +++ b/lib/dns/tests/dispatch_test.c @@ -456,10 +456,10 @@ dispatch_timeout_tcp_connect(void **state) { region.base = rbuf; region.length = sizeof(rbuf); - result = dns_dispatch_addresponse(dispatch, 0, T_CLIENT_CONNECT, - &tcp_server_addr, timeout_connected, - client_senddone, response, ®ion, - &id, &dispentry); + result = dns_dispatch_add(dispatch, 0, T_CLIENT_CONNECT, + &tcp_server_addr, timeout_connected, + client_senddone, response, ®ion, &id, + &dispentry); assert_int_equal(result, ISC_R_SUCCESS); memset(message, 0, sizeof(message)); @@ -473,7 +473,7 @@ dispatch_timeout_tcp_connect(void **state) { uv_sem_wait(&sem); - dns_dispatch_removeresponse(&dispentry); + dns_dispatch_done(&dispentry); dns_dispatch_detach(&dispatch); dns_dispatchmgr_detach(&dispatchmgr); @@ -517,9 +517,9 @@ dispatch_timeout_tcp_response(void **state __attribute__((unused))) { region.base = rbuf; region.length = sizeof(rbuf); - result = dns_dispatch_addresponse( - dispatch, 0, T_CLIENT_CONNECT, &tcp_server_addr, connected, - client_senddone, response_timeout, ®ion, &id, &dispentry); + result = dns_dispatch_add(dispatch, 0, T_CLIENT_CONNECT, + &tcp_server_addr, connected, client_senddone, + response_timeout, ®ion, &id, &dispentry); assert_int_equal(result, ISC_R_SUCCESS); memset(message, 0, sizeof(message)); @@ -539,7 +539,7 @@ dispatch_timeout_tcp_response(void **state __attribute__((unused))) { isc_nmsocket_close(&sock); assert_null(sock); - dns_dispatch_removeresponse(&dispentry); + dns_dispatch_done(&dispentry); dns_dispatch_detach(&dispatch); dns_dispatchmgr_detach(&dispatchmgr); @@ -573,9 +573,9 @@ dispatch_tcp_response(void **state __attribute__((unused))) { region.base = rbuf; region.length = sizeof(rbuf); - result = dns_dispatch_addresponse( - dispatch, 0, T_CLIENT_CONNECT, &tcp_server_addr, connected, - client_senddone, response, ®ion, &id, &dispentry); + result = dns_dispatch_add(dispatch, 0, T_CLIENT_CONNECT, + &tcp_server_addr, connected, client_senddone, + response, ®ion, &id, &dispentry); assert_int_equal(result, ISC_R_SUCCESS); memset(message, 0, sizeof(message)); @@ -598,7 +598,7 @@ dispatch_tcp_response(void **state __attribute__((unused))) { isc_nmsocket_close(&sock); assert_null(sock); - dns_dispatch_removeresponse(&dispentry); + dns_dispatch_done(&dispentry); dns_dispatch_detach(&dispatch); dns_dispatchmgr_detach(&dispatchmgr); @@ -632,9 +632,9 @@ dispatch_timeout_udp_response(void **state __attribute__((unused))) { region.base = rbuf; region.length = sizeof(rbuf); - result = dns_dispatch_addresponse( - dispatch, 0, T_CLIENT_CONNECT, &udp_server_addr, connected, - client_senddone, response_timeout, ®ion, &id, &dispentry); + result = dns_dispatch_add(dispatch, 0, T_CLIENT_CONNECT, + &udp_server_addr, connected, client_senddone, + response_timeout, ®ion, &id, &dispentry); assert_int_equal(result, ISC_R_SUCCESS); memset(message, 0, sizeof(message)); @@ -654,7 +654,7 @@ dispatch_timeout_udp_response(void **state __attribute__((unused))) { isc_nmsocket_close(&sock); assert_null(sock); - dns_dispatch_removeresponse(&dispentry); + dns_dispatch_done(&dispentry); dns_dispatch_detach(&dispatch); dns_dispatchmgr_detach(&dispatchmgr); @@ -688,9 +688,9 @@ dispatch_getnext(void **state) { region.base = rbuf; region.length = sizeof(rbuf); - result = dns_dispatch_addresponse( - dispatch, 0, T_CLIENT_CONNECT, &udp_server_addr, connected, - client_senddone, response_getnext, ®ion, &id, &dispentry); + result = dns_dispatch_add(dispatch, 0, T_CLIENT_CONNECT, + &udp_server_addr, connected, client_senddone, + response_getnext, ®ion, &id, &dispentry); assert_int_equal(result, ISC_R_SUCCESS); memset(message, 0, sizeof(message)); @@ -711,7 +711,7 @@ dispatch_getnext(void **state) { isc_nmsocket_close(&sock); assert_null(sock); - dns_dispatch_removeresponse(&dispentry); + dns_dispatch_done(&dispentry); dns_dispatch_detach(&dispatch); dns_dispatchmgr_detach(&dispatchmgr); }