From: Ondřej Surý <ondrej@sury.org>
Date: Tue, 6 Apr 2021 16:27:38 +0000 (+0200)
Subject: Refactor async callbacks and fix the double tlsdnsconnect callback
X-Git-Tag: v9.17.12~10^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=72ef5f465d22e7a815288d46196f911770f4e87d;p=thirdparty%2Fbind9.git

Refactor async callbacks and fix the double tlsdnsconnect callback

The isc_nm_tlsdnsconnect() call could end up with two connect callbacks
called when the timeout fired and the TCP connection was aborted,
but the TLS handshake was not complete yet.  isc__nm_connecttimeout_cb()
forgot to clean up sock->tls.pending_req when the connect callback was
called with ISC_R_TIMEDOUT, leading to a second callback running later.

A new argument has been added to the isc__nm_*_failed_connect_cb and
isc__nm_*_failed_read_cb functions, to indicate whether the callback
needs to run asynchronously or not.
---

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index edca27fdebb..ed285f8367f 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -1853,7 +1853,8 @@ isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
 void
 isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
 void
-isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			      bool async);
 
 isc_result_t
 isc__nm_tcpdns_processbuffer(isc_nmsocket_t *sock);
@@ -1899,9 +1900,9 @@ void
 isc__nm_failed_accept_cb(isc_nmsocket_t *sock, isc_result_t eresult);
 void
 isc__nm_failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
-			  isc_result_t eresult);
+			  isc_result_t eresult, bool async);
 void
-isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, bool async);
 
 void
 isc__nmsocket_connecttimeout_cb(uv_timer_t *timer);
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 407637081f0..34828e0c2b2 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -1662,26 +1662,26 @@ isc__nm_failed_accept_cb(isc_nmsocket_t *sock, isc_result_t eresult) {
 
 void
 isc__nm_failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
-			  isc_result_t eresult) {
+			  isc_result_t eresult, bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(VALID_UVREQ(req));
 	REQUIRE(sock->tid == isc_nm_tid());
-	REQUIRE(atomic_load(&sock->connecting));
 	REQUIRE(req->cb.connect != NULL);
 
 	isc__nmsocket_timer_stop(sock);
 	uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
 
-	atomic_store(&sock->connecting, false);
+	INSIST(atomic_compare_exchange_strong(&sock->connecting,
+					      &(bool){ true }, false));
 
 	isc__nmsocket_clearcb(sock);
-	isc__nm_connectcb(sock, req, eresult, true);
+	isc__nm_connectcb(sock, req, eresult, async);
 
 	isc__nmsocket_prep_destroy(sock);
 }
 
 void
-isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+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:
@@ -1694,7 +1694,7 @@ isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 		isc__nm_tcpdns_failed_read_cb(sock, result);
 		return;
 	case isc_nm_tlsdnssocket:
-		isc__nm_tlsdns_failed_read_cb(sock, result);
+		isc__nm_tlsdns_failed_read_cb(sock, result, async);
 		return;
 	default:
 		INSIST(0);
@@ -1711,20 +1711,27 @@ isc__nmsocket_connecttimeout_cb(uv_timer_t *timer) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
 	REQUIRE(atomic_load(&sock->connecting));
-	REQUIRE(atomic_load(&sock->client));
 	REQUIRE(VALID_UVREQ(req));
 	REQUIRE(VALID_NMHANDLE(req->handle));
 
 	isc__nmsocket_timer_stop(sock);
 
+	if (sock->tls.pending_req != NULL) {
+		REQUIRE(req == sock->tls.pending_req);
+		sock->tls.pending_req = NULL;
+	}
+
 	/* Call the connect callback directly */
+
 	req->cb.connect(req->handle, ISC_R_TIMEDOUT, req->cbarg);
 
 	/* Timer is not running, cleanup and shutdown everything */
 	if (!isc__nmsocket_timer_running(sock)) {
+		INSIST(atomic_compare_exchange_strong(&sock->connecting,
+						      &(bool){ true }, false));
+		isc__nm_uvreq_put(&req, sock);
 		isc__nmsocket_clearcb(sock);
 		isc__nmsocket_shutdown(sock);
-		atomic_store(&sock->connecting, false);
 	}
 }
 
@@ -1746,10 +1753,10 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer) {
 
 		if (!isc__nmsocket_timer_running(sock)) {
 			isc__nmsocket_clearcb(sock);
-			isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+			isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		}
 	} else {
-		isc__nm_failed_read_cb(sock, ISC_R_TIMEDOUT);
+		isc__nm_failed_read_cb(sock, ISC_R_TIMEDOUT, false);
 	}
 }
 
diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 4c5e45167c8..e4d71829993 100644
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -139,7 +139,6 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
 
 	r = uv_timer_init(&worker->loop, &sock->timer);
 	RUNTIME_CHECK(r == 0);
-	uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
 
 	r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd);
 	if (r != 0) {
@@ -232,6 +231,10 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 	isc__nmsocket_timer_stop(sock);
 	uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
 
+	if (!atomic_load(&sock->connecting)) {
+		return;
+	}
+
 	req = uv_handle_get_data((uv_handle_t *)uvreq);
 
 	REQUIRE(VALID_UVREQ(req));
@@ -257,8 +260,6 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 		goto error;
 	}
 
-	uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
-
 	isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CONNECT]);
 	r = uv_tcp_getpeername(&sock->uv_handle.tcp, (struct sockaddr *)&ss,
 			       &(int){ sizeof(ss) });
@@ -277,7 +278,7 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 	return;
 
 error:
-	isc__nm_failed_connect_cb(sock, req, result);
+	isc__nm_failed_connect_cb(sock, req, result, false);
 }
 
 void
diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index b2dba833ed5..7b2efa419ec 100644
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -104,7 +104,6 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
 
 	r = uv_timer_init(&worker->loop, &sock->timer);
 	RUNTIME_CHECK(r == 0);
