EXP: Always keep at least 600 handles

The isc_astack API is now completely unused, just remove it.

lib/isc/Makefile.am

@@ -8,7 +8,6 @@ libisc_la_HEADERS =			\
 	include/isc/align.h		\
 	include/isc/app.h		\
 	include/isc/assertions.h	\
-	include/isc/astack.h		\
 	include/isc/atomic.h		\
 	include/isc/attributes.h	\
 	include/isc/backtrace.h		\
@@ -118,7 +117,6 @@ libisc_la_SOURCES =		\
 	aes.c			\
 	app.c			\
 	assertions.c		\
-	astack.c		\
 	backtrace.c		\
 	base32.c		\
 	base64.c		\

lib/isc/astack.c

@@ -1,85 +0,0 @@
-/*
- * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
- *
- * SPDX-License-Identifier: MPL-2.0
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, you can obtain one at https://mozilla.org/MPL/2.0/.
- *
- * See the COPYRIGHT file distributed with this work for additional
- * information regarding copyright ownership.
- */
-
-#include <inttypes.h>
-#include <string.h>
-
-#include <isc/astack.h>
-#include <isc/atomic.h>
-#include <isc/mem.h>
-#include <isc/mutex.h>
-#include <isc/types.h>
-#include <isc/util.h>
-
-struct isc_astack {
-	isc_mem_t *mctx;
-	size_t size;
-	size_t pos;
-	isc_mutex_t lock;
-	uintptr_t nodes[];
-};
-
-isc_astack_t *
-isc_astack_new(isc_mem_t *mctx, size_t size) {
-	isc_astack_t *stack = isc_mem_get(
-		mctx, sizeof(isc_astack_t) + size * sizeof(uintptr_t));
-
-	*stack = (isc_astack_t){
-		.size = size,
-	};
-	isc_mem_attach(mctx, &stack->mctx);
-	memset(stack->nodes, 0, size * sizeof(uintptr_t));
-	isc_mutex_init(&stack->lock);
-	return (stack);
-}
-
-bool
-isc_astack_trypush(isc_astack_t *stack, void *obj) {
-	if (!isc_mutex_trylock(&stack->lock)) {
-		if (stack->pos >= stack->size) {
-			UNLOCK(&stack->lock);
-			return (false);
-		}
-		stack->nodes[stack->pos++] = (uintptr_t)obj;
-		UNLOCK(&stack->lock);
-		return (true);
-	} else {
-		return (false);
-	}
-}
-
-void *
-isc_astack_pop(isc_astack_t *stack) {
-	LOCK(&stack->lock);
-	uintptr_t rv;
-	if (stack->pos == 0) {
-		rv = 0;
-	} else {
-		rv = stack->nodes[--stack->pos];
-	}
-	UNLOCK(&stack->lock);
-	return ((void *)rv);
-}
-
-void
-isc_astack_destroy(isc_astack_t *stack) {
-	LOCK(&stack->lock);
-	REQUIRE(stack->pos == 0);
-	UNLOCK(&stack->lock);
-
-	isc_mutex_destroy(&stack->lock);
-
-	isc_mem_putanddetach(&stack->mctx, stack,
-			     sizeof(struct isc_astack) +
-				     stack->size * sizeof(uintptr_t));
-}

lib/isc/include/isc/astack.h

@@ -1,49 +0,0 @@
-/*
- * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
- *
- * SPDX-License-Identifier: MPL-2.0
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, you can obtain one at https://mozilla.org/MPL/2.0/.
- *
- * See the COPYRIGHT file distributed with this work for additional
- * information regarding copyright ownership.
- */
-
-#pragma once
-
-#include <inttypes.h>
-
-#include <isc/mem.h>
-#include <isc/types.h>
-
-isc_astack_t *
-isc_astack_new(isc_mem_t *mctx, size_t size);
-/*%<
- * Allocate and initialize a new array stack of size 'size'.
- */
-
-void
-isc_astack_destroy(isc_astack_t *stack);
-/*%<
- * Free an array stack 'stack'.
- *
- * Requires:
- * \li	'stack' is empty.
- */
-
-bool
-isc_astack_trypush(isc_astack_t *stack, void *obj);
-/*%<
- * Try to push 'obj' onto array stack 'astack'. On failure, either
- * because the stack size limit has been reached or because another
- * thread has already changed the stack pointer, return 'false'.
- */
-
-void *
-isc_astack_pop(isc_astack_t *stack);
-/*%<
- * Pop an object off of array stack 'stack'. If the stack is empty,
- * return NULL.
- */

