Refactor the isc__nm_uvreq_t to have idle callback

Change the isc__nm_uvreq_t to have the idle callback as a separate
member as we always need to use it to properly close the uvreq.

Slightly refactor uvreq_put and uvreq_get to remove the unneeded
arguments - in uvreq_get(), we always use sock->worker, and in
uvreq_put, we always use req->sock, so there's not reason to pass those
extra arguments.

diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c
index f4d29b1d3e2705b70495745c52c766e1e0e9677c..5aed9dfa36739a0d4656b4db39fd0b3746217ebf 100644 (file)
--- a/lib/isc/netmgr/http.c
+++ b/lib/isc/netmgr/http.c
@@ -1156,8 +1156,7 @@ http_send_outgoing(isc_nm_http_session_t *session, isc_nmhandle_t *httphandle,
 
                        INSIST(VALID_NMHANDLE(httphandle));
 
-                       newcb = isc__nm_uvreq_get(httphandle->sock->worker,
-                                                 httphandle->sock);
+                       newcb = isc__nm_uvreq_get(httphandle->sock);
                        newcb->cb.send = cb;
                        newcb->cbarg = cbarg;
                        isc_nmhandle_attach(httphandle, &newcb->handle);
@@ -1483,7 +1482,7 @@ isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
        atomic_init(&sock->client, true);
 
        if (isc__nm_closing(worker)) {
-               isc__nm_uvreq_t *req = isc__nm_uvreq_get(worker, sock);
+               isc__nm_uvreq_t *req = isc__nm_uvreq_get(sock);
 
                req->cb.connect = cb;
                req->cbarg = cbarg;
@@ -2178,7 +2177,7 @@ isc__nm_http_send(isc_nmhandle_t *handle, const isc_region_t *region,
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_tid());
 
-       uvreq = isc__nm_uvreq_get(sock->worker, sock);
+       uvreq = isc__nm_uvreq_get(sock);
        isc_nmhandle_attach(handle, &uvreq->handle);
        uvreq->cb.send = cb;
        uvreq->cbarg = cbarg;
@@ -2198,7 +2197,7 @@ failed_send_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
        if (req->cb.send != NULL) {
                isc__nm_sendcb(sock, req, eresult, true);
        } else {
-               isc__nm_uvreq_put(&req, sock);
+               isc__nm_uvreq_put(&req);
        }
 }
 
@@ -2218,7 +2217,7 @@ client_httpsend(isc_nmhandle_t *handle, isc_nmsocket_t *sock,
        }
 
        http_do_bio(sock->h2.session, handle, cb, cbarg);
-       isc__nm_uvreq_put(&req, sock);
+       isc__nm_uvreq_put(&req);
 }
 
 static void
@@ -2273,7 +2272,7 @@ server_httpsend(isc_nmhandle_t *handle, isc_nmsocket_t *sock,
        } else {
                cb(handle, result, cbarg);
        }
