Make the netmgr read callback to be asynchronous only when needed

Previously, the read callback would be synchronous only on success or
timeout.  Add an option (similar to what other callbacks have) to decide
whether we need the asynchronous read callback on a higher level.

On a general level, we need the asynchronous callbacks to happen only
when we are invoking the callback from the public API.  If the path to
the callback went through the libuv callback or netmgr callback, we are
already on asynchronous path, and there's no need to make the call to
the callback asynchronous again.

For the read callback, this means we need the asynchronous path for
failure paths inside the isc_nm_read() (which calls isc__nm_udp_read(),
isc__nm_tcp_read(), etc...) - all other invocations of the read callback
could be synchronous, because those are called from the respective libuv
or netmgr read callbacks.

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index da6d28836d00712655fc0757b596001e0f4cc1fd..d17c18d81bae432e87f82af881481dbde9595042 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -1224,7 +1224,7 @@ isc__nm_async_connectcb(isc__networker_t *worker, isc__netievent_t *ev0);
 
 void
 isc__nm_readcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
-              isc_result_t eresult);
+              isc_result_t eresult, bool async);
 void
 isc__nm_async_readcb(isc__networker_t *worker, isc__netievent_t *ev0);
 
@@ -1581,7 +1581,8 @@ isc__nmhandle_tls_setwritetimeout(isc_nmhandle_t *handle,
                                  uint64_t write_timeout);
 
 void
-isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                          bool async);
 
 void
 isc__nm_http_stoplistening(isc_nmsocket_t *sock);
@@ -1894,11 +1895,14 @@ NETIEVENT_SOCKET_TLSCTX_DECL(settlsctx);
 NETIEVENT_SOCKET_DECL(sockstop);
 
 void
-isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                          bool async);
 void
-isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                          bool async);
 void
-isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                             bool async);
 void
 isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
                              bool async);

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 5efd4be5737ceb05872cbc6581a1afa7e8a1da77..7583e531f5afe46741ff5129b6be5af8a2047ea6 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -1392,20 +1392,20 @@ isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, bool async) {
        REQUIRE(VALID_NMSOCK(sock));
        switch (sock->type) {
        case isc_nm_udpsocket:
-               isc__nm_udp_failed_read_cb(sock, result);
+               isc__nm_udp_failed_read_cb(sock, result, async);
                return;
        case isc_nm_tcpsocket:
-               isc__nm_tcp_failed_read_cb(sock, result);
+               isc__nm_tcp_failed_read_cb(sock, result, async);
                return;
        case isc_nm_tcpdnssocket:
-               isc__nm_tcpdns_failed_read_cb(sock, result);
+               isc__nm_tcpdns_failed_read_cb(sock, result, async);
                return;
        case isc_nm_tlsdnssocket:
                isc__nm_tlsdns_failed_read_cb(sock, result, async);
                return;
 #ifdef HAVE_LIBNGHTTP2
        case isc_nm_tlssocket:
-               isc__nm_tls_failed_read_cb(sock, result);
+               isc__nm_tls_failed_read_cb(sock, result, async);
                return;
 #endif
        default:
@@ -1497,7 +1497,7 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer) {
 
                if (sock->recv_cb != NULL) {
                        isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
-                       isc__nm_readcb(sock, req, ISC_R_TIMEDOUT);
+                       isc__nm_readcb(sock, req, ISC_R_TIMEDOUT, false);
                }
 
                if (!isc__nmsocket_timer_running(sock)) {
@@ -2212,24 +2212,24 @@ isc__nm_async_connectcb(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 void
 isc__nm_readcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
-              isc_result_t eresult) {
+              isc_result_t eresult, bool async) {
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(VALID_UVREQ(uvreq));
        REQUIRE(VALID_NMHANDLE(uvreq->handle));
 
-       if (eresult == ISC_R_SUCCESS || eresult == ISC_R_TIMEDOUT) {
+       if (!async) {
                isc__netievent_readcb_t ievent = { .type = netievent_readcb,
                                                   .sock = sock,
                                                   .req = uvreq,
                                                   .result = eresult };
 
                isc__nm_async_readcb(NULL, (isc__netievent_t *)&ievent);
-       } else {
-               isc__netievent_readcb_t *ievent = isc__nm_get_netievent_readcb(
-                       sock->worker, sock, uvreq, eresult);
-               isc__nm_enqueue_ievent(sock->worker,
-                                      (isc__netievent_t *)ievent);
+               return;
        }
+
+       isc__netievent_readcb_t *ievent = isc__nm_get_netievent_readcb(
+               sock->worker, sock, uvreq, eresult);
+       isc__nm_enqueue_ievent(sock->worker, (isc__netievent_t *)ievent);
 }
 
 void

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 0283596e012d1230800ecd7a3cc7a8a80b2a89ff..9b2e9380035aa6fca14f49138f511ea1751573c1 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -698,7 +698,8 @@ isc__nm_async_tcpstop(isc__networker_t *worker, isc__netievent_t *ev0) {
 }
 
 void
-isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                          bool async) {
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(result != ISC_R_SUCCESS);
 
@@ -713,7 +714,7 @@ isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
        if (sock->recv_cb != NULL) {
                isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
                isc__nmsocket_clearcb(sock);
-               isc__nm_readcb(sock, req, result);
+               isc__nm_readcb(sock, req, result, async);
        }
 
 destroy:
@@ -769,18 +770,15 @@ isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
        return;
 failure:
        sock->reading = true;
-       isc__nm_tcp_failed_read_cb(sock, result);
+       isc__nm_tcp_failed_read_cb(sock, result, true);
 }
 
 void
 isc__nm_tcp_read_stop(isc_nmhandle_t *handle) {
-       isc_nmsocket_t *sock = NULL;
-
        REQUIRE(VALID_NMHANDLE(handle));
+       REQUIRE(VALID_NMSOCK(handle->sock));
 
-       sock = handle->sock;
-
-       REQUIRE(VALID_NMSOCK(sock));
+       isc_nmsocket_t *sock = handle->sock;
 
        isc__nmsocket_timer_stop(sock);
        isc__nm_stop_reading(sock);
@@ -802,7 +800,7 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
        netmgr = sock->worker->netmgr;
 
        if (isc__nmsocket_closing(sock)) {
-               isc__nm_tcp_failed_read_cb(sock, ISC_R_CANCELED);
+               isc__nm_tcp_failed_read_cb(sock, ISC_R_CANCELED, false);
                goto free;
        }
 
@@ -811,7 +809,8 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
                        isc__nm_incstats(sock, STATID_RECVFAIL);
                }
 
-               isc__nm_tcp_failed_read_cb(sock, isc_uverr2result(nread));
+               isc__nm_tcp_failed_read_cb(sock, isc_uverr2result(nread),
+                                          false);
 
                goto free;
        }
@@ -832,7 +831,7 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
                                              : atomic_load(&netmgr->idle));
        }
 
