ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Call the connected dns_dispatch callback asynchronously

Browse Source 
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.
This commit is contained in:
Ondřej Surý
2022-12-20 08:39:36 +01:00
parent 3fac4ca57e
commit 9dd8deaf01
1 changed files with 36 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
lib/dns/dispatch.c
View File

@@ -1609,7 +1609,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);
@@ -1631,9 +1630,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:
@@ -1664,12 +1660,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));
@@ -1687,7 +1677,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);
@@ -1705,8 +1694,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:
@@ -1774,20 +1761,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);
}
@@ -1888,7 +1868,29 @@ 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)) {
/*
* If there are pending responses, call the connect
* callbacks for all of them.
*/
for (resp = ISC_LIST_HEAD(disp->pending); resp != NULL; resp = next) {
next = ISC_LIST_NEXT(resp, plink);
ISC_LIST_UNLINK(disp->pending, resp, plink);
ISC_LIST_APPEND(resps, resp, rlink);
resp->result = eresult;
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;
dispentry_log(resp, LVL(90), "start reading");
} else {
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) {
@@ -1898,24 +1900,6 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
disp->state = DNS_DISPATCHSTATE_NONE;
}
/*
* If there are pending responses, call the connect
* callbacks for all of them.
*/
for (resp = ISC_LIST_HEAD(disp->pending); resp != NULL; resp = next) {
next = ISC_LIST_NEXT(resp, plink);
ISC_LIST_UNLINK(disp->pending, resp, plink);
ISC_LIST_APPEND(resps, resp, rlink);
if (eresult == ISC_R_SUCCESS) {
resp->state = DNS_DISPATCHSTATE_CONNECTED;
ISC_LIST_APPEND(disp->active, resp, alink);
resp->reading = true;
dispentry_log(resp, LVL(90), "start reading");
} else {
resp->state = DNS_DISPATCHSTATE_NONE;
}
}
UNLOCK(&disp->lock);
for (resp = ISC_LIST_HEAD(resps); resp != NULL; resp = next) {
@@ -1923,8 +1907,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 */
}
@@ -1946,8 +1930,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;
@@ -1956,6 +1941,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);
@@ -1978,7 +1965,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",