-       isc__nm_uvreq_put(&req, sock);
+       isc__nm_uvreq_put(&req);
 }
 
 static void

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 30c74687b3b74bb94883c67e0ff7ea740184ba83..023fbec40bace2fedf4c782abf88114b94246cb6 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -142,10 +142,10 @@ STATIC_ASSERT(ISC_NETMGR_TCP_RECVBUF_SIZE <= ISC_NETMGR_RECVBUF_SIZE,
        ievent->file = file;      \
        ievent->line = line;      \
        ievent->func = func;
-#define isc__nm_uvreq_get(req, sock) \
-       isc___nm_uvreq_get(req, sock, __FILE__, __LINE__, __func__)
-#define isc__nm_uvreq_put(req, sock) \
-       isc___nm_uvreq_put(req, sock, __FILE__, __LINE__, __func__)
+#define isc__nm_uvreq_get(sock) \
+       isc___nm_uvreq_get(sock, __FILE__, __LINE__, __func__)
+#define isc__nm_uvreq_put(req) \
+       isc___nm_uvreq_put(req, __FILE__, __LINE__, __func__)
 #define isc__nmsocket_init(sock, mgr, type, iface, parent)            \
        isc___nmsocket_init(sock, mgr, type, iface, parent, __FILE__, \
                            __LINE__, __func__)
@@ -168,8 +168,8 @@ STATIC_ASSERT(ISC_NETMGR_TCP_RECVBUF_SIZE <= ISC_NETMGR_RECVBUF_SIZE,
 #define FLARG_PASS
 #define FLARG_IEVENT(ievent)
 #define FLARG_IEVENT_PASS(ievent)
-#define isc__nm_uvreq_get(req, sock) isc___nm_uvreq_get(req, sock)
-#define isc__nm_uvreq_put(req, sock) isc___nm_uvreq_put(req, sock)
+#define isc__nm_uvreq_get(sock) isc___nm_uvreq_get(sock)
+#define isc__nm_uvreq_put(req) isc___nm_uvreq_put(req)
 #define isc__nmsocket_init(sock, mgr, type, iface, parent) \
        isc___nmsocket_init(sock, mgr, type, iface, parent)
 #define isc__nmsocket_put(sockp)          isc___nmsocket_put(sockp)
@@ -278,6 +278,7 @@ struct isc__nm_uvreq {
        isc_nm_timer_t *timer; /* TCP write timer */
        int connect_tries;     /* connect retries */
        isc_result_t result;
+       uv_idle_t idle;
 
        union {
                uv_handle_t handle;
@@ -289,10 +290,9 @@ struct isc__nm_uvreq {
                uv_connect_t connect;
                uv_udp_send_t udp_send;
                uv_fs_t fs;
-               uv_idle_t idle;
        } uv_req;
        ISC_LINK(isc__nm_uvreq_t) link;
-       ISC_LINK(isc__nm_uvreq_t) inactive_link;
+       ISC_LINK(isc__nm_uvreq_t) active_link;
 };
 
 /*
@@ -628,11 +628,16 @@ struct isc_nmsocket {
        atomic_bool keepalive;
 
        /*%
-        * 'spare' handles for that can be reused to avoid allocations,
-        * for UDP.
+        * 'spare' handles for that can be reused to avoid allocations, for UDP.
         */
        ISC_LIST(isc_nmhandle_t) inactive_handles;
 
+       /*%
+        * 'active' handles and uvreqs, mostly for debugging purposes.
+        */
+       ISC_LIST(isc_nmhandle_t) active_handles;
+       ISC_LIST(isc__nm_uvreq_t) active_uvreqs;
+
        /*%
         * Used to pass a result back from listen or connect events.
         */
@@ -665,7 +670,6 @@ struct isc_nmsocket {
        int backtrace_size;
 #endif
        LINK(isc_nmsocket_t) active_link;
-       ISC_LIST(isc_nmhandle_t) active_handles;
 };
 
 void
@@ -696,14 +700,14 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t const *peer,
  */
 
 isc__nm_uvreq_t *
-isc___nm_uvreq_get(isc__networker_t *worker, isc_nmsocket_t *sock FLARG);
+isc___nm_uvreq_get(isc_nmsocket_t *sock FLARG);
 /*%<
  * Get a UV request structure for the socket 'sock', allocating a
  * new one if there isn't one available in 'sock->inactivereqs'.
  */
 
 void
-isc___nm_uvreq_put(isc__nm_uvreq_t **req, isc_nmsocket_t *sock FLARG);
+isc___nm_uvreq_put(isc__nm_uvreq_t **req FLARG);
 /*%<
  * Completes the use of a UV request structure, setting '*req' to NULL.
  *

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index b34e2348c0a2a5483ec1ffb943ad200c687ebac5..aede4e92a8a50015a1da62780a352f58de9efc4b 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -48,13 +48,6 @@
 #include "openssl_shim.h"
 #include "trampoline_p.h"
 
-/*%
- * How many isc_nmhandles and isc_nm_uvreqs will we be
- * caching for reuse in a socket.
- */
-#define ISC_NM_HANDLES_STACK_SIZE 600
-#define ISC_NM_REQS_STACK_SIZE   600
-
 /*%
  * Shortcut index arrays to get access to statistics counters.
  */
@@ -127,6 +120,9 @@ nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG);
 static void
 nmhandle_free(isc_nmsocket_t *sock, isc_nmhandle_t *handle);
 
+static void
+uvreq_free(uv_handle_t *handle);
+
 /*%<
  * Issue a 'handle closed' callback on the socket.
  */
@@ -249,8 +245,6 @@ isc_netmgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, isc_nm_t **netmgrp) {
 
                isc_mempool_create(worker->mctx, sizeof(isc__nm_uvreq_t),
                                   &worker->uvreq_pool);
-               isc_mempool_setfreemax(worker->uvreq_pool,
-                                      ISC_NM_REQS_STACK_SIZE);
 
                isc_loop_attach(loop, &worker->loop);
                isc_loop_teardown(loop, networker_teardown, worker);
@@ -872,7 +866,7 @@ dequeue_handle(isc_nmsocket_t *sock) {
                return (handle);
        }
 #else
-       UNUSED(sock);
+       INSIST(ISC_LIST_EMPTY(sock->inactive_handles));
 #endif /* !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ */
        return (NULL);
 }
@@ -1081,7 +1075,7 @@ isc__nm_failed_send_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
        if (req->cb.send != NULL) {
                isc__nm_sendcb(sock, req, eresult, async);
        } else {
-               isc__nm_uvreq_put(&req, sock);
+               isc__nm_uvreq_put(&req);
        }
 }
 
