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 af796eebd381a3cff982a29fc47a4cda511693ab..31b80c329aba5fdc88312efc4a2424a9d559cdb3 100644 (file)
--- 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",