-       isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+       isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
 
        /* The readcb could have paused the reading */
        if (sock->reading) {

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 069b5e748de67055d15946fddf661014247cdb09..e1016bea497f443293c96dd5f62d3dafa31b7a3b 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -671,7 +671,8 @@ isc__nm_async_tcpdnsstop(isc__networker_t *worker, isc__netievent_t *ev0) {
 }
 
 void
-isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                             bool async) {
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(result != ISC_R_SUCCESS);
 
@@ -686,7 +687,7 @@ isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
        if (sock->recv_cb != NULL) {
                isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
                isc__nmsocket_clearcb(sock);
-               isc__nm_readcb(sock, req, result);
+               isc__nm_readcb(sock, req, result, async);
        }
 
 destroy:
@@ -848,7 +849,7 @@ isc__nm_tcpdns_processbuffer(isc_nmsocket_t *sock) {
         */
        REQUIRE(sock->processing == false);
        sock->processing = true;
-       isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+       isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
        sock->processing = false;
 
        len += 2;

diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index e5737c253d9c0d33462377ab62a7da9d5cede852..528e6d442bbdce7806f59a142bb5adcce34cbe73 100644 (file)
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -891,7 +891,7 @@ isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
        if (sock->recv_cb != NULL) {
                isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
                isc__nmsocket_clearcb(sock);
-               isc__nm_readcb(sock, req, result);
+               isc__nm_readcb(sock, req, result, async);
        }
 
 destroy:
@@ -1054,7 +1054,7 @@ isc__nm_tlsdns_processbuffer(isc_nmsocket_t *sock) {
         */
        REQUIRE(sock->processing == false);
        sock->processing = true;
-       isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+       isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
        sock->processing = false;
 
        len += 2;

diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c
index 35fa09fec2c8a531a759896738ecac68bdc14c63..e564d61fe47e32f3d399d83dbc4d6492cd4aa31c 100644 (file)
--- a/lib/isc/netmgr/tlsstream.c
+++ b/lib/isc/netmgr/tlsstream.c
@@ -218,7 +218,10 @@ tls_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result) {
 }
 
 void
-isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                          bool async) {
+       UNUSED(async);
+
        if (!inactive(sock) && sock->tlsstream.state == TLS_IO) {
                tls_do_bio(sock, NULL, NULL, true);
        } else if (sock->reading) {
@@ -871,10 +874,12 @@ isc__nm_tls_read_stop(isc_nmhandle_t *handle) {
        REQUIRE(VALID_NMHANDLE(handle));
        REQUIRE(VALID_NMSOCK(handle->sock));
 
-       handle->sock->reading = false;
+       isc_nmsocket_t *sock = handle->sock;
+
+       sock->reading = false;
 
-       if (handle->sock->outerhandle != NULL) {
-               isc_nm_read_stop(handle->sock->outerhandle);
+       if (sock->outerhandle != NULL) {
+               isc_nm_read_stop(sock->outerhandle);
        }
 }
 

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 3900372173eaf120498f9a3caa2c4634aa53a705..0c5d4cf07d4363601dded851a01ece89a20b9aa4 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -617,7 +617,7 @@ isc__nm_udp_read_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 
        REQUIRE(!sock->processing);
        sock->processing = true;
-       isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+       isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
        sock->processing = false;
 
 free:
@@ -879,7 +879,8 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 }
 
 void
-isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+                          bool async) {
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(result != ISC_R_SUCCESS);
        REQUIRE(sock->tid == isc_tid());
@@ -892,14 +893,15 @@ isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
                if (!sock->recv_read) {
                        goto destroy;
                }
-               sock->recv_read = false;
 
                if (sock->recv_cb != NULL) {
                        isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
                        isc__nmsocket_clearcb(sock);
-                       isc__nm_readcb(sock, req, result);
+                       isc__nm_readcb(sock, req, result, async);
                }
 
+               sock->recv_read = false;
+
        destroy:
                isc__nmsocket_prep_destroy(sock);
                return;
@@ -919,7 +921,7 @@ isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 
        if (sock->recv_cb != NULL) {
                isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
-               isc__nm_readcb(sock, req, result);
+               isc__nm_readcb(sock, req, result, async);
        }
 }
 
@@ -966,7 +968,7 @@ isc__nm_udp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 
 fail:
        sock->reading = true; /* required by the next call */
-       isc__nm_failed_read_cb(sock, result, false);
+       isc__nm_failed_read_cb(sock, result, true);
 }
 
 static void