lib/isc/include/isc/netmgr.h

@@ -62,6 +62,8 @@ typedef void (*isc_nm_cb_t)(isc_nmhandle_t *handle, isc_result_t result,
  * 'eresult' the result of the event.
  * 'cbarg'  the callback argument passed to isc_nm_send(),
  *          isc_nm_tcp_connect(), or isc_nm_listentcp()
+ *
+ * NOTE: The handle is valid only when eresult == ISC_R_SUCCESS
  */
 
 typedef void (*isc_nm_opaquecb_t)(void *arg);

lib/isc/include/isc/types.h

@@ -33,7 +33,6 @@
 
 /* Core Types.  Alphabetized by defined type. */
 
-typedef struct isc_astack isc_astack_t;		 /*%< Array-based fast stack */
 typedef struct isc_appctx isc_appctx_t;		 /*%< Application context */
 typedef struct isc_buffer isc_buffer_t;		 /*%< Buffer */
 typedef ISC_LIST(isc_buffer_t) isc_bufferlist_t; /*%< Buffer List */
@@ -127,12 +126,12 @@ typedef enum {
 
 typedef enum isc_nmsocket_type {
 	isc_nm_nonesocket = 0,
-	isc_nm_udpsocket = 1 << 1,
-	isc_nm_tcpsocket = 1 << 2,
-	isc_nm_tcpdnssocket = 1 << 3,
-	isc_nm_tlssocket = 1 << 4,
-	isc_nm_tlsdnssocket = 1 << 5,
-	isc_nm_httpsocket = 1 << 6,
+	isc_nm_udpsocket = 1,
+	isc_nm_tcpsocket = 2,
+	isc_nm_tcpdnssocket = 3,
+	isc_nm_tlssocket = 4,
+	isc_nm_tlsdnssocket = 5,
+	isc_nm_httpsocket = 6,
 	isc_nm_maxsocket,
 
 	isc_nm_udplistener, /* Aggregate of nm_udpsocks */
@@ -140,7 +139,8 @@ typedef enum isc_nmsocket_type {
 	isc_nm_tlslistener,
 	isc_nm_tcpdnslistener,
 	isc_nm_tlsdnslistener,
-	isc_nm_httplistener
+	isc_nm_httplistener,
+	isc_nm_maxlistener,
 } isc_nmsocket_type;
 
 typedef isc_nmsocket_type isc_nmsocket_type_t;

lib/isc/netmgr/http.c

@@ -1455,7 +1455,6 @@ isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 		req->cbarg = cbarg;
 		req->peer = *peer;
 		req->local = *local;
-		req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 		if (isc__nm_in_netthread()) {
 			sock->tid = isc_nm_tid();

lib/isc/netmgr/netmgr-int.h

@@ -19,7 +19,6 @@
 #include <openssl/err.h>
 #include <openssl/ssl.h>
 
-#include <isc/astack.h>
 #include <isc/atomic.h>
 #include <isc/barrier.h>
 #include <isc/buffer.h>
@@ -201,6 +200,8 @@ typedef enum {
 	NETIEVENT_MAX = 4,
 } netievent_type_t;
 
+typedef struct isc__nm_uvreq isc__nm_uvreq_t;
+
 /*
  * Single network event loop worker.
  */
@@ -242,6 +243,11 @@ struct isc_nmhandle {
 	int magic;
 	isc_refcount_t references;
 
+	isc_mem_t *mctx;
+
+	/*% Extra data allocated at the end of each isc_nmhandle_t */
+	size_t extrahandlesize;
+
 	/*
 	 * The socket is not 'attached' in the traditional
 	 * reference-counting sense. Instead, we keep all handles in an
@@ -262,6 +268,8 @@ struct isc_nmhandle {
 	int backtrace_size;
 	LINK(isc_nmhandle_t) active_link;
 #endif
+	LINK(isc_nmhandle_t) link;
+
 	void *opaque;
 	char extra[];
 };
@@ -357,7 +365,6 @@ typedef union {
 #define UVREQ_MAGIC    ISC_MAGIC('N', 'M', 'U', 'R')
 #define VALID_UVREQ(t) ISC_MAGIC_VALID(t, UVREQ_MAGIC)
 
-typedef struct isc__nm_uvreq isc__nm_uvreq_t;
 struct isc__nm_uvreq {
 	int magic;
 	isc_nmsocket_t *sock;
@@ -1042,8 +1049,8 @@ struct isc_nmsocket {
 	 * 'spare' handles for that can be reused to avoid allocations,
 	 * for UDP.
 	 */
-	isc_astack_t *inactivehandles;
-	isc_astack_t *inactivereqs;
+	uint_fast32_t ih;
+	ISC_LIST(isc_nmhandle_t) inactivehandles;
 
 	/*%
 	 * Used to wait for TCP listening events to complete, and
@@ -1088,7 +1095,7 @@ struct isc_nmsocket {
 	/*%
 	 * Current number of active handles.
 	 */
-	atomic_int_fast32_t ah;
+	atomic_uint_fast32_t ah;
 
 	/*% Buffer for TCPDNS processing */
 	size_t buf_size;

lib/isc/netmgr/netmgr.c

@@ -140,7 +140,7 @@ int isc_dscp_check_value = -1;
 static void
 nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG);
 static void
-nmhandle_free(isc_nmsocket_t *sock, isc_nmhandle_t *handle);
+nmhandle_free(isc_mem_t *mctx, isc_nmhandle_t *handle);
 static isc_threadresult_t
 nm_thread(isc_threadarg_t worker0);
 static void
@@ -1204,8 +1204,7 @@ isc___nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target FLARG) {
  */
 static void
 nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) {
-	isc_nmhandle_t *handle = NULL;
-	isc__nm_uvreq_t *uvreq = NULL;
+	isc_nmhandle_t *handle;
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(!isc__nmsocket_active(sock));
@@ -1256,10 +1255,6 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) {
 		isc___nmsocket_detach(&sock->outer FLARG_PASS);
 	}
 
-	while ((handle = isc_astack_pop(sock->inactivehandles)) != NULL) {
-		nmhandle_free(sock, handle);
-	}
-
 	if (sock->buf != NULL) {
 		isc_mem_put(sock->mgr->mctx, sock->buf, sock->buf_size);
 	}
@@ -1268,15 +1263,18 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) {
 		isc_quota_detach(&sock->quota);
 	}
 
+	handle = ISC_LIST_HEAD(sock->inactivehandles);
+	while (handle != NULL) {
+		sock->ih--;
+		isc_nmhandle_t *next = ISC_LIST_NEXT(handle, link);
+		ISC_LIST_DEQUEUE(sock->inactivehandles, handle, link);
+		nmhandle_free(sock->mgr->mctx, handle);
+		handle = next;
+	}
+	INSIST(sock->ih == 0);
+
 	sock->pquota = NULL;
 
-	isc_astack_destroy(sock->inactivehandles);
-
-	while ((uvreq = isc_astack_pop(sock->inactivereqs)) != NULL) {
-		isc_mem_put(sock->mgr->mctx, uvreq, sizeof(*uvreq));
-	}
-
-	isc_astack_destroy(sock->inactivereqs);
 	sock->magic = 0;
 
 	isc_condition_destroy(&sock->scond);
@@ -1302,7 +1300,7 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) {
 
 static void
 nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) {
-	int active_handles;
+	uint_fast32_t active_handles;
 	bool destroy = false;
 
 	NETMGR_TRACE_LOG("%s():%p->references = %" PRIuFAST32 "\n", __func__,
@@ -1331,11 +1329,12 @@ nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) {
 		return;
 	}
 
-	active_handles = atomic_load(&sock->ah);
+	active_handles = atomic_load_acquire(&sock->ah);
 	if (sock->children != NULL) {
 		for (size_t i = 0; i < sock->nchildren; i++) {
 			LOCK(&sock->children[i].lock);
-			active_handles += atomic_load(&sock->children[i].ah);
+			active_handles +=
+				atomic_load_acquire(&sock->children[i].ah);
 			UNLOCK(&sock->children[i].lock);
 		}
 	}
@@ -1344,7 +1343,8 @@ nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) {
 		destroy = true;
 	}
 
-	NETMGR_TRACE_LOG("%s:%p->active_handles = %d, .statichandle = %p\n",
+	NETMGR_TRACE_LOG("%s:%p->active_handles = %" PRIuFAST32
+			 ", .statichandle = %p\n",
 			 __func__, sock, active_handles, sock->statichandle);
 
 	if (destroy) {
@@ -1467,12 +1467,13 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
 	REQUIRE(sock != NULL);
 	REQUIRE(mgr != NULL);
 
-	*sock = (isc_nmsocket_t){ .type = type,
-				  .fd = -1,
-				  .inactivehandles = isc_astack_new(
-					  mgr->mctx, ISC_NM_HANDLES_STACK_SIZE),
-				  .inactivereqs = isc_astack_new(
-					  mgr->mctx, ISC_NM_REQS_STACK_SIZE) };
+	*sock = (isc_nmsocket_t){
+		.type = type,
+		.fd = -1,
+	};
+
+	sock->ih = 0;
+	ISC_LIST_INIT(sock->inactivehandles);
 
 	if (iface != NULL) {
 		family = iface->type.sa.sa_family;
@@ -1544,6 +1545,7 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
 	isc_condition_init(&sock->cond);
 	isc_condition_init(&sock->scond);
 	isc_refcount_init(&sock->references, 1);
+	atomic_init(&sock->ah, 0);
 
 #if HAVE_LIBNGHTTP2
 	memset(&sock->tlsstream, 0, sizeof(sock->tlsstream));
@@ -1559,7 +1561,6 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
 	atomic_init(&sock->listening, 0);
 	atomic_init(&sock->closed, 0);
 	atomic_init(&sock->destroying, 0);
-	atomic_init(&sock->ah, 0);
 	atomic_init(&sock->client, 0);
 	atomic_init(&sock->connecting, false);
 	atomic_init(&sock->keepalive, false);
@@ -1602,37 +1603,37 @@ isc__nm_free_uvbuf(isc_nmsocket_t *sock, const uv_buf_t *buf) {
 	worker->recvbuf_inuse = false;
 }
 
-static isc_nmhandle_t *
-alloc_handle(isc_nmsocket_t *sock) {
-	isc_nmhandle_t *handle =
-		isc_mem_get(sock->mgr->mctx,
-			    sizeof(isc_nmhandle_t) + sock->extrahandlesize);
-
-	*handle = (isc_nmhandle_t){ .magic = NMHANDLE_MAGIC };
-#ifdef NETMGR_TRACE
-	ISC_LINK_INIT(handle, active_link);
-#endif
-	isc_refcount_init(&handle->references, 1);
-
-	return (handle);
-}
-
 isc_nmhandle_t *
 isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer,
 		   isc_sockaddr_t *local FLARG) {
 	isc_nmhandle_t *handle = NULL;
+	uint_fast32_t ah;
 
 	REQUIRE(VALID_NMSOCK(sock));
 
-	handle = isc_astack_pop(sock->inactivehandles);
+	REQUIRE(isc__nm_in_netthread());
+	REQUIRE(sock->tid == isc_nm_tid());
+
+	handle = ISC_LIST_HEAD(sock->inactivehandles);
+	if (handle != NULL) {
+		sock->ih--;
+		ISC_LIST_DEQUEUE(sock->inactivehandles, handle, link);
+		INSIST(handle->extrahandlesize == sock->extrahandlesize);
+	}
 
 	if (handle == NULL) {
-		handle = alloc_handle(sock);
-	} else {
-		isc_refcount_init(&handle->references, 1);
-		INSIST(VALID_NMHANDLE(handle));
+		handle = isc_mem_get(sock->mgr->mctx,
+				     sizeof(isc_nmhandle_t) +
+					     sock->extrahandlesize);
+		*handle = (isc_nmhandle_t){
+			.extrahandlesize = sock->extrahandlesize,
+		};
 	}
 
+	isc_refcount_init(&handle->references, 1);
+
+	ISC_LINK_INIT(handle, link);
+
 	NETMGR_TRACE_LOG(
 		"isc__nmhandle_get():handle %p->references = %" PRIuFAST32 "\n",
 		handle, isc_refcount_current(&handle->references));
@@ -1655,10 +1656,24 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer,
 		handle->local = sock->iface;
 	}
 
-	(void)atomic_fetch_add(&sock->ah, 1);
+	ah = atomic_fetch_add_relaxed(&sock->ah, 1) + 1;
+
+	/*
+	 * Garbage collect one more handle if the number of inactive handles
+	 * exceeds number of active handles * 2; this will gradually cleanup the
+	 * unused handles after a spike on an active server.
+	 */
+	if (sock->ih > ah * 2 + 600) {
+		isc_nmhandle_t *tail = ISC_LIST_TAIL(sock->inactivehandles);
+		INSIST(tail != NULL);
+		sock->ih--;
+		ISC_LIST_DEQUEUE(sock->inactivehandles, tail, link);
+		nmhandle_free(sock->mgr->mctx, tail);
+	}
 
 #ifdef NETMGR_TRACE
 	LOCK(&sock->lock);
+	ISC_LINK_INIT(handle, active_link);
 	ISC_LIST_APPEND(sock->active_handles, handle, active_link);
 	UNLOCK(&sock->lock);
 #endif
@@ -1694,6 +1709,8 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer,
 	}
 #endif
 
+	handle->magic = NMHANDLE_MAGIC;
+
 	return (handle);
 }
 
@@ -1722,8 +1739,8 @@ isc_nmhandle_is_stream(isc_nmhandle_t *handle) {
 }
 
 static void
-nmhandle_free(isc_nmsocket_t *sock, isc_nmhandle_t *handle) {
-	size_t extra = sock->extrahandlesize;
+nmhandle_free(isc_mem_t *mctx, isc_nmhandle_t *handle) {
+	size_t extra = handle->extrahandlesize;
 
 	isc_refcount_destroy(&handle->references);
 
@@ -1733,35 +1750,30 @@ nmhandle_free(isc_nmsocket_t *sock, isc_nmhandle_t *handle) {
 
 	*handle = (isc_nmhandle_t){ .magic = 0 };
 
-	isc_mem_put(sock->mgr->mctx, handle, sizeof(isc_nmhandle_t) + extra);
+	isc_mem_put(mctx, handle, sizeof(isc_nmhandle_t) + extra);
 }
 
 static void
 nmhandle_deactivate(isc_nmsocket_t *sock, isc_nmhandle_t *handle) {
-	bool reuse = false;
-
-	/*
-	 * We do all of this under lock to avoid races with socket
-	 * destruction.  We have to do this now, because at this point the
-	 * socket is either unused or still attached to event->sock.
-	 */
-	LOCK(&sock->lock);
+	REQUIRE(isc__nm_in_netthread());
+	REQUIRE(sock->tid == isc_nm_tid());
 
 #ifdef NETMGR_TRACE
 	ISC_LIST_UNLINK(sock->active_handles, handle, active_link);
 #endif
 
-	INSIST(atomic_fetch_sub(&sock->ah, 1) > 0);
+	INSIST(atomic_fetch_sub_release(&sock->ah, 1) > 0);
 
 #if !__SANITIZE_ADDRESS && !__SANITIZE_THREAD__
-	if (atomic_load(&sock->active)) {
-		reuse = isc_astack_trypush(sock->inactivehandles, handle);
+	if (!isc__nmsocket_closing(sock)) {
+		ISC_LIST_ENQUEUE(sock->inactivehandles, handle, link);
+		sock->ih++;
+	} else {
+		nmhandle_free(sock->mgr->mctx, handle);
 	}
+#else  /* !__SANITIZE_ADDRESS && !__SANITIZE_THREAD__ */
+	nmhandle_free(sock->mgr->mctx, handle);
 #endif /* !__SANITIZE_ADDRESS && !__SANITIZE_THREAD__ */
-	if (!reuse) {
-		nmhandle_free(sock, handle);
-	}
-	UNLOCK(&sock->lock);
 }
 
 void
@@ -2293,7 +2305,7 @@ processbuffer(isc_nmsocket_t *sock) {
 void
 isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
 	for (;;) {
-		int_fast32_t ah = atomic_load(&sock->ah);
+		uint_fast32_t ah = atomic_load_acquire(&sock->ah);
 		isc_result_t result = processbuffer(sock);
 		switch (result) {
 		case ISC_R_NOMORE:
@@ -2472,14 +2484,7 @@ isc___nm_uvreq_get(isc_nm_t *mgr, isc_nmsocket_t *sock FLARG) {
 	REQUIRE(VALID_NM(mgr));
 	REQUIRE(VALID_NMSOCK(sock));
 
-	if (sock != NULL && isc__nmsocket_active(sock)) {
-		/* Try to reuse one */
-		req = isc_astack_pop(sock->inactivereqs);
-	}
-
-	if (req == NULL) {
-		req = isc_mem_get(mgr->mctx, sizeof(*req));
-	}
+	req = isc_mem_get(mgr->mctx, sizeof(*req));
 
 	*req = (isc__nm_uvreq_t){ .magic = 0 };
 	ISC_LINK_INIT(req, link);
@@ -2512,10 +2517,7 @@ isc___nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock FLARG) {
 	handle = req->handle;
 	req->handle = NULL;
 
-	if (!isc__nmsocket_active(sock) ||
-	    !isc_astack_trypush(sock->inactivereqs, req)) {
-		isc_mem_put(sock->mgr->mctx, req, sizeof(*req));
-	}
+	isc_mem_put(sock->mgr->mctx, req, sizeof(*req));
 
 	if (handle != NULL) {
 		isc__nmhandle_detach(&handle FLARG_PASS);
@@ -2694,7 +2696,8 @@ isc__nm_connectcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
 		  isc_result_t eresult, bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(VALID_UVREQ(uvreq));
-	REQUIRE(VALID_NMHANDLE(uvreq->handle));
+	REQUIRE((uvreq->handle == NULL && eresult != ISC_R_SUCCESS) ||
+		VALID_NMHANDLE(uvreq->handle));
 
 	if (!async) {
 		isc__netievent_connectcb_t ievent = { .sock = sock,
@@ -2721,7 +2724,8 @@ isc__nm_async_connectcb(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(VALID_UVREQ(uvreq));
-	REQUIRE(VALID_NMHANDLE(uvreq->handle));
+	REQUIRE((uvreq->handle == NULL && eresult != ISC_R_SUCCESS) ||
+		VALID_NMHANDLE(uvreq->handle));
 	REQUIRE(ievent->sock->tid == isc_nm_tid());
 	REQUIRE(uvreq->cb.connect != NULL);
 

lib/isc/netmgr/tcp.c

@@ -209,6 +209,9 @@ isc__nm_async_tcpconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(sock->type == isc_nm_tcpsocket);
 	REQUIRE(sock->parent == NULL);
 	REQUIRE(sock->tid == isc_nm_tid());
+	REQUIRE(req->handle == NULL);
+
+	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = tcp_connect_direct(sock, req);
 	if (result != ISC_R_SUCCESS) {
@@ -324,7 +327,6 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 	req->cbarg = cbarg;
 	req->peer = *peer;
 	req->local = *local;
-	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = isc__nm_socket(sa_family, SOCK_STREAM, 0, &sock->fd);
 	if (result != ISC_R_SUCCESS) {

lib/isc/netmgr/tcpdns.c

@@ -177,6 +177,9 @@ isc__nm_async_tcpdnsconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(sock->type == isc_nm_tcpdnssocket);
 	REQUIRE(sock->parent == NULL);
 	REQUIRE(sock->tid == isc_nm_tid());
+	REQUIRE(req->handle == NULL);
+
+	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = tcpdns_connect_direct(sock, req);
 	if (result != ISC_R_SUCCESS) {
@@ -283,7 +286,6 @@ isc_nm_tcpdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 	req->cbarg = cbarg;
 	req->peer = *peer;
 	req->local = *local;
-	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = isc__nm_socket(sa_family, SOCK_STREAM, 0, &sock->fd);
 	if (result != ISC_R_SUCCESS) {

lib/isc/netmgr/tlsdns.c

@@ -193,6 +193,9 @@ isc__nm_async_tlsdnsconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(sock->type == isc_nm_tlsdnssocket);
 	REQUIRE(sock->parent == NULL);
 	REQUIRE(sock->tid == isc_nm_tid());
+	REQUIRE(req->handle == NULL);
+
+	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = tlsdns_connect_direct(sock, req);
 	if (result != ISC_R_SUCCESS) {
@@ -339,7 +342,6 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 	req->cbarg = cbarg;
 	req->peer = *peer;
 	req->local = *local;
-	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = isc__nm_socket(sa_family, SOCK_STREAM, 0, &sock->fd);
 	if (result != ISC_R_SUCCESS) {

lib/isc/netmgr/udp.c

@@ -953,6 +953,9 @@ isc__nm_async_udpconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(sock->type == isc_nm_udpsocket);
 	REQUIRE(sock->parent == NULL);
 	REQUIRE(sock->tid == isc_nm_tid());
+	REQUIRE(req->handle == NULL);
+
+	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = udp_connect_direct(sock, req);
 	if (result != ISC_R_SUCCESS) {
@@ -1005,7 +1008,6 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 	req->cbarg = cbarg;
 	req->peer = *peer;
 	req->local = *local;
-	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
 
 	result = isc__nm_socket(sa_family, SOCK_DGRAM, 0, &sock->fd);
 	if (result != ISC_R_SUCCESS) {

lib/isc/tests/doh_test.c

@@ -115,6 +115,7 @@ static isc_nm_http_endpoints_t *endpoints = NULL;
 #endif
 
 typedef struct csdata {
+	isc_mem_t *mctx;
 	isc_nm_recv_cb_t reply_cb;
 	void *cb_arg;
 	isc_region_t region;
@@ -124,28 +125,28 @@ static void
 connect_send_cb(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	csdata_t data;
 
-	REQUIRE(VALID_NMHANDLE(handle));
-
 	(void)atomic_fetch_sub(&active_cconnects, 1);
 	memmove(&data, arg, sizeof(data));
-	isc_mem_put(handle->sock->mgr->mctx, arg, sizeof(data));
+	isc_mem_put(data.mctx, arg, sizeof(data));
 	if (result != ISC_R_SUCCESS) {
 		goto error;
 	}
 
+	REQUIRE(VALID_NMHANDLE(handle));
+
 	result = isc__nm_http_request(handle, &data.region, data.reply_cb,
 				      data.cb_arg);
 	if (result != ISC_R_SUCCESS) {
 		goto error;
 	}
 
-	isc_mem_put(handle->sock->mgr->mctx, data.region.base,
-		    data.region.length);
+	isc_mem_put(data.mctx, data.region.base, data.region.length);
+	isc_mem_detach(&data.mctx);
 	return;
 error:
 	data.reply_cb(handle, result, NULL, data.cb_arg);
-	isc_mem_put(handle->sock->mgr->mctx, data.region.base,
-		    data.region.length);
+	isc_mem_put(data.mctx, data.region.base, data.region.length);
+	isc_mem_detach(&data.mctx);
 	if (result == ISC_R_TOOMANYOPENFILES) {
 		atomic_store(&slowdown, true);
 	} else {
@@ -166,6 +167,7 @@ connect_send_request(isc_nm_t *mgr, const char *uri, bool post,
 	memmove(copy.base, region->base, region->length);
 	data = isc_mem_get(mgr->mctx, sizeof(*data));
 	*data = (csdata_t){ .reply_cb = cb, .cb_arg = cbarg, .region = copy };
+	isc_mem_attach(mgr->mctx, &data->mctx);
 	if (tls) {
 		ctx = client_tlsctx;
 	}
@@ -700,12 +702,12 @@ doh_timeout_recovery_GET(void **state) {
 static void
 doh_receive_send_reply_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 			  isc_region_t *region, void *cbarg) {
-	isc_nmhandle_t *thandle = NULL;
-	assert_non_null(handle);
 	UNUSED(region);
 
-	isc_nmhandle_attach(handle, &thandle);
 	if (eresult == ISC_R_SUCCESS) {
+		isc_nmhandle_t *thandle = NULL;
+		REQUIRE(VALID_NMHANDLE(handle));
+		isc_nmhandle_attach(handle, &thandle);
 		int_fast64_t sends = atomic_fetch_sub(&nsends, 1);
 		atomic_fetch_add(&csends, 1);
 		atomic_fetch_add(&creads, 1);
@@ -724,10 +726,10 @@ doh_receive_send_reply_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 				assert_true(eresult == ISC_R_SUCCESS);
 			}
 		}
+		isc_nmhandle_detach(&thandle);
 	} else {
 		atomic_store(&was_error, true);
 	}
-	isc_nmhandle_detach(&thandle);
 }
 
 static isc_threadresult_t