@@ -1348,7 +1342,7 @@ isc__nm_uvreq_t *
 isc__nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr) {
        isc__nm_uvreq_t *req = NULL;
 
-       req = isc__nm_uvreq_get(sock->worker, sock);
+       req = isc__nm_uvreq_get(sock);
        req->cb.recv = sock->recv_cb;
        req->cbarg = sock->recv_cbarg;
 
@@ -1595,57 +1589,62 @@ isc_nmhandle_netmgr(isc_nmhandle_t *handle) {
 }
 
 isc__nm_uvreq_t *
-isc___nm_uvreq_get(isc__networker_t *worker, isc_nmsocket_t *sock FLARG) {
-       isc__nm_uvreq_t *req = NULL;
-
-       REQUIRE(worker != NULL);
+isc___nm_uvreq_get(isc_nmsocket_t *sock FLARG) {
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_tid());
 
-       req = isc_mempool_get(worker->uvreq_pool);
+       isc__networker_t *worker = sock->worker;
+
+       isc__nm_uvreq_t *req = isc_mempool_get(worker->uvreq_pool);
        *req = (isc__nm_uvreq_t){
                .connect_tries = 3,
                .link = ISC_LINK_INITIALIZER,
-               .inactive_link = ISC_LINK_INITIALIZER,
-               .uv_req.req.data = req,
+               .active_link = ISC_LINK_INITIALIZER,
                .magic = UVREQ_MAGIC,
        };
+       uv_handle_set_data(&req->uv_req.handle, req);
+
+       int r = uv_idle_init(&worker->loop->loop, &req->idle);
+       UV_RUNTIME_CHECK(uv_idle_init, r);
+       uv_handle_set_data(&req->idle, req);
+
        isc___nmsocket_attach(sock, &req->sock FLARG_PASS);
 
+       ISC_LIST_APPEND(sock->active_uvreqs, req, active_link);
+
        return (req);
 }
 
-void
-isc___nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock FLARG) {
-       REQUIRE(req0 != NULL);
-       REQUIRE(VALID_UVREQ(*req0));
-       REQUIRE(VALID_NMSOCK(sock));
-       REQUIRE(sock->tid == isc_tid());
+static void
+uvreq_free(uv_handle_t *handle) {
+       isc__nm_uvreq_t *req = uv_handle_get_data(handle);
+       isc_nmsocket_t *sock = req->sock;
 
-       isc__nm_uvreq_t *req = NULL;
-       isc_nmhandle_t *handle = NULL;
-       isc__networker_t *worker = sock->worker;
+       isc_mempool_put(sock->worker->uvreq_pool, req);
 
-       req = *req0;
-       *req0 = NULL;
+       isc___nmsocket_detach(&sock FLARG_PASS);
+}
 
-       INSIST(sock == req->sock);
+void
+isc___nm_uvreq_put(isc__nm_uvreq_t **reqp FLARG) {
+       REQUIRE(reqp != NULL && VALID_UVREQ(*reqp));
 
-       req->magic = 0;
+       isc__nm_uvreq_t *req = *reqp;
+       isc_nmhandle_t *handle = req->handle;
+       isc_nmsocket_t *sock = req->sock;
 
-       /*
-        * We need to save this first to make sure that handle,
-        * sock, and the netmgr won't all disappear.
-        */
-       ISC_SWAP(handle, req->handle);
+       *reqp = NULL;
+       req->handle = NULL;
 
-       isc_mempool_put(worker->uvreq_pool, req);
+       REQUIRE(VALID_UVREQ(req));
+
+       ISC_LIST_UNLINK(sock->active_uvreqs, req, active_link);
 
        if (handle != NULL) {
                isc__nmhandle_detach(&handle FLARG_PASS);
        }
 
-       isc___nmsocket_detach(&sock FLARG_PASS);
+       uv_close(&req->idle, uvreq_free);
 }
 
 void
@@ -1838,13 +1837,6 @@ isc__nmsocket_barrier_init(isc_nmsocket_t *listener) {
        listener->barriers_initialised = true;
 }
 
-static void
-isc__nm_uvreq_free(uv_handle_t *handle) {
-       isc__nm_uvreq_t *uvreq = uv_handle_get_data(handle);
-
-       isc__nm_uvreq_put(&uvreq, uvreq->handle->sock);
-}
-
 static void
 isc___nm_connectcb(uv_idle_t *handle) {
        isc__nm_uvreq_t *uvreq = uv_handle_get_data(handle);
@@ -1852,7 +1844,7 @@ isc___nm_connectcb(uv_idle_t *handle) {
        uvreq->cb.connect(uvreq->handle, uvreq->result, uvreq->cbarg);
 
        uv_idle_stop(handle);
-       uv_close(handle, isc__nm_uvreq_free);
+       isc__nm_uvreq_put(&uvreq);
 }
 
 void
@@ -1865,15 +1857,13 @@ isc__nm_connectcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
 
        if (!async) {
                uvreq->cb.connect(uvreq->handle, eresult, uvreq->cbarg);
-               isc__nm_uvreq_put(&uvreq, uvreq->handle->sock);
+               isc__nm_uvreq_put(&uvreq);
                return;
        }
 
        uvreq->result = eresult;
 
-       uv_idle_init(&sock->worker->loop->loop, &uvreq->uv_req.idle);
-       uv_idle_start(&uvreq->uv_req.idle, isc___nm_connectcb);
-       uv_handle_set_data(&uvreq->uv_req.idle, uvreq);
+       uv_idle_start(&uvreq->idle, isc___nm_connectcb);
 }
 
 static void
@@ -1887,7 +1877,7 @@ isc___nm_readcb(uv_idle_t *handle) {
        uvreq->cb.recv(uvreq->handle, uvreq->result, &region, uvreq->cbarg);
 
        uv_idle_stop(handle);
-       uv_close(handle, isc__nm_uvreq_free);
+       isc__nm_uvreq_put(&uvreq);
 }
 
 void
@@ -1904,15 +1894,13 @@ isc__nm_readcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
                region.length = uvreq->uvbuf.len;
 
                uvreq->cb.recv(uvreq->handle, eresult, &region, uvreq->cbarg);
-               isc__nm_uvreq_put(&uvreq, uvreq->handle->sock);
+               isc__nm_uvreq_put(&uvreq);
                return;
        }
 
        uvreq->result = eresult;
 