-	uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
 
 	if (isc__nm_closing(sock)) {
 		result = ISC_R_CANCELED;
@@ -206,19 +205,16 @@ tcpdns_connect_cb(uv_connect_t *uvreq, int status) {
 	isc__nmsocket_timer_stop(sock);
 	uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
 
+	if (!atomic_load(&sock->connecting)) {
+		return;
+	}
+
 	req = uv_handle_get_data((uv_handle_t *)uvreq);
 
 	REQUIRE(VALID_UVREQ(req));
 	REQUIRE(VALID_NMHANDLE(req->handle));
 
-	if (!atomic_load(&sock->connecting)) {
-		/*
-		 * The connect was cancelled from timeout; just clean up
-		 * the req.
-		 */
-		isc__nm_uvreq_put(&req, sock);
-		return;
-	} else if (isc__nmsocket_closing(sock)) {
+	if (isc__nmsocket_closing(sock)) {
 		/* Socket was closed midflight by isc__nm_tcpdns_shutdown() */
 		result = ISC_R_CANCELED;
 		goto error;
@@ -249,7 +245,7 @@ tcpdns_connect_cb(uv_connect_t *uvreq, int status) {
 	return;
 
 error:
-	isc__nm_failed_connect_cb(sock, req, result);
+	isc__nm_failed_connect_cb(sock, req, result, false);
 }
 
 void
@@ -720,7 +716,7 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 	if (isc__nmsocket_closing(sock)) {
 		sock->reading = true;
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		return;
 	}
 
@@ -823,7 +819,7 @@ isc__nm_tcpdns_read_cb(uv_stream_t *stream, ssize_t nread,
 	REQUIRE(buf != NULL);
 
 	if (isc__nmsocket_closing(sock)) {
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, true);
 		goto free;
 	}
 
@@ -833,7 +829,7 @@ isc__nm_tcpdns_read_cb(uv_stream_t *stream, ssize_t nread,
 					 sock->statsindex[STATID_RECVFAIL]);
 		}
 
-		isc__nm_failed_read_cb(sock, isc__nm_uverr2result(nread));
+		isc__nm_failed_read_cb(sock, isc__nm_uverr2result(nread), true);
 		goto free;
 	}
 
@@ -1381,7 +1377,7 @@ isc__nm_tcpdns_shutdown(isc_nmsocket_t *sock) {
 	}
 
 	if (sock->statichandle != NULL) {
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		return;
 	}
 
@@ -1421,7 +1417,7 @@ isc__nm_async_tcpdnscancel(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
 
-	isc__nm_failed_read_cb(sock, ISC_R_EOF);
+	isc__nm_failed_read_cb(sock, ISC_R_EOF, false);
 }
 
 void
diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index fe437bf83bb..7b80dcc43ad 100644
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -196,6 +196,8 @@ isc__nm_async_tlsdnsconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 	result = tlsdns_connect_direct(sock, req);
 	if (result != ISC_R_SUCCESS) {
+		INSIST(atomic_compare_exchange_strong(&sock->connecting,
+						      &(bool){ true }, false));
 		isc__nmsocket_clearcb(sock);
 		isc__nm_connectcb(sock, req, result, true);
 		atomic_store(&sock->active, false);
@@ -219,19 +221,16 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
 
+	if (!atomic_load(&sock->connecting)) {
+		return;
+	}
+
 	req = uv_handle_get_data((uv_handle_t *)uvreq);
 
 	REQUIRE(VALID_UVREQ(req));
 	REQUIRE(VALID_NMHANDLE(req->handle));
 
-	if (!atomic_load(&sock->connecting)) {
-		/*
-		 * The connect was cancelled from timeout; just clean up
-		 * the req.
-		 */
-		isc__nm_uvreq_put(&req, sock);
-		return;
-	} else if (isc__nmsocket_closing(sock)) {
+	if (isc__nmsocket_closing(sock)) {
 		/* Socket was closed midflight by isc__nm_tlsdns_shutdown() */
 		result = ISC_R_CANCELED;
 		goto error;
@@ -300,7 +299,7 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status) {
 	return;
 
 error:
-	isc__nm_failed_connect_cb(sock, req, result);
+	isc__nm_failed_connect_cb(sock, req, result, false);
 }
 
 void
@@ -328,6 +327,7 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 	sock->result = ISC_R_DEFAULT;
 	sock->tls.ctx = sslctx;
 	atomic_init(&sock->client, true);
+	atomic_init(&sock->connecting, true);
 
 	req = isc__nm_uvreq_get(mgr, sock);
 	req->cb.connect = cb;
@@ -338,14 +338,11 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 
 	result = isc__nm_socket(sa_family, SOCK_STREAM, 0, &sock->fd);
 	if (result != ISC_R_SUCCESS) {
-		if (isc__nm_in_netthread()) {
-			sock->tid = isc_nm_tid();
-		}
-		isc__nmsocket_clearcb(sock);
-		isc__nm_connectcb(sock, req, result, true);
-		atomic_store(&sock->closed, true);
-		isc__nmsocket_detach(&sock);
-		return;
+		goto failure;
+	}
+
+	if (isc__nm_closing(sock)) {
+		goto failure;
 	}
 
 	/* 2 minute timeout */
@@ -373,6 +370,18 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 	atomic_store(&sock->active, true);
 	BROADCAST(&sock->scond);
 	UNLOCK(&sock->lock);
+	return;
+failure:
+	if (isc__nm_in_netthread()) {
+		sock->tid = isc_nm_tid();
+	}
+
+	INSIST(atomic_compare_exchange_strong(&sock->connecting,
+					      &(bool){ true }, false));
+	isc__nmsocket_clearcb(sock);
+	isc__nm_connectcb(sock, req, result, true);
+	atomic_store(&sock->closed, true);
+	isc__nmsocket_detach(&sock);
 }
 
 static uv_os_sock_t
@@ -774,7 +783,8 @@ isc__nm_async_tlsdnsstop(isc__networker_t *worker, isc__netievent_t *ev0) {
 }
 
 void
-isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			      bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(result != ISC_R_SUCCESS);
 
@@ -784,7 +794,7 @@ isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 	if (sock->tls.pending_req != NULL) {
 		isc__nm_uvreq_t *req = sock->tls.pending_req;
 		sock->tls.pending_req = NULL;
-		isc__nm_failed_connect_cb(sock, req, ISC_R_CANCELED);
+		isc__nm_failed_connect_cb(sock, req, ISC_R_CANCELED, async);
 	}
 
 	if (!sock->recv_read) {
@@ -860,13 +870,13 @@ isc__nm_async_tlsdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 	if (isc__nmsocket_closing(sock)) {
 		sock->reading = true;
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		return;
 	}
 
 	result = tls_cycle(sock);
 	if (result != ISC_R_SUCCESS) {
-		isc__nm_failed_read_cb(sock, result);
+		isc__nm_failed_read_cb(sock, result, false);
 	}
 }
 
@@ -1062,8 +1072,8 @@ tls_cycle_input(isc_nmsocket_t *sock) {
 			isc__nmsocket_timer_stop(sock);
 			uv_handle_set_data((uv_handle_t *)&sock->timer, sock);
 
-			atomic_store(&sock->connecting, false);
-
+			INSIST(atomic_compare_exchange_strong(
+				&sock->connecting, &(bool){ true }, false));
 			isc__nm_connectcb(sock, req, ISC_R_SUCCESS, true);
 		}
 		async_tlsdns_cycle(sock);
@@ -1080,9 +1090,10 @@ tls_error(isc_nmsocket_t *sock, isc_result_t result) {
 		if (atomic_load(&sock->connecting)) {
 			isc__nm_uvreq_t *req = sock->tls.pending_req;
 			sock->tls.pending_req = NULL;
-			isc__nm_failed_connect_cb(sock, req, result);
+
+			isc__nm_failed_connect_cb(sock, req, result, false);
 		} else {
-			isc__nm_tlsdns_failed_read_cb(sock, result);
+			isc__nm_tlsdns_failed_read_cb(sock, result, false);
 		}
 		break;
 	case TLS_STATE_ERROR:
@@ -1299,7 +1310,7 @@ isc__nm_tlsdns_read_cb(uv_stream_t *stream, ssize_t nread,
 	REQUIRE(buf != NULL);
 
 	if (isc__nmsocket_closing(sock)) {
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, true);
 		goto free;
 	}
 
@@ -1309,7 +1320,7 @@ isc__nm_tlsdns_read_cb(uv_stream_t *stream, ssize_t nread,
 					 sock->statsindex[STATID_RECVFAIL]);
 		}
 
-		isc__nm_failed_read_cb(sock, isc__nm_uverr2result(nread));
+		isc__nm_failed_read_cb(sock, isc__nm_uverr2result(nread), true);
 
 		goto free;
 	}
@@ -1324,13 +1335,13 @@ isc__nm_tlsdns_read_cb(uv_stream_t *stream, ssize_t nread,
 	rv = BIO_write_ex(sock->tls.app_wbio, buf->base, (size_t)nread, &len);
 
 	if (rv <= 0 || (size_t)nread != len) {
-		isc__nm_failed_read_cb(sock, ISC_R_TLSERROR);
+		isc__nm_failed_read_cb(sock, ISC_R_TLSERROR, true);
 		goto free;
 	}
 
 	result = tls_cycle(sock);
 	if (result != ISC_R_SUCCESS) {
-		isc__nm_failed_read_cb(sock, result);
+		isc__nm_failed_read_cb(sock, result, true);
 	}
 free:
 	async_tlsdns_cycle(sock);
@@ -1924,7 +1935,8 @@ isc__nm_tlsdns_shutdown(isc_nmsocket_t *sock) {
 			isc__nm_uvreq_t *req = sock->tls.pending_req;
 			sock->tls.pending_req = NULL;
 
-			isc__nm_failed_connect_cb(sock, req, ISC_R_CANCELED);
+			isc__nm_failed_connect_cb(sock, req, ISC_R_CANCELED,
+						  false);
 			return;
 		}
 
@@ -1936,7 +1948,7 @@ isc__nm_tlsdns_shutdown(isc_nmsocket_t *sock) {
 	}
 
 	if (sock->statichandle != NULL) {
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		return;
 	}
 
@@ -1976,7 +1988,7 @@ isc__nm_async_tlsdnscancel(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
 
-	isc__nm_failed_read_cb(sock, ISC_R_EOF);
+	isc__nm_failed_read_cb(sock, ISC_R_EOF, false);
 }
 
 void
diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 02981f27236..b7bbe6598e8 100644
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -383,7 +383,7 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 	 *   we can free the buffer and bail.
 	 */
 	if (addr == NULL) {
-		isc__nm_failed_read_cb(sock, ISC_R_EOF);
+		isc__nm_failed_read_cb(sock, ISC_R_EOF, false);
 		goto free;
 	}
 
@@ -391,12 +391,13 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 	 * - If the socket is no longer active.
 	 */
 	if (!isc__nmsocket_active(sock)) {
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		goto free;
 	}
 
 	if (nrecv < 0) {
-		isc__nm_failed_read_cb(sock, isc__nm_uverr2result(nrecv));
+		isc__nm_failed_read_cb(sock, isc__nm_uverr2result(nrecv),
+				       false);
 		goto free;
 	}
 
@@ -874,7 +875,7 @@ isc__nm_async_udpread(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 	if (isc__nmsocket_closing(sock)) {
 		sock->reading = true;
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		return;
 	}
 
@@ -1089,7 +1090,7 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock) {
 	 * interested in the callback.
 	 */
 	if (sock->statichandle != NULL) {
-		isc__nm_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
 		return;
 	}
 
@@ -1133,5 +1134,5 @@ isc__nm_async_udpcancel(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(sock->tid == isc_nm_tid());
 	REQUIRE(atomic_load(&sock->client));
 
-	isc__nm_failed_read_cb(sock, ISC_R_EOF);
+	isc__nm_failed_read_cb(sock, ISC_R_EOF, false);
 }
diff --git a/lib/isc/tests/netmgr_test.c b/lib/isc/tests/netmgr_test.c
index 04362a361a7..0d363d5774e 100644
--- a/lib/isc/tests/netmgr_test.c
+++ b/lib/isc/tests/netmgr_test.c
@@ -99,7 +99,8 @@ static bool skip_long_tests = false;
 #define T_ADVERTISED 120 * 1000
 #define T_CONNECT    30 * 1000
 
-#define WAIT_REPEATS 100
+/* Wait for 1 second (1000 * 1000 microseconds) */
+#define WAIT_REPEATS 1000
 #define T_WAIT	     1000 /* In microseconds */
 
 #define WAIT_FOR(v, op, val)                                \
@@ -198,11 +199,22 @@ _setup(void **state __attribute__((unused))) {
 		skip_long_tests = true;
 	}
 
+	if (isc_tlsctx_createserver(NULL, NULL, &tcp_listen_tlsctx) !=
+	    ISC_R_SUCCESS) {
+		return (-1);
+	}
+	if (isc_tlsctx_createclient(&tcp_connect_tlsctx) != ISC_R_SUCCESS) {
+		return (-1);
+	}
+
 	return (0);
 }
 
 static int
 _teardown(void **state __attribute__((unused))) {
+	isc_tlsctx_free(&tcp_connect_tlsctx);
+	isc_tlsctx_free(&tcp_listen_tlsctx);
+
 	isc_test_end();
 
 	return (0);
@@ -279,14 +291,6 @@ nm_setup(void **state __attribute__((unused))) {
 	isc__nm_closesocket(tcp_listen_sock);
 	tcp_listen_sock = -1;
 
-	if (isc_tlsctx_createserver(NULL, NULL, &tcp_listen_tlsctx) !=
-	    ISC_R_SUCCESS) {
-		return (-1);
-	}
-	if (isc_tlsctx_createclient(&tcp_connect_tlsctx) != ISC_R_SUCCESS) {
-		return (-1);
-	}
-
 	atomic_store(&do_send, true);
 	atomic_store(&nsends, esends);
 
@@ -336,9 +340,6 @@ nm_teardown(void **state __attribute__((unused))) {
 	isc_nm_destroy(&listen_nm);
 	assert_null(listen_nm);
 
-	isc_tlsctx_free(&tcp_connect_tlsctx);
-	isc_tlsctx_free(&tcp_listen_tlsctx);
-
 	WAIT_FOR_EQ(active_cconnects, 0);
 	WAIT_FOR_EQ(active_csends, 0);
 	WAIT_FOR_EQ(active_csends, 0);
@@ -575,7 +576,8 @@ connect_thread(isc_threadarg_t arg) {
 	isc_sockaddr_fromin6(&connect_addr, &in6addr_loopback, 0);
 
 	while (atomic_load(&do_send)) {
-		uint_fast32_t active = isc_refcount_current(&active_cconnects);
+		uint_fast32_t active =
+			isc_refcount_increment0(&active_cconnects);
 		if (active > workers) {
 			/*
 			 * If we have more active connections than workers,
@@ -594,7 +596,6 @@ connect_thread(isc_threadarg_t arg) {
 
 static void
 udp_connect(isc_nm_t *nm) {
-	isc_refcount_increment0(&active_cconnects);
 	isc_nm_udpconnect(nm, (isc_nmiface_t *)&udp_connect_addr,
 			  (isc_nmiface_t *)&udp_listen_addr, connect_connect_cb,
 			  NULL, T_CONNECT, 0);
@@ -1054,7 +1055,6 @@ tcp_listener_init_quota(size_t nthreads);
 
 static void
 tcp_connect(isc_nm_t *nm) {
-	isc_refcount_increment0(&active_cconnects);
 	isc_nm_tcpconnect(nm, (isc_nmiface_t *)&tcp_connect_addr,
 			  (isc_nmiface_t *)&tcp_listen_addr, connect_connect_cb,
 			  NULL, 1, 0);
@@ -1466,7 +1466,6 @@ tcp_half_recv_half_send_quota(void **state) {
 
 static void
 tcpdns_connect(isc_nm_t *nm) {
-	isc_refcount_increment0(&active_cconnects);
 	isc_nm_tcpdnsconnect(nm, (isc_nmiface_t *)&tcp_connect_addr,
 			     (isc_nmiface_t *)&tcp_listen_addr,
 			     connect_connect_cb, NULL, T_CONNECT, 0);
@@ -1824,7 +1823,6 @@ tcpdns_half_recv_half_send(void **state __attribute__((unused))) {
 
 static void
 tlsdns_connect(isc_nm_t *nm) {
-	isc_refcount_increment0(&active_cconnects);
 	isc_nm_tlsdnsconnect(nm, (isc_nmiface_t *)&tcp_connect_addr,
 			     (isc_nmiface_t *)&tcp_listen_addr,
 			     connect_connect_cb, NULL, T_CONNECT, 0,