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.

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

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.
  *

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);
 }
 
+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_mempool_put(sock->worker->uvreq_pool, req);
+
+	isc___nmsocket_detach(&sock FLARG_PASS);
+}
+
 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());
+isc___nm_uvreq_put(isc__nm_uvreq_t **reqp FLARG) {
+	REQUIRE(reqp != NULL && VALID_UVREQ(*reqp));
 
-	isc__nm_uvreq_t *req = NULL;
-	isc_nmhandle_t *handle = NULL;
-	isc__networker_t *worker = sock->worker;
+	isc__nm_uvreq_t *req = *reqp;
+	isc_nmhandle_t *handle = req->handle;
+	isc_nmsocket_t *sock = req->sock;
 
-	req = *req0;
-	*req0 = NULL;
+	*reqp = NULL;
+	req->handle = NULL;
 
-	INSIST(sock == req->sock);
+	REQUIRE(VALID_UVREQ(req));
 
-	req->magic = 0;
-
-	/*
-	 * We need to save this first to make sure that handle,
-	 * sock, and the netmgr won't all disappear.
-	 */
-	ISC_SWAP(handle, req->handle);
-
-	isc_mempool_put(worker->uvreq_pool, 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

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

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);
 	}

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;

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;