From: Ondřej Surý Date: Tue, 20 Dec 2022 07:39:36 +0000 (+0100) Subject: Call the connected dns_dispatch callback asynchronously X-Git-Tag: v9.18.11~28^2~1 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=be3cf85cfe8b22b6e0fe75ed139d0df021b950c3;p=thirdparty%2Fbind9.git Call the connected dns_dispatch callback asynchronously The dns_request code is very sensitive about calling the connected and deadlocks when the timing is "right" in several places. Move the call to the connected callback to the (udp|tcp)_connected() functions, so they are called asynchronously instead of directly from the (udp|tcp)_dispentry_cancel() functions. (cherry picked from commit 9dd8deaf018cf7f93dc1d2ba08cc35764e76cc0f) --- diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index af796eebd38..31b80c329ab 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -1574,7 +1574,6 @@ udp_dispentry_cancel(dns_dispentry_t *resp, isc_result_t result) { dns_dispatch_t *disp = resp->disp; dns_dispatchmgr_t *mgr = disp->mgr; dns_qid_t *qid = mgr->qid; - dispatch_cb_t connected = NULL; dispatch_cb_t response = NULL; LOCK(&disp->lock); @@ -1596,9 +1595,6 @@ udp_dispentry_cancel(dns_dispentry_t *resp, isc_result_t result) { break; case DNS_DISPATCHSTATE_CONNECTING: - dns_dispentry_ref(resp); /* DISPENTRY008 */ - ISC_LIST_UNLINK(disp->pending, resp, plink); - connected = resp->connected; break; case DNS_DISPATCHSTATE_CONNECTED: @@ -1629,12 +1625,6 @@ udp_dispentry_cancel(dns_dispentry_t *resp, isc_result_t result) { unlock: UNLOCK(&disp->lock); - if (connected) { - dispentry_log(resp, LVL(90), "connect callback: %s", - isc_result_totext(result)); - connected(result, NULL, resp->arg); - dns_dispentry_detach(&resp); /* DISPENTRY008 */ - } if (response) { dispentry_log(resp, LVL(90), "read callback: %s", isc_result_totext(result)); @@ -1652,7 +1642,6 @@ tcp_dispentry_cancel(dns_dispentry_t *resp, isc_result_t result) { dns_dispatch_t *disp = resp->disp; dns_dispatchmgr_t *mgr = disp->mgr; dns_qid_t *qid = mgr->qid; - dispatch_cb_t connected = NULL; dns_displist_t resps = ISC_LIST_INITIALIZER; LOCK(&disp->lock); @@ -1670,8 +1659,6 @@ tcp_dispentry_cancel(dns_dispentry_t *resp, isc_result_t result) { break; case DNS_DISPATCHSTATE_CONNECTING: - ISC_LIST_UNLINK(disp->pending, resp, plink); - connected = resp->connected; break; case DNS_DISPATCHSTATE_CONNECTED: @@ -1739,20 +1726,13 @@ unlock: UNLOCK(&disp->lock); /* - * NOTE: Calling the connected and response callbacks directly from here - * should be done asynchronously, as the dns_dispatch_done() is usually - * called directly from the response callback, so there's a slight - * chance that the call stack will get higher here, but it's mitigated - * by the ".reading" flag, so we don't ever go into a loop. + * NOTE: Calling the response callback directly from here should be done + * asynchronously, as the dns_dispatch_done() is usually called directly + * from the response callback, so there's a slight chance that the call + * stack will get higher here, but it's mitigated by the ".reading" + * flag, so we don't ever go into a loop. */ - if (connected) { - dispentry_log(resp, LVL(90), "connect callback: %s", - isc_result_totext(result)); - connected(result, NULL, resp->arg); - dns_dispentry_detach(&resp); /* DISPENTRY005 */ - } - tcp_recv_processall(&resps, NULL); } @@ -1853,16 +1833,6 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { LOCK(&disp->lock); INSIST(disp->state == DNS_DISPATCHSTATE_CONNECTING); - if (ISC_LIST_EMPTY(disp->pending)) { - /* All responses have been canceled */ - disp->state = DNS_DISPATCHSTATE_CANCELED; - } else if (eresult == ISC_R_SUCCESS) { - disp->state = DNS_DISPATCHSTATE_CONNECTED; - tcp_startrecv(handle, disp, resp); - } else { - disp->state = DNS_DISPATCHSTATE_NONE; - } - /* * If there are pending responses, call the connect * callbacks for all of them. @@ -1871,8 +1841,11 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { next = ISC_LIST_NEXT(resp, plink); ISC_LIST_UNLINK(disp->pending, resp, plink); ISC_LIST_APPEND(resps, resp, rlink); + resp->result = eresult; - if (eresult == ISC_R_SUCCESS) { + if (resp->state == DNS_DISPATCHSTATE_CANCELED) { + resp->result = ISC_R_CANCELED; + } else if (eresult == ISC_R_SUCCESS) { resp->state = DNS_DISPATCHSTATE_CONNECTED; ISC_LIST_APPEND(disp->active, resp, alink); resp->reading = true; @@ -1881,6 +1854,17 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { resp->state = DNS_DISPATCHSTATE_NONE; } } + + if (ISC_LIST_EMPTY(disp->active)) { + /* All responses have been canceled */ + disp->state = DNS_DISPATCHSTATE_CANCELED; + } else if (eresult == ISC_R_SUCCESS) { + disp->state = DNS_DISPATCHSTATE_CONNECTED; + tcp_startrecv(handle, disp, resp); + } else { + disp->state = DNS_DISPATCHSTATE_NONE; + } + UNLOCK(&disp->lock); for (resp = ISC_LIST_HEAD(resps); resp != NULL; resp = next) { @@ -1888,8 +1872,8 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { ISC_LIST_UNLINK(resps, resp, rlink); dispentry_log(resp, LVL(90), "connect callback: %s", - isc_result_totext(eresult)); - resp->connected(eresult, NULL, resp->arg); + isc_result_totext(resp->result)); + resp->connected(resp->result, NULL, resp->arg); dns_dispentry_detach(&resp); /* DISPENTRY005 */ } @@ -1911,8 +1895,9 @@ udp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { switch (resp->state) { case DNS_DISPATCHSTATE_CANCELED: - UNLOCK(&disp->lock); - goto detach; + eresult = ISC_R_CANCELED; + ISC_LIST_UNLINK(disp->pending, resp, plink); + goto unlock; case DNS_DISPATCHSTATE_CONNECTING: ISC_LIST_UNLINK(disp->pending, resp, plink); break; @@ -1921,6 +1906,8 @@ udp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { } switch (eresult) { + case ISC_R_CANCELED: + break; case ISC_R_SUCCESS: resp->state = DNS_DISPATCHSTATE_CONNECTED; udp_startrecv(handle, resp); @@ -1943,7 +1930,7 @@ udp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { resp->state = DNS_DISPATCHSTATE_NONE; break; } - +unlock: UNLOCK(&disp->lock); dispentry_log(resp, LVL(90), "connect callback: %s",