-       uv_idle_init(&sock->worker->loop->loop, &uvreq->uv_req.idle);
-       uv_idle_start(&uvreq->uv_req.idle, isc___nm_readcb);
-       uv_handle_set_data(&uvreq->uv_req.idle, uvreq);
+       uv_idle_start(&uvreq->idle, isc___nm_readcb);
 }
 
 static void
@@ -1922,7 +1910,7 @@ isc___nm_sendcb(uv_idle_t *handle) {
        uvreq->cb.send(uvreq->handle, uvreq->result, uvreq->cbarg);
 
        uv_idle_stop(handle);
-       uv_close(handle, isc__nm_uvreq_free);
+       isc__nm_uvreq_put(&uvreq);
 }
 
 void
@@ -1935,13 +1923,11 @@ isc__nm_sendcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
 
        if (!async) {
                uvreq->cb.send(uvreq->handle, uvreq->result, uvreq->cbarg);
-               isc__nm_uvreq_put(&uvreq, uvreq->handle->sock);
+               isc__nm_uvreq_put(&uvreq);
                return;
        }
 
-       uv_idle_init(&sock->worker->loop->loop, &uvreq->uv_req.idle);
-       uv_idle_start(&uvreq->uv_req.idle, isc___nm_sendcb);
-       uv_handle_set_data(&uvreq->uv_req.idle, uvreq);
+       uv_idle_start(&uvreq->idle, isc___nm_sendcb);
 }
 
 static void

diff --git a/lib/isc/netmgr/streamdns.c b/lib/isc/netmgr/streamdns.c
index a34e46d3e6ce8d67293f55e291dcf1bfb8d9d556..5a3a16ab748796ddd5107eae0d6ca0b3bd5e4d88 100644 (file)
--- a/lib/isc/netmgr/streamdns.c
+++ b/lib/isc/netmgr/streamdns.c
@@ -893,7 +893,7 @@ isc__nm_streamdns_send(isc_nmhandle_t *handle, const isc_region_t *region,
        REQUIRE(sock->type == isc_nm_streamdnssocket);
        REQUIRE(sock->tid == isc_tid());
 
-       uvreq = isc__nm_uvreq_get(sock->worker, sock);
+       uvreq = isc__nm_uvreq_get(sock);
        isc_nmhandle_attach(handle, &uvreq->handle);
        uvreq->cb.send = cb;
        uvreq->cbarg = cbarg;
@@ -917,7 +917,7 @@ isc__nm_streamdns_send(isc_nmhandle_t *handle, const isc_region_t *region,
        isc__nm_senddns(sock->outerhandle, &data, streamdns_writecb,
                        (void *)send_req);
 
-       isc__nm_uvreq_put(&uvreq, sock);
+       isc__nm_uvreq_put(&uvreq);
 }
 
 static void

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 5de84444ad5284b1e7c3b09c5439a64ef1041728..0be807f2b7adf435b167561524c54f57594ff029 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -197,7 +197,7 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
                 * The connect was cancelled from timeout; just clean up
                 * the req.
                 */
-               isc__nm_uvreq_put(&req, sock);
+               isc__nm_uvreq_put(&req);
                return;
        } else if (isc__nm_closing(worker)) {
                /* Network manager shutting down */
@@ -291,7 +291,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
        sock->fd = fd;
        atomic_init(&sock->client, true);
 
-       req = isc__nm_uvreq_get(worker, sock);
+       req = isc__nm_uvreq_get(sock);
        req->cb.connect = cb;
        req->cbarg = cbarg;
        req->peer = *peer;
@@ -1009,7 +1009,7 @@ tcp_send(isc_nmhandle_t *handle, const isc_region_t *region, isc_nm_cb_t cb,
        REQUIRE(sock->type == isc_nm_tcpsocket);
        REQUIRE(sock->tid == isc_tid());
 
-       uvreq = isc__nm_uvreq_get(sock->worker, sock);
+       uvreq = isc__nm_uvreq_get(sock);
        if (dnsmsg) {
                *(uint16_t *)uvreq->tcplen = htons(region->length);
        }

diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c
index 6d6cc12df01bac522f5433d2e95e9dd1c53133ee..c1293ecaac8a9854887dfca6ca0ed9f41c8eabcf 100644 (file)
--- a/lib/isc/netmgr/tlsstream.c
+++ b/lib/isc/netmgr/tlsstream.c
@@ -981,7 +981,7 @@ tls_send_direct(void *arg) {
 
        tls_do_bio(sock, NULL, req, false);
 done:
-       isc__nm_uvreq_put(&req, sock);
+       isc__nm_uvreq_put(&req);
        return;
 }
 
@@ -998,7 +998,7 @@ tls_send(isc_nmhandle_t *handle, const isc_region_t *region, isc_nm_cb_t cb,
 
        REQUIRE(sock->type == isc_nm_tlssocket);
 
-       uvreq = isc__nm_uvreq_get(sock->worker, sock);
+       uvreq = isc__nm_uvreq_get(sock);
        isc_nmhandle_attach(handle, &uvreq->handle);
        uvreq->cb.send = cb;
        uvreq->cbarg = cbarg;

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 0ab10c525302704dc38655319bebd1e6bcce6d9a..f7fa9a6317f9add4645b4bb84194f4f695a7651a 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -388,7 +388,7 @@ isc_nm_routeconnect(isc_nm_t *mgr, isc_nm_cb_t cb, void *cbarg) {
        sock->route_sock = true;
        sock->fd = fd;
 
-       req = isc__nm_uvreq_get(worker, sock);
+       req = isc__nm_uvreq_get(sock);
        req->cb.connect = cb;
        req->cbarg = cbarg;
        req->handle = isc__nmhandle_get(sock, NULL, NULL);
@@ -681,7 +681,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, const isc_region_t *region,
                return;
        }
 
-       uvreq = isc__nm_uvreq_get(sock->worker, sock);
+       uvreq = isc__nm_uvreq_get(sock);
        uvreq->uvbuf.base = (char *)region->base;
        uvreq->uvbuf.len = region->length;
 
@@ -827,7 +827,7 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
        (void)isc__nm_socket_min_mtu(sock->fd, sa_family);
 
        /* Initialize the request */
-       req = isc__nm_uvreq_get(worker, sock);
+       req = isc__nm_uvreq_get(sock);
        req->cb.connect = cb;
        req->cbarg = cbarg;
        req->peer = *peer;