Fix the streaming read callback shutdown logic

When shutting down TCP sockets, the read callback calling logic was
flawed, it would call either one less callback or one extra.  Fix the
logic in the way:

1. When isc_nm_read() has been called but isc_nm_read_stop() hasn't on
   the handle, the read callback will be called with ISC_R_CANCELED to
   cancel active reading from the socket/handle.

2. When isc_nm_read() has been called and isc_nm_read_stop() has been
   called on the on the handle, the read callback will be called with
   ISC_R_SHUTTINGDOWN to signal that the dormant (not-reading) socket
   is being shut down.

3. The .reading and .recv_read flags are little bit tricky.  The
   .reading flag indicates if the outer layer is reading the data (that
   would be uv_tcp_t for TCP and isc_nmsocket_t (TCP) for TLSStream),
   the .recv_read flag indicates whether somebody is interested in the
   data read from the socket.

   Usually, you would expect that the .reading should be false when
   .recv_read is false, but it gets even more tricky with TLSStream as
   the TLS protocol might need to read from the socket even when sending
   data.

   Fix the usage of the .recv_read and .reading flags in the TLSStream
   to their true meaning - which mostly consist of using .recv_read
   everywhere and then wrapping isc_nm_read() and isc_nm_read_stop()
   with the .reading flag.

4. The TLS failed read helper has been modified to resemble the TCP code
   as much as possible, clearing and re-setting the .recv_read flag in
   the TCP timeout code has been fixed and .recv_read is now cleared
   when isc_nm_read_stop() has been called on the streaming socket.

5. The use of Network Manager in the named_controlconf, isccc_ccmsg, and
   isc_httpd units have been greatly simplified due to the improved design.

6. More unit tests for TCP and TLS testing the shutdown conditions have
   been added.

Co-authored-by: Ondřej Surý <ondrej@isc.org>
Co-authored-by: Artem Boldariev <artem@isc.org>

bin/named/controlconf.c

@@ -27,6 +27,7 @@
 #include <isc/netmgr.h>
 #include <isc/nonce.h>
 #include <isc/random.h>
+#include <isc/refcount.h>
 #include <isc/result.h>
 #include <isc/stdtime.h>
 #include <isc/string.h>
@@ -48,6 +49,8 @@
 #include <named/main.h>
 #include <named/server.h>
 
+#undef NAMED_CONTROLCONF_TRACE
+
 typedef struct controlkey controlkey_t;
 typedef ISC_LIST(controlkey_t) controlkeylist_t;
 
@@ -65,12 +68,8 @@ struct controlkey {
 };
 
 struct controlconnection {
-	isc_nmhandle_t *readhandle;
-	isc_nmhandle_t *sendhandle;
-	isc_nmhandle_t *cmdhandle;
+	isc_refcount_t references;
 	isccc_ccmsg_t ccmsg;
-	bool reading;
-	bool sending;
 	controllistener_t *listener;
 	isccc_sexpr_t *ctrl;
 	isc_buffer_t *buffer;
@@ -83,6 +82,7 @@ struct controlconnection {
 	isc_stdtime_t now;
 	isc_result_t result;
 	ISC_LINK(controlconnection_t) link;
+	bool shuttingdown;
 };
 
 struct controllistener {
@@ -92,7 +92,7 @@ struct controllistener {
 	isc_nmsocket_t *sock;
 	dns_acl_t *acl;
 	bool exiting;
-	isc_refcount_t refs;
+	isc_refcount_t references;
 	controlkeylist_t keys;
 	isc_mutex_t connections_lock;
 	controlconnectionlist_t connections;
@@ -107,7 +107,7 @@ struct controllistener {
 struct named_controls {
 	named_server_t *server;
 	controllistenerlist_t listeners;
-	atomic_bool shuttingdown;
+	bool shuttingdown;
 	isc_mutex_t symtab_lock;
 	isccc_symtab_t *symtab;
 };
@@ -116,6 +116,35 @@ static isc_result_t
 control_newconn(isc_nmhandle_t *handle, isc_result_t result, void *arg);
 static void
 control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg);
+static void
+conn_cleanup(controlconnection_t *conn);
+static void
+conn_free(controlconnection_t *conn);
+
+#if NAMED_CONTROLCONF_TRACE
+#define controllistener_ref(ptr) \
+	controllistener__ref(ptr, __func__, __FILE__, __LINE__)
+#define controllistener_unref(ptr) \
+	controllistener__unref(ptr, __func__, __FILE__, __LINE__)
+#define controllistener_attach(ptr, ptrp) \
+	controllistener__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
+#define controllistener_detach(ptrp) \
+	controllistener__detach(ptrp, __func__, __FILE__, __LINE__)
+ISC_REFCOUNT_TRACE_DECL(controllistener);
+
+#define controlconnection_ref(ptr) \
+	controlconnection__ref(ptr, __func__, __FILE__, __LINE__)
+#define controlconnection_unref(ptr) \
+	controlconnection__unref(ptr, __func__, __FILE__, __LINE__)
+#define controlconnection_attach(ptr, ptrp) \
+	controlconnection__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
+#define controlconnection_detach(ptrp) \
+	controlconnection__detach(ptrp, __func__, __FILE__, __LINE__)
+ISC_REFCOUNT_TRACE_DECL(controlconnection);
+#else
+ISC_REFCOUNT_DECL(controllistener);
+ISC_REFCOUNT_DECL(controlconnection);
+#endif
 
 #define CLOCKSKEW 300
 
@@ -144,7 +173,7 @@ free_listener(controllistener_t *listener) {
 	INSIST(listener->exiting);
 	INSIST(ISC_LIST_EMPTY(listener->connections));
 
-	isc_refcount_destroy(&listener->refs);
+	isc_refcount_destroy(&listener->references);
 
 	REQUIRE(listener->sock == NULL);
 
@@ -158,18 +187,27 @@ free_listener(controllistener_t *listener) {
 	isc_mem_putanddetach(&listener->mctx, listener, sizeof(*listener));
 }
 
-static void
-maybe_free_listener(controllistener_t *listener) {
-	if (isc_refcount_decrement(&listener->refs) == 1) {
-		free_listener(listener);
-	}
-}
+#if NAMED_CONTROLCONF_TRACE
+ISC_REFCOUNT_TRACE_IMPL(controllistener, free_listener);
+ISC_REFCOUNT_TRACE_IMPL(controlconnection, conn_free);
+#else
+ISC_REFCOUNT_IMPL(controllistener, free_listener);
+ISC_REFCOUNT_IMPL(controlconnection, conn_free);
+#endif
 
 static void
 shutdown_listener(controllistener_t *listener) {
 	if (!listener->exiting) {
 		char socktext[ISC_SOCKADDR_FORMATSIZE];
 
+		for (controlconnection_t *conn =
+			     ISC_LIST_HEAD(listener->connections);
+		     conn != NULL; conn = ISC_LIST_HEAD(listener->connections))
+		{
+			control_recvmessage(conn->ccmsg.handle,
+					    ISC_R_SHUTTINGDOWN, conn);
+		}
+
 		ISC_LIST_UNLINK(listener->controls->listeners, listener, link);
 
 		isc_sockaddr_format(&listener->address, socktext,
@@ -188,7 +226,7 @@ shutdown_listener(controllistener_t *listener) {
 
 	isc_nm_stoplistening(listener->sock);
 	isc_nmsocket_close(&listener->sock);
-	maybe_free_listener(listener);
+	controllistener_detach(&listener);
 }
 
 static bool
@@ -217,18 +255,12 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	controllistener_t *listener = conn->listener;
 	isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(handle);
 
-	REQUIRE(conn->sending);
-
-	conn->sending = false;
-
 	if (conn->result == ISC_R_SHUTTINGDOWN) {
 		isc_loopmgr_shutdown(named_g_loopmgr);
 		goto cleanup_sendhandle;
 	}
 
-	if (atomic_load_acquire(&listener->controls->shuttingdown) ||
-	    result == ISC_R_SHUTTINGDOWN)
-	{
+	if (listener->controls->shuttingdown || result == ISC_R_SHUTTINGDOWN) {
 		goto cleanup_sendhandle;
 	} else if (result != ISC_R_SUCCESS) {
 		char socktext[ISC_SOCKADDR_FORMATSIZE];
@@ -241,16 +273,13 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 		goto cleanup_sendhandle;
 	}
 
-	isc_nmhandle_attach(handle, &conn->readhandle);
-	conn->reading = true;
-
-	isc_nmhandle_detach(&conn->sendhandle);
-
 	isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn);
-	return;
 
 cleanup_sendhandle:
-	isc_nmhandle_detach(&conn->sendhandle);
+	if (result != ISC_R_SUCCESS) {
+		control_recvmessage(handle, result, conn);
+	}
+	controlconnection_detach(&conn);
 }
 
 static void
@@ -285,7 +314,7 @@ conn_cleanup(controlconnection_t *conn) {
 }
 
 static void
-control_respond(isc_nmhandle_t *handle, controlconnection_t *conn) {
+control_respond(controlconnection_t *conn) {
 	controllistener_t *listener = conn->listener;
 	isccc_sexpr_t *data = NULL;
 	isc_buffer_t b;
@@ -347,7 +376,7 @@ control_respond(isc_nmhandle_t *handle, controlconnection_t *conn) {
 	result = isccc_cc_towire(conn->response, &conn->buffer, conn->alg,
 				 &conn->secret);
 	if (result != ISC_R_SUCCESS) {
-		goto cleanup;
+		return;
 	}
 
 	isc_buffer_init(&b, conn->buffer->base, 4);
@@ -356,19 +385,11 @@ control_respond(isc_nmhandle_t *handle, controlconnection_t *conn) {
 	r.base = conn->buffer->base;
 	r.length = conn->buffer->used;
 
-	isc_nmhandle_attach(handle, &conn->sendhandle);
-	conn->sending = true;
-	conn_cleanup(conn);
-
-	isc_nmhandle_detach(&conn->cmdhandle);
-
-	isc_nm_send(conn->sendhandle, &r, control_senddone, conn);
-
-	return;
+	controlconnection_ref(conn);
+	isccc_ccmsg_sendmessage(&conn->ccmsg, &r, control_senddone, conn);
 
 cleanup:
 	conn_cleanup(conn);
-	isc_nmhandle_detach(&conn->cmdhandle);
 }
 
 static void
@@ -376,19 +397,17 @@ control_command(void *arg) {
 	controlconnection_t *conn = (controlconnection_t *)arg;
 	controllistener_t *listener = conn->listener;
 
-	if (atomic_load_acquire(&listener->controls->shuttingdown)) {
-		conn_cleanup(conn);
-		isc_nmhandle_detach(&conn->cmdhandle);
-		return;
+	if (!listener->controls->shuttingdown) {
+		conn->result = named_control_docommand(
+			conn->request, listener->readonly, &conn->text);
+		control_respond(conn);
 	}
-
-	conn->result = named_control_docommand(conn->request,
-					       listener->readonly, &conn->text);
-	control_respond(conn->cmdhandle, conn);
+	controlconnection_detach(&conn);
 }
 
 static void
-control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
+control_recvmessage(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result,
+		    void *arg) {
 	controlconnection_t *conn = (controlconnection_t *)arg;
 	controllistener_t *listener = conn->listener;
 	controlkey_t *key = NULL;
@@ -396,22 +415,22 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	isccc_time_t exp;
 	uint32_t nonce;
 
-	conn->reading = false;
+	if (conn->shuttingdown) {
+		return;
+	}
 
 	/* Is the server shutting down? */
-	if (atomic_load_acquire(&listener->controls->shuttingdown)) {
-		goto cleanup_readhandle;
+	if (listener->controls->shuttingdown) {
+		result = ISC_R_SHUTTINGDOWN;
 	}
 
 	if (result != ISC_R_SUCCESS) {
 		if (result == ISC_R_SHUTTINGDOWN) {
-			atomic_store_release(&listener->controls->shuttingdown,
-					     true);
+			listener->controls->shuttingdown = true;
 		} else if (result != ISC_R_EOF) {
 			log_invalid(&conn->ccmsg, result);
 		}
-
-		goto cleanup_readhandle;
+		goto cleanup;
 	}
 
 	for (key = ISC_LIST_HEAD(listener->keys); key != NULL;
@@ -419,8 +438,7 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	{
 		isccc_region_t ccregion;
 
-		ccregion.rstart = isc_buffer_base(conn->ccmsg.buffer);
-		ccregion.rend = isc_buffer_used(conn->ccmsg.buffer);
+		isccc_ccmsg_toregion(&conn->ccmsg, &ccregion);
 		conn->secret.rstart = isc_mem_get(listener->mctx,
 						  key->secret.length);
 		memmove(conn->secret.rstart, key->secret.base,
@@ -507,9 +525,6 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 
 	isc_buffer_allocate(listener->mctx, &conn->text, 2 * 2048);
 
-	isc_nmhandle_attach(handle, &conn->cmdhandle);
-	isc_nmhandle_detach(&conn->readhandle);
-
 	if (conn->nonce == 0) {
 		/*
 		 * Establish nonce.
@@ -518,45 +533,33 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 			isc_nonce_buf(&conn->nonce, sizeof(conn->nonce));
 		}
 		conn->result = ISC_R_SUCCESS;
-		control_respond(handle, conn);
+		control_respond(conn);
 		return;
 	}
 
 	/*
 	 * Trigger the command.
 	 */
-
+	controlconnection_ref(conn);
 	isc_async_run(named_g_mainloop, control_command, conn);
 
 	return;
 
 cleanup:
-	conn_cleanup(conn);
-
-cleanup_readhandle:
-	/*
-	 * readhandle could be NULL if we're shutting down,
-	 * but if not we need to detach it.
-	 */
-	if (conn->readhandle != NULL) {
-		isc_nmhandle_detach(&conn->readhandle);
-	}
+	conn->shuttingdown = true;
+	controlconnection_detach(&conn);
 }
 
 static void
-conn_reset(void *arg) {
-	controlconnection_t *conn = (controlconnection_t *)arg;
+conn_free(controlconnection_t *conn) {
 	controllistener_t *listener = conn->listener;
 
+	conn_cleanup(conn);
+
 	if (conn->buffer != NULL) {
 		isc_buffer_free(&conn->buffer);
 	}
 
-	if (conn->reading) {
-		isccc_ccmsg_cancelread(&conn->ccmsg);
-		return;
-	}
-
 	LOCK(&listener->connections_lock);
 	ISC_LIST_UNLINK(listener->connections, conn, link);
 	UNLOCK(&listener->connections_lock);
@@ -567,12 +570,6 @@ conn_reset(void *arg) {
 #endif /* ifdef ENABLE_AFL */
 
 	isccc_ccmsg_invalidate(&conn->ccmsg);
-}
-
-static void
-conn_put(void *arg) {
-	controlconnection_t *conn = (controlconnection_t *)arg;
-	controllistener_t *listener = conn->listener;
 
 	isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 		      NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3),
@@ -580,27 +577,22 @@ conn_put(void *arg) {
 
 	isc_mem_put(listener->mctx, conn, sizeof(*conn));
 
-	maybe_free_listener(listener);
+	controllistener_detach(&listener);
 }
 
 static void
 newconnection(controllistener_t *listener, isc_nmhandle_t *handle) {
-	controlconnection_t *conn = NULL;
+	controlconnection_t *conn = isc_mem_get(listener->mctx, sizeof(*conn));
+	isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
+		      NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3),
+		      "allocate new control connection");
 
-	conn = isc_nmhandle_getdata(handle);
-	if (conn == NULL) {
-		conn = isc_mem_get(listener->mctx, sizeof(*conn));
-		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
-			      NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3),
-			      "allocate new control connection");
-		isc_nmhandle_setdata(handle, conn, conn_reset, conn_put);
-		isc_refcount_increment(&listener->refs);
-	}
+	*conn = (controlconnection_t){ .alg = DST_ALG_UNKNOWN };
 
-	*conn = (controlconnection_t){ .listener = listener,
-				       .reading = false,
-				       .alg = DST_ALG_UNKNOWN };
+	isc_refcount_init(&conn->references, 1);
+	controllistener_attach(listener, &conn->listener);
 
+	/* isccc_ccmsg_init() attaches to the handle */
 	isccc_ccmsg_init(listener->mctx, handle, &conn->ccmsg);
 
 	/* Set a 32 KiB upper limit on incoming message. */
@@ -610,9 +602,6 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) {
 	ISC_LIST_INITANDAPPEND(listener->connections, conn, link);
 	UNLOCK(&listener->connections_lock);
 
-	isc_nmhandle_attach(handle, &conn->readhandle);
-	conn->reading = true;
-
 	isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn);
 }
 
@@ -663,7 +652,7 @@ controls_shutdown(named_controls_t *controls) {
 void
 named_controls_shutdown(named_controls_t *controls) {
 	controls_shutdown(controls);
-	atomic_store_release(&controls->shuttingdown, true);
+	controls->shuttingdown = true;
 }
 
 static isc_result_t
@@ -1095,7 +1084,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp,
 	ISC_LINK_INIT(listener, link);
 	ISC_LIST_INIT(listener->keys);
 	ISC_LIST_INIT(listener->connections);
-	isc_refcount_init(&listener->refs, 1);
+	isc_refcount_init(&listener->references, 1);
 
 	/*
 	 * Make the ACL.
@@ -1177,7 +1166,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp,
 	return;
 
 cleanup:
-	isc_refcount_decrement(&listener->refs);
+	isc_refcount_decrement(&listener->references);
 	listener->exiting = true;
 	free_listener(listener);
 
@@ -1193,8 +1182,6 @@ cleanup:
 	}
 
 	*listenerp = NULL;
-
-	/* XXXDCL return error results? fail hard? */
 }
 
 isc_result_t
@@ -1455,7 +1442,6 @@ named_controls_create(named_server_t *server, named_controls_t **ctrlsp) {
 
 	ISC_LIST_INIT(controls->listeners);
 
-	atomic_init(&controls->shuttingdown, false);
 	isc_mutex_init(&controls->symtab_lock);
 	LOCK(&controls->symtab_lock);
 	result = isccc_cc_createsymtab(&controls->symtab);

bin/rndc/rndc.c

@@ -17,7 +17,6 @@
 #include <stdbool.h>
 #include <stdlib.h>
 
-#include <isc/atomic.h>
 #include <isc/attributes.h>
 #include <isc/buffer.h>
 #include <isc/commandline.h>
@@ -76,18 +75,12 @@ static isccc_region_t secret;
 static bool failed = false;
 static bool c_flag = false;
 static isc_mem_t *rndc_mctx = NULL;
-static atomic_uint_fast32_t sends = 0;
-static atomic_uint_fast32_t recvs = 0;
-static atomic_uint_fast32_t connects = 0;
 static char *command = NULL;
 static char *args = NULL;
 static char program[256];
 static uint32_t serial;
 static bool quiet = false;
 static bool showresult = false;
-static bool shuttingdown = false;
-static isc_nmhandle_t *recvdone_handle = NULL;
-static isc_nmhandle_t *recvnonce_handle = NULL;
 
 static void
 rndc_startconnect(isc_sockaddr_t *addr);
@@ -285,22 +278,11 @@ get_addresses(const char *host, in_port_t port) {
 }
 
 static void
-rndc_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
-	isc_nmhandle_t *sendhandle = (isc_nmhandle_t *)arg;
-
+rndc_senddone(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result,
+	      void *arg ISC_ATTR_UNUSED) {
 	if (result != ISC_R_SUCCESS) {
 		fatal("send failed: %s", isc_result_totext(result));
 	}
-
-	REQUIRE(sendhandle == handle);
-	isc_nmhandle_detach(&sendhandle);
-
-	if (atomic_fetch_sub_release(&sends, 1) == 1 &&
-	    atomic_load_acquire(&recvs) == 0)
-	{
-		shuttingdown = true;
-		isc_loopmgr_shutdown(loopmgr);
-	}
 }
 
 static void
@@ -312,16 +294,10 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	char *errormsg = NULL;
 	char *textmsg = NULL;
 
+	REQUIRE(handle != NULL);
 	REQUIRE(ccmsg != NULL);
 
-	if (shuttingdown && (result == ISC_R_EOF || result == ISC_R_CANCELED)) {
-		atomic_fetch_sub_release(&recvs, 1);
-		if (handle != NULL) {
-			REQUIRE(recvdone_handle == handle);
-			isc_nmhandle_detach(&recvdone_handle);
-		}
-		return;
-	} else if (result == ISC_R_EOF) {
+	if (result == ISC_R_EOF) {
 		fatal("connection to remote host closed.\n"
 		      "* This may indicate that the\n"
 		      "* remote server is using an older\n"
@@ -377,22 +353,15 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 
 	isccc_sexpr_free(&response);
 
-	REQUIRE(recvdone_handle == handle);
-	isc_nmhandle_detach(&recvdone_handle);
-
-	if (atomic_fetch_sub_release(&recvs, 1) == 1 &&
-	    atomic_load_acquire(&sends) == 0)
-	{
-		shuttingdown = true;
-		isc_loopmgr_shutdown(loopmgr);
-	}
+	isccc_ccmsg_invalidate(ccmsg);
+	isc_loopmgr_shutdown(loopmgr);
 }
 
 static void
-rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
+rndc_recvnonce(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result,
+	       void *arg) {
 	isccc_ccmsg_t *ccmsg = (isccc_ccmsg_t *)arg;
 	isccc_sexpr_t *response = NULL;
-	isc_nmhandle_t *sendhandle = NULL;
 	isccc_sexpr_t *_ctrl = NULL;
 	isccc_region_t source;
 	uint32_t nonce;
@@ -404,14 +373,7 @@ rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 
 	REQUIRE(ccmsg != NULL);
 
-	if (shuttingdown && (result == ISC_R_EOF || result == ISC_R_CANCELED)) {
-		atomic_fetch_sub_release(&recvs, 1);
-		if (handle != NULL) {
-			REQUIRE(recvnonce_handle == handle);
-			isc_nmhandle_detach(&recvnonce_handle);
-		}
-		return;
-	} else if (result == ISC_R_EOF) {
+	if (result == ISC_R_EOF) {
 		fatal("connection to remote host closed.\n"
 		      "* This may indicate that the\n"
 		      "* remote server is using an older\n"
@@ -471,17 +433,8 @@ rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	r.base = databuf->base;
 	r.length = databuf->used;
 
-	isc_nmhandle_attach(handle, &recvdone_handle);
-	atomic_fetch_add_relaxed(&recvs, 1);
 	isccc_ccmsg_readmessage(ccmsg, rndc_recvdone, ccmsg);
-
-	isc_nmhandle_attach(handle, &sendhandle);
-	atomic_fetch_add_relaxed(&sends, 1);
-	isc_nm_send(handle, &r, rndc_senddone, sendhandle);
-
-	REQUIRE(recvnonce_handle == handle);
-	isc_nmhandle_detach(&recvnonce_handle);
-	atomic_fetch_sub_release(&recvs, 1);
+	isccc_ccmsg_sendmessage(ccmsg, &r, rndc_senddone, NULL);
 
 	isccc_sexpr_free(&response);
 	isccc_sexpr_free(&request);
@@ -497,13 +450,10 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	isccc_time_t now = isc_stdtime_now();
 	isc_region_t r;
 	isc_buffer_t b;
-	isc_nmhandle_t *connhandle = NULL;
-	isc_nmhandle_t *sendhandle = NULL;
 
 	REQUIRE(ccmsg != NULL);
 
 	if (result != ISC_R_SUCCESS) {
-		atomic_fetch_sub_release(&connects, 1);
 		isc_sockaddr_format(&serveraddrs[currentaddr], socktext,
 				    sizeof(socktext));
 		if (++currentaddr < nserveraddrs) {
@@ -517,8 +467,6 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 		      isc_result_totext(result));
 	}
 
-	isc_nmhandle_attach(handle, &connhandle);
-
 	DO("create message", isccc_cc_createmessage(1, NULL, NULL, ++serial,
 						    now, now + 60, &request));
 	data = isccc_alist_lookup(request, "_data");
@@ -542,19 +490,12 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	r.base = databuf->base;
 	r.length = databuf->used;
 
+	/* isccc_ccmsg_init() attaches to the handle */
 	isccc_ccmsg_init(rndc_mctx, handle, ccmsg);
 	isccc_ccmsg_setmaxsize(ccmsg, 1024 * 1024);
 
-	isc_nmhandle_attach(handle, &recvnonce_handle);
-	atomic_fetch_add_relaxed(&recvs, 1);
 	isccc_ccmsg_readmessage(ccmsg, rndc_recvnonce, ccmsg);
-
-	isc_nmhandle_attach(handle, &sendhandle);
-	atomic_fetch_add_relaxed(&sends, 1);
-	isc_nm_send(handle, &r, rndc_senddone, sendhandle);
-
-	isc_nmhandle_detach(&connhandle);
-	atomic_fetch_sub_release(&connects, 1);
+	isccc_ccmsg_sendmessage(ccmsg, &r, rndc_senddone, NULL);
 
 	isccc_sexpr_free(&request);
 }
@@ -584,7 +525,6 @@ rndc_startconnect(isc_sockaddr_t *addr) {
 		UNREACHABLE();
 	}
 
-	atomic_fetch_add_relaxed(&connects, 1);
 	isc_nm_tcpconnect(netmgr, local, addr, rndc_connected, &rndc_ccmsg,
 			  60000);
 }
@@ -1062,14 +1002,6 @@ main(int argc, char **argv) {
 
 	isc_loopmgr_run(loopmgr);
 
-	/*
-	 * Note: when TCP connections are shut down, there will be a final
-	 * call to the isccc callback routine with &rndc_ccmsg as its
-	 * argument. We therefore need to delay invalidating it until
-	 * after the netmgr is closed down.
-	 */
-	isccc_ccmsg_invalidate(&rndc_ccmsg);
-
 	isc_log_destroy(&log);
 	isc_log_setcontext(NULL);
 

doc/design/netmgr.md

@@ -79,31 +79,103 @@ inactive-handles stack is full or when the socket is destroyed) then the
 associated object's 'put' callback will be called to free any resources
 it allocated.
 
-## UDP listening
+## Streaming Protocols
 
-UDP listener sockets automatically create an array of 'child' sockets,
-each associated with one networker, and all listening on the same address
-via `SO_REUSEADDR`.  (The parent's reference counter is used for all the
-parent and child sockets together; none are destroyed until there are no
-remaining references to any of tem.)
+Currently, we have two streaming protocols available in Network Manager - TCP
+and TLS.  The underlying premise is that they both expose the same interface to
+the clients.
 
-## TCP listening
+### Servers (Listening)
 
-A TCP listener socket cannot listen on multiple threads in parallel,
-so receiving a TCP connection can cause a context switch, but this is
-expected to be rare enough not to impact performance significantly.
+The users of the API calls ``isc_nm_listentcp()`` or ``isc_nm_listentls()`` with
+the accept callback as argument.
 
-When connected, a TCP socket will attach to the system-wide TCP clients
-quota.
+When connection is accepted, the accept callback is called with a handle and
+status and it can return a non-``ISC_R_RESULT`` to abort the connection.
 
-## TCP listening for DNS
+The accept callback should generally immediately call ``isc_nm_read()`` to setup
+the read callback.  Not doing so, can lead to a data race - if the NM is shut
+down before the ``isc_nm_read()`` call, the socket can become dangling until
+``isc_nm_read()`` is finally called.
 
-A TCPDNS listener is a wrapper around a TCP socket which specifically
-handles DNS traffic, including the two-byte length field that prepends DNS
-messages over TCP.
+When ``isc_nm_read()`` is called, the read callback will receive:
 
-Other wrapper socket types can be added in the future, such as a TLS socket
-wrapper to implement encryption or an HTTP wrapper to implement the HTTP
-protocol. This will enable the system to have a transport-neutral network
-manager socket over which DNS can be sent without knowing anything about
-transport, encryption, etc.
+- 0-<n> calls with ``ISC_R_SUCCESS`` state
+- exactly 1 call with non-``ISC_R_SUCCESS`` state when the connection is
+  interrupted (locally closed, remotely closed, NM shutting down, etc.)
+
+The ``isc_nm_read_stop()`` can be used to pause reading from the socket and only
+the final non-``ISC_R_SUCCESS`` callback will be received in such case.
+
+### Clients (Connecting)
+
+The users of the API calls ``isc_nm_tcpconnect()`` or ``isc_nm_tlsconnect()``
+with the connect callback as argument.
+
+When connection is established, the connect callback is called with a handle and
+status.
+
+The connect callback should generally immediately call ``isc_nm_read()`` - see
+the same caveat in the accepting part.
+
+When ``isc__nm_read()`` is called on the connected socket, the read callback
+will receive:
+
+- 0-<n> calls with ``ISC_R_SUCCESS`` state
+- exactly 1 call with non-``ISC_R_SUCCESS`` state when the connection is
+  interrupted (locally closed, remotely closed, NM shutting down, etc.)
+
+The ``isc_nm_read_stop()`` can be used to pause reading from the socket and only
+the final non-``ISC_R_SUCCESS`` callback will be received in such case.
+
+## DNS Message Protocols
+
+Currently, we have three (four) DNS Message Protocols implemented in the Network Manager:
+
+- UDP
+- StreamDNS (TCPDNS and TLSDNS)
+- HTTP
+
+### Servers (Listening)
+
+The users of the API calls ``isc_nm_listenudp()`` or
+``isc_nm_listenstreamdns()`` with:
+
+- accept callback
+- read callback
+
+The StreamDNS accepts an optional TLS context for DoT (otherwise DNS over TCP
+will be used).
+
+The HTTP listening is more complicated - the users need to setup the endpoints
+with the read callback and pass the 1-<n> endpoints to the
+``isc_nm_listenhttp()`` call.
+
+The accept callback is used only to implement "firewall"-like functionality, it
+could be used to tear down the connection early in the process.
+
+After the connection has been accepted, the read callback will receive:
+
+- 0-<n> calls with ``ISC_R_SUCCESS`` state
+- exactly 1 call with non-``ISC_R_SUCCESS`` state when the connection is
+  interrupted (locally closed, remotely closed, NM shutting down, etc.)
+
+Each read callback will contain a full assembled DNS message.
+
+### Clients (Connecting)
+
+The users of the API calls ``isc_nm_udpconnect()``,
+``isc_nm_streamdnsconnect()``, or ``isc_nm_httpconnect()`` with a connect
+callback.
+
+When connection is established, the connect callback is called with a handle and
+status.
+
+The connect callback should generally immediately call ``isc_nm_read()`` - see
+the caveat in the previous parts.
+
+After the connection has been connected, the read callback will receive exactly
+1 call for each ``isc_nm_read()`` call - either with ``ISC_R_SUCCESS`` if the
+DNS message was successfully read or non-``ISC_R_SUCCESS`` indicating the error
+condition.  The read callback either needs to issue new ``isc_nm_read()`` call
+or detach from the handle if no further messages are required.

lib/isc/httpd.c

@@ -92,8 +92,7 @@ struct isc_httpd {
 	isc_httpdmgr_t *mgr; /*%< our parent */
 	ISC_LINK(isc_httpd_t) link;
 
-	isc_nmhandle_t *handle;	    /* Permanent pointer to handle */
-	isc_nmhandle_t *readhandle; /* Waiting for a read callback */
+	isc_nmhandle_t *handle; /* Permanent pointer to handle */
 
 	int flags;
 
@@ -134,7 +133,6 @@ struct isc_httpdmgr {
 typedef struct isc_httpd_sendreq {
 	isc_mem_t *mctx;
 	isc_httpd_t *httpd;
-	isc_nmhandle_t *handle;
 
 	/*%
 	 * Transmit data state.
@@ -178,9 +176,7 @@ httpd_request(isc_nmhandle_t *, isc_result_t, isc_region_t *, void *);
 static void
 httpd_senddone(isc_nmhandle_t *, isc_result_t, void *);
 static void
-httpd_reset(void *);
-static void
-httpd_put(void *);
+httpd_free(isc_httpd_t *httpd);
 
 static void
 httpd_addheader(isc_httpd_sendreq_t *, const char *, const char *);
@@ -509,8 +505,7 @@ process_request(isc_httpd_t *httpd, size_t last_len) {
 }
 
 static void
-httpd_reset(void *arg) {
-	isc_httpd_t *httpd = (isc_httpd_t *)arg;
+httpd_free(isc_httpd_t *httpd) {
 	isc_httpdmgr_t *httpdmgr = NULL;
 
 	REQUIRE(VALID_HTTPD(httpd));
@@ -533,6 +528,19 @@ httpd_reset(void *arg) {
 	httpd->path = NULL;
 	httpd->up = (isc_url_parser_t){ 0 };
 	isc_time_set(&httpd->if_modified_since, 0, 0);
+
+	httpd->magic = 0;
+	httpd->mgr = NULL;
+
+	isc_mem_put(httpdmgr->mctx, httpd, sizeof(*httpd));
+
+	httpdmgr_detach(&httpdmgr);
+
+#if ENABLE_AFL
+	if (finishhook != NULL) {
+		finishhook();
+	}
+#endif /* ENABLE_AFL */
 }
 
 static void
@@ -567,60 +575,27 @@ isc__httpd_sendreq_new(isc_httpd_t *httpd) {
 	return (req);
 }
 
-static void
-httpd_put(void *arg) {
-	isc_httpd_t *httpd = (isc_httpd_t *)arg;
-	isc_httpdmgr_t *mgr = NULL;
-
-	REQUIRE(VALID_HTTPD(httpd));
-
-	mgr = httpd->mgr;
-	REQUIRE(VALID_HTTPDMGR(mgr));
-
-	httpd->magic = 0;
-	httpd->mgr = NULL;
-
-	isc_mem_put(mgr->mctx, httpd, sizeof(*httpd));
-
-	httpdmgr_detach(&mgr);
-
-#if ENABLE_AFL
-	if (finishhook != NULL) {
-		finishhook();
-	}
-#endif /* ENABLE_AFL */
-}
-
 static void
 new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) {
 	isc_httpd_t *httpd = NULL;
 
 	REQUIRE(VALID_HTTPDMGR(httpdmgr));
 
-	httpd = isc_nmhandle_getdata(handle);
-	if (httpd == NULL) {
-		httpd = isc_mem_get(httpdmgr->mctx, sizeof(*httpd));
-		*httpd = (isc_httpd_t){ .handle = NULL };
-		httpdmgr_attach(httpdmgr, &httpd->mgr);
-	}
+	httpd = isc_mem_get(httpdmgr->mctx, sizeof(*httpd));
+	*httpd = (isc_httpd_t){
+		.magic = HTTPD_MAGIC,
+		.link = ISC_LINK_INITIALIZER,
+	};
 
-	if (httpd->handle == NULL) {
-		isc_nmhandle_setdata(handle, httpd, httpd_reset, httpd_put);
-		httpd->handle = handle;
-	} else {
-		INSIST(httpd->handle == handle);
-	}
+	isc_nmhandle_attach(handle, &httpd->handle);
 
-	ISC_LINK_INIT(httpd, link);
-
-	httpd->magic = HTTPD_MAGIC;
+	httpdmgr_attach(httpdmgr, &httpd->mgr);
 
 	LOCK(&httpdmgr->lock);
 	ISC_LIST_APPEND(httpdmgr->running, httpd, link);
 	UNLOCK(&httpdmgr->lock);
 
-	isc_nmhandle_attach(httpd->handle, &httpd->readhandle);
-	isc_nm_read(handle, httpd_request, httpdmgr);
+	isc_nm_read(handle, httpd_request, httpd);
 }
 
 static isc_result_t
@@ -885,14 +860,12 @@ prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd,
 static void
 httpd_request(isc_nmhandle_t *handle, isc_result_t eresult,
 	      isc_region_t *region, void *arg) {
-	isc_result_t result;
-	isc_httpdmgr_t *mgr = arg;
-	isc_httpd_t *httpd = NULL;
+	isc_httpd_t *httpd = arg;
+	isc_httpdmgr_t *mgr = httpd->mgr;
 	isc_httpd_sendreq_t *req = NULL;
 	isc_region_t r;
 	size_t last_len = 0;
-
-	httpd = isc_nmhandle_getdata(handle);
+	isc_result_t result;
 
 	REQUIRE(VALID_HTTPD(httpd));
 
@@ -902,10 +875,9 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult,
 		goto close_readhandle;
 	}
 
-	REQUIRE(httpd->readhandle == handle);
 	REQUIRE((mgr->flags & ISC_HTTPDMGR_SHUTTINGDOWN) == 0);
 
-	isc_nm_read_stop(httpd->readhandle);
+	isc_nm_read_stop(handle);
 
 	/*
 	 * If we are being called from httpd_senddone(), the last HTTP request
@@ -932,7 +904,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult,
 			goto close_readhandle;
 		}
 
-		/* Wait for more data, the readhandle is still attached */
+		/* Wait for more data, the handle is still attached */
 		isc_nm_read(handle, httpd_request, arg);
 		return;
 	}
@@ -948,16 +920,15 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult,
 	 */
 	isc_buffer_usedregion(req->sendbuffer, &r);
 
-	isc_nmhandle_attach(httpd->handle, &req->handle);
-	isc_nm_send(httpd->handle, &r, httpd_senddone, req);
-
-	isc_nmhandle_detach(&httpd->readhandle);
+	isc_nmhandle_ref(handle);
+	isc_nm_send(handle, &r, httpd_senddone, req);
 	return;
 
 close_readhandle:
-	isc_nm_read_stop(httpd->readhandle);
-	isc_nmhandle_close(httpd->readhandle);
-	isc_nmhandle_detach(&httpd->readhandle);
+	isc_nmhandle_close(httpd->handle);
+	isc_nmhandle_detach(&httpd->handle);
+
+	httpd_free(httpd);
 }
 
 void
@@ -978,9 +949,9 @@ isc_httpdmgr_shutdown(isc_httpdmgr_t **httpdmgrp) {
 
 	httpd = ISC_LIST_HEAD(httpdmgr->running);
 	while (httpd != NULL) {
-		if (httpd->readhandle != NULL) {
-			httpd_request(httpd->readhandle, ISC_R_SHUTTINGDOWN,
-				      NULL, httpdmgr);
+		if (httpd->handle != NULL) {
+			httpd_request(httpd->handle, ISC_R_SUCCESS, NULL,
+				      httpd);
 		}
 		httpd = ISC_LIST_NEXT(httpd, link);
 	}
@@ -1045,17 +1016,16 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
 		goto detach;
 	}
 
-	if (eresult == ISC_R_SUCCESS && (httpd->flags & HTTPD_CLOSE) == 0) {
-		/*
-		 * Calling httpd_request() with region NULL restarts
-		 * reading.
-		 */
-		isc_nmhandle_attach(handle, &httpd->readhandle);
-		httpd_request(handle, ISC_R_SUCCESS, NULL, httpd->mgr);
-	} else {
-		isc_nmhandle_close(handle);
+	if (eresult == ISC_R_SUCCESS && (httpd->flags & HTTPD_CLOSE) != 0) {
+		eresult = ISC_R_EOF;
 	}
 
+	/*
+	 * Calling httpd_request() with region NULL restarts
+	 * reading.
+	 */
+	httpd_request(handle, eresult, NULL, httpd);
+
 detach:
 	isc_nmhandle_detach(&handle);
 	isc__httpd_sendreq_free(req);

lib/isc/include/isc/netmgr.h

@@ -18,6 +18,7 @@
 #include <unistd.h>
 
 #include <isc/mem.h>
+#include <isc/refcount.h>
 #include <isc/region.h>
 #include <isc/result.h>
 #include <isc/tls.h>
@@ -148,25 +149,18 @@ isc_nmsocket_set_max_streams(isc_nmsocket_t *listener,
  */
 
 #if ISC_NETMGR_TRACE
-#define isc_nmhandle_attach(handle, dest) \
-	isc__nmhandle_attach(handle, dest, __FILE__, __LINE__, __func__)
-#define isc_nmhandle_detach(handlep) \
-	isc__nmhandle_detach(handlep, __FILE__, __LINE__, __func__)
-#define FLARG_PASS , file, line, func
-#define FLARG                                                                  \
-	, const char *file ISC_ATTR_UNUSED, unsigned int line ISC_ATTR_UNUSED, \
-		const char *func ISC_ATTR_UNUSED
+#define isc_nmhandle_ref(ptr) \
+	isc_nmhandle__ref(ptr, __func__, __FILE__, __LINE__)
+#define isc_nmhandle_unref(ptr) \
+	isc_nmhandle__unref(ptr, __func__, __FILE__, __LINE__)
+#define isc_nmhandle_attach(ptr, ptrp) \
+	isc_nmhandle__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
+#define isc_nmhandle_detach(ptrp) \
+	isc_nmhandle__detach(ptrp, __func__, __FILE__, __LINE__)
+ISC_REFCOUNT_TRACE_DECL(isc_nmhandle);
 #else
-#define isc_nmhandle_attach(handle, dest) isc__nmhandle_attach(handle, dest)
-#define isc_nmhandle_detach(handlep)	  isc__nmhandle_detach(handlep)
-#define FLARG_PASS
-#define FLARG
+ISC_REFCOUNT_DECL(isc_nmhandle);
 #endif
-
-void
-isc__nmhandle_attach(isc_nmhandle_t *handle, isc_nmhandle_t **dest FLARG);
-void
-isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG);
 /*%<
  * Increment/decrement the reference counter in a netmgr handle.
  *
@@ -175,7 +169,6 @@ isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG);
  * event loop. When references go to zero, the associated socket will be
  * closed and deleted.
  */
-#undef FLARG
 
 void *
 isc_nmhandle_getdata(isc_nmhandle_t *handle);
@@ -310,12 +303,18 @@ isc_nm_cancelread(isc_nmhandle_t *handle);
  * active handles with a result code of ISC_R_CANCELED.
  *
  * Requires:
- * \li	'sock' is a valid datagram-like netmgr socket
+ * \li	'handle' is a valid netmgr handle
  * \li	...for which a read/recv callback has been defined.
  */
 
 void
 isc_nmhandle_close(isc_nmhandle_t *handle);
+/*%<
+ * Close the active handle - no further read callbacks will happen.
+ *
+ * Requires:
+ * 'li	'handle' is a valid netmgr handle
+ */
 
 void
 isc_nm_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
@@ -349,7 +348,8 @@ isc_nm_listentcp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
 
 void
 isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
-		  isc_nm_cb_t cb, void *cbarg, unsigned int timeout);
+		  isc_nm_cb_t connect_cb, void *connect_cbarg,
+		  unsigned int timeout);
 /*%<
  * Create a socket using netmgr 'mgr', bind it to the address 'local',
  * and connect it to the address 'peer'.
@@ -507,7 +507,8 @@ isc_nm_listentls(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
 
 void
 isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
-		  isc_nm_cb_t cb, void *cbarg, isc_tlsctx_t *ctx,
+		  isc_nm_cb_t connect_cb, void *connect_cbarg,
+		  isc_tlsctx_t			    *ctx,
 		  isc_tlsctx_client_session_cache_t *client_sess_cache,
 		  unsigned int			     timeout);
 

lib/isc/netmgr/http.c

@@ -975,15 +975,13 @@ client_submit_request(isc_nm_http_session_t *session, http_cstream_t *stream) {
  * Read callback from TLS socket.
  */
 static void
-http_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
-	    void *data) {
+http_readcb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result,
+	    isc_region_t *region, void *data) {
 	isc_nm_http_session_t *session = (isc_nm_http_session_t *)data;
 	ssize_t readlen;
 
 	REQUIRE(VALID_HTTP2_SESSION(session));
 
-	UNUSED(handle);
-
 	if (result != ISC_R_SUCCESS) {
 		if (result != ISC_R_TIMEDOUT) {
 			session->reading = false;

lib/isc/netmgr/netmgr-int.h

@@ -115,53 +115,58 @@ STATIC_ASSERT(ISC_NETMGR_TCP_RECVBUF_SIZE <= ISC_NETMGR_RECVBUF_SIZE,
 
 #if defined(__linux__)
 #include <syscall.h>
-#define gettid() (uint32_t) syscall(SYS_gettid)
+#define gettid() (uint64_t) syscall(SYS_gettid)
+#elif defined(__FreeBSD__)
+#include <pthread_np.h>
+#define gettid() (uint64_t)(pthread_getthreadid_np())
+#elif defined(__OpenBSD__)
+#include <unistd.h>
+#define gettid() (uint64_t)(getthrid())
+#elif defined(__NetBSD__)
+#include <lwp.h>
+#define gettid() (uint64_t)(_lwp_self())
+#elif defined(__DragonFly__)
+#include <unistd.h>
+#define gettid() (uint64_t)(lwp_gettid())
 #else
-#define gettid() (uint32_t) pthread_self()
+#define gettid() (uint64_t)(pthread_self())
 #endif
 
 #define NETMGR_TRACE_LOG(format, ...)                                \
-	fprintf(stderr, "%" PRIu32 ":%d:%s:%u:%s:" format, gettid(), \
+	fprintf(stderr, "%" PRIu64 ":%d:%s:%u:%s:" format, gettid(), \
 		isc_tid(), file, line, func, __VA_ARGS__)
 
-#define FLARG                                                                  \
-	, const char *file ISC_ATTR_UNUSED, unsigned int line ISC_ATTR_UNUSED, \
-		const char *func ISC_ATTR_UNUSED
-#define FLARG_PASS , file, line, func
-#define FLARG_IEVENT(ievent)              \
-	const char *file = ievent->file;  \
-	unsigned int line = ievent->line; \
-	const char *func = ievent->func;
-#define FLARG_IEVENT_PASS(ievent) \
-	ievent->file = file;      \
-	ievent->line = line;      \
-	ievent->func = func;
+#define FLARG                                                                 \
+	, const char *func ISC_ATTR_UNUSED, const char *file ISC_ATTR_UNUSED, \
+		unsigned int line ISC_ATTR_UNUSED
+
+#define FLARG_PASS , func, file, line
 #define isc__nm_uvreq_get(sock) \
-	isc___nm_uvreq_get(sock, __FILE__, __LINE__, __func__)
+	isc___nm_uvreq_get(sock, __func__, __FILE__, __LINE__)
 #define isc__nm_uvreq_put(req) \
-	isc___nm_uvreq_put(req, __FILE__, __LINE__, __func__)
+	isc___nm_uvreq_put(req, __func__, __FILE__, __LINE__)
 #define isc__nmsocket_init(sock, mgr, type, iface, parent)            \
-	isc___nmsocket_init(sock, mgr, type, iface, parent, __FILE__, \
-			    __LINE__, __func__)
+	isc___nmsocket_init(sock, mgr, type, iface, parent, __func__, \
+			    __FILE__, __LINE__)
 #define isc__nmsocket_put(sockp) \
-	isc___nmsocket_put(sockp, __FILE__, __LINE__, __func__)
+	isc___nmsocket_put(sockp, __func__, __FILE__, __LINE__)
 #define isc__nmsocket_attach(sock, target) \
-	isc___nmsocket_attach(sock, target, __FILE__, __LINE__, __func__)
+	isc___nmsocket_attach(sock, target, __func__, __FILE__, __LINE__)
 #define isc__nmsocket_detach(socketp) \
-	isc___nmsocket_detach(socketp, __FILE__, __LINE__, __func__)
+	isc___nmsocket_detach(socketp, __func__, __FILE__, __LINE__)
 #define isc__nmsocket_close(socketp) \
-	isc___nmsocket_close(socketp, __FILE__, __LINE__, __func__)
+	isc___nmsocket_close(socketp, __func__, __FILE__, __LINE__)
 #define isc__nmhandle_get(sock, peer, local) \
-	isc___nmhandle_get(sock, peer, local, __FILE__, __LINE__, __func__)
+	isc___nmhandle_get(sock, peer, local, __func__, __FILE__, __LINE__)
 #define isc__nmsocket_prep_destroy(sock) \
-	isc___nmsocket_prep_destroy(sock, __FILE__, __LINE__, __func__)
+	isc___nmsocket_prep_destroy(sock, __func__, __FILE__, __LINE__)
+#define isc__nm_get_read_req(sock, sockaddr) \
+	isc___nm_get_read_req(sock, sockaddr, __func__, __FILE__, __LINE__)
 #else
 #define NETMGR_TRACE_LOG(format, ...)
 
 #define FLARG
 #define FLARG_PASS
-#define FLARG_IEVENT(ievent)
-#define FLARG_IEVENT_PASS(ievent)
 #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) \
@@ -173,6 +178,8 @@ STATIC_ASSERT(ISC_NETMGR_TCP_RECVBUF_SIZE <= ISC_NETMGR_RECVBUF_SIZE,
 #define isc__nmhandle_get(sock, peer, local) \
 	isc___nmhandle_get(sock, peer, local)
 #define isc__nmsocket_prep_destroy(sock) isc___nmsocket_prep_destroy(sock)
+#define isc__nm_get_read_req(sock, sockaddr) \
+	isc___nm_get_read_req(sock, sockaddr)
 #endif
 
 typedef struct isc__nm_uvreq isc__nm_uvreq_t;
@@ -504,6 +511,7 @@ struct isc_nmsocket {
 		size_t nsending;
 		bool tcp_nodelay_value;
 		isc_nmsocket_tls_send_req_t *send_req; /*%< Send req to reuse */
+		bool reading;
 	} tlsstream;
 
 #if HAVE_LIBNGHTTP2
@@ -637,7 +645,6 @@ struct isc_nmsocket {
 	isc_nmhandle_t *recv_handle;
 	isc_nm_recv_cb_t recv_cb;
 	void *recv_cbarg;
-	bool recv_read;
 
 	isc_nm_cb_t connect_cb;
 	void *connect_cbarg;
@@ -806,12 +813,6 @@ isc__nm_udp_close(isc_nmsocket_t *sock);
  * Close a UDP socket.
  */
 
-void
-isc__nm_udp_cancelread(isc_nmhandle_t *handle);
-/*%<
- * Stop reading on a connected UDP handle.
- */
-
 void
 isc__nm_udp_shutdown(isc_nmsocket_t *sock);
 /*%<
@@ -862,12 +863,6 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock);
  * sockets.
  */
 
-void
-isc__nm_tcp_cancelread(isc_nmhandle_t *handle);
-/*%<
- * Stop reading on a connected TCP handle.
- */
-
 void
 isc__nm_tcp_stoplistening(isc_nmsocket_t *sock);
 /*%<
@@ -1089,9 +1084,6 @@ isc__nm_streamdns_stoplistening(isc_nmsocket_t *sock);
 void
 isc__nm_streamdns_cleanup_data(isc_nmsocket_t *sock);
 
-void
-isc__nm_streamdns_cancelread(isc_nmhandle_t *handle);
-
 void
 isc__nmhandle_streamdns_cleartimeout(isc_nmhandle_t *handle);
 
@@ -1249,7 +1241,7 @@ isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
 			   bool async);
 
 isc__nm_uvreq_t *
-isc__nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr);
+isc___nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr FLARG);
 
 void
 isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf);

lib/isc/netmgr/netmgr.c

@@ -580,15 +580,6 @@ nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) {
 	}
 }
 
-void
-isc_nmhandle_close(isc_nmhandle_t *handle) {
-	REQUIRE(VALID_NMHANDLE(handle));
-	REQUIRE(VALID_NMSOCK(handle->sock));
-
-	isc__nmsocket_clearcb(handle->sock);
-	isc__nm_failed_read_cb(handle->sock, ISC_R_EOF, false);
-}
-
 void
 isc___nmsocket_prep_destroy(isc_nmsocket_t *sock FLARG) {
 	REQUIRE(sock->parent == NULL);
@@ -904,19 +895,6 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t const *peer,
 	return (handle);
 }
 
-void
-isc__nmhandle_attach(isc_nmhandle_t *handle, isc_nmhandle_t **handlep FLARG) {
-	REQUIRE(VALID_NMHANDLE(handle));
-	REQUIRE(handlep != NULL && *handlep == NULL);
-
-	NETMGR_TRACE_LOG("isc__nmhandle_attach():handle %p->references = "
-			 "%" PRIuFAST32 "\n",
-			 handle, isc_refcount_current(&handle->references) + 1);
-
-	isc_refcount_increment(&handle->references);
-	*handlep = handle;
-}
-
 bool
 isc_nmhandle_is_stream(isc_nmhandle_t *handle) {
 	REQUIRE(VALID_NMHANDLE(handle));
@@ -1003,26 +981,11 @@ nmhandle_destroy(isc_nmhandle_t *handle) {
 		    handle);
 }
 
-void
-isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG) {
-	isc_nmhandle_t *handle = NULL;
-
-	REQUIRE(handlep != NULL);
-	REQUIRE(VALID_NMHANDLE(*handlep));
-
-	handle = *handlep;
-	*handlep = NULL;
-
-	REQUIRE(handle->sock->tid == isc_tid());
-
-	NETMGR_TRACE_LOG("isc__nmhandle_detach():%p->references = %" PRIuFAST32
-			 "\n",
-			 handle, isc_refcount_current(&handle->references) - 1);
-
-	if (isc_refcount_decrement(&handle->references) == 1) {
-		nmhandle_destroy(handle);
-	}
-}
+#if ISC_NETMGR_TRACE
+ISC_REFCOUNT_TRACE_IMPL(isc_nmhandle, nmhandle_destroy)
+#else
+ISC_REFCOUNT_IMPL(isc_nmhandle, nmhandle_destroy);
+#endif
 
 void *
 isc_nmhandle_getdata(isc_nmhandle_t *handle) {
@@ -1117,7 +1080,6 @@ isc__nmsocket_connecttimeout_cb(uv_timer_t *timer) {
 	 */
 	REQUIRE(!sock->timedout);
 	sock->timedout = true;
-	isc__nmsocket_clearcb(sock);
 	isc__nmsocket_shutdown(sock);
 }
 
@@ -1168,13 +1130,10 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer) {
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_tid());
-	REQUIRE(sock->reading);
 
 	if (sock->client) {
 		uv_timer_stop(timer);
 
-		sock->recv_read = false;
-
 		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, false);
@@ -1182,7 +1141,7 @@ 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, false);
+			isc__nm_failed_read_cb(sock, ISC_R_TIMEDOUT, false);
 		}
 	} else {
 		isc__nm_failed_read_cb(sock, ISC_R_TIMEDOUT, false);
@@ -1285,7 +1244,7 @@ isc__nmsocket_timer_stop(isc_nmsocket_t *sock) {
 }
 
 isc__nm_uvreq_t *
-isc__nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr) {
+isc___nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr FLARG) {
 	isc__nm_uvreq_t *req = NULL;
 
 	req = isc__nm_uvreq_get(sock);
@@ -1295,16 +1254,32 @@ isc__nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr) {
 	switch (sock->type) {
 	case isc_nm_tcpsocket:
 	case isc_nm_tlssocket:
+#if ISC_NETMGR_TRACE
+		isc_nmhandle__attach(sock->statichandle,
+				     &req->handle FLARG_PASS);
+#else
 		isc_nmhandle_attach(sock->statichandle, &req->handle);
+#endif
 		break;
 	case isc_nm_streamdnssocket:
+#if ISC_NETMGR_TRACE
+		isc_nmhandle__attach(sock->recv_handle,
+				     &req->handle FLARG_PASS);
+#else
 		isc_nmhandle_attach(sock->recv_handle, &req->handle);
+#endif
 		break;
 	default:
 		if (sock->client && sock->statichandle != NULL) {
+#if ISC_NETMGR_TRACE
+			isc_nmhandle__attach(sock->statichandle,
+					     &req->handle FLARG_PASS);
+#else
 			isc_nmhandle_attach(sock->statichandle, &req->handle);
+#endif
 		} else {
-			req->handle = isc__nmhandle_get(sock, sockaddr, NULL);
+			req->handle = isc___nmhandle_get(sock, sockaddr,
+							 NULL FLARG_PASS);
 		}
 		break;
 	}
@@ -1357,7 +1332,7 @@ isc__nm_start_reading(isc_nmsocket_t *sock) {
 	isc_result_t result = ISC_R_SUCCESS;
 	int r;
 
-	if (sock->reading) {
+	if (uv_is_active(&sock->uv_handle.handle)) {
 		return (ISC_R_SUCCESS);
 	}
 
@@ -1375,8 +1350,6 @@ isc__nm_start_reading(isc_nmsocket_t *sock) {
 	}
 	if (r != 0) {
 		result = isc_uverr2result(r);
-	} else {
-		sock->reading = true;
 	}
 
 	return (result);
@@ -1386,7 +1359,7 @@ void
 isc__nm_stop_reading(isc_nmsocket_t *sock) {
 	int r;
 
-	if (!sock->reading) {
+	if (!uv_is_active(&sock->uv_handle.handle)) {
 		return;
 	}
 
@@ -1402,7 +1375,6 @@ isc__nm_stop_reading(isc_nmsocket_t *sock) {
 	default:
 		UNREACHABLE();
 	}
-	sock->reading = false;
 }
 
 bool
@@ -1578,7 +1550,11 @@ isc___nm_uvreq_put(isc__nm_uvreq_t **reqp FLARG) {
 	ISC_LIST_UNLINK(sock->active_uvreqs, req, active_link);
 
 	if (handle != NULL) {
-		isc__nmhandle_detach(&handle FLARG_PASS);
+#if ISC_NETMGR_TRACE
+		isc_nmhandle__detach(&handle, func, file, line);
+#else
+		isc_nmhandle_detach(&handle);
+#endif
 	}
 
 	isc_mempool_put(sock->worker->uvreq_pool, req);
@@ -1659,20 +1635,37 @@ isc_nm_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 	}
 }
 
-void
-isc_nm_cancelread(isc_nmhandle_t *handle) {
+static void
+cancelread_cb(void *arg) {
+	isc_nmhandle_t *handle = arg;
+
 	REQUIRE(VALID_NMHANDLE(handle));
+	REQUIRE(VALID_NMSOCK(handle->sock));
+	REQUIRE(handle->sock->tid == isc_tid());
+
+	REQUIRE(handle->sock->tid == isc_tid());
 
 	switch (handle->sock->type) {
 	case isc_nm_udpsocket:
-		isc__nm_udp_cancelread(handle);
-		break;
 	case isc_nm_streamdnssocket:
-		isc__nm_streamdns_cancelread(handle);
+	case isc_nm_httpsocket:
+		isc__nm_failed_read_cb(handle->sock, ISC_R_CANCELED, false);
 		break;
 	default:
 		UNREACHABLE();
 	}
+
+	isc_nmhandle_detach(&handle);
+}
+
+void
+isc_nm_cancelread(isc_nmhandle_t *handle) {
+	REQUIRE(VALID_NMHANDLE(handle));
+	REQUIRE(VALID_NMSOCK(handle->sock));
+
+	/* Running this directly could cause a dead-lock */
+	isc_nmhandle_ref(handle);
+	isc_async_run(handle->sock->worker->loop, cancelread_cb, handle);
 }
 
 void
@@ -1693,6 +1686,15 @@ isc_nm_read_stop(isc_nmhandle_t *handle) {
 	}
 }
 
+void
+isc_nmhandle_close(isc_nmhandle_t *handle) {
+	REQUIRE(VALID_NMHANDLE(handle));
+	REQUIRE(VALID_NMSOCK(handle->sock));
+
+	isc__nmsocket_clearcb(handle->sock);
+	isc__nmsocket_prep_destroy(handle->sock);
+}
+
 void
 isc_nm_stoplistening(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
@@ -2505,8 +2507,7 @@ nmsocket_dump(isc_nmsocket_t *sock) {
 		"Parent %p, listener %p, server %p, statichandle = "
 		"%p\n",
 		sock->parent, sock->listener, sock->server, sock->statichandle);
-	fprintf(stderr, "Flags:%s%s%s%s%s\n",
-		atomic_load_acquire(&sock->active) ? " active" : "",
+	fprintf(stderr, "Flags:%s%s%s%s%s\n", sock->active ? " active" : "",
 		sock->closing ? " closing" : "",
 		sock->destroying ? " destroying" : "",
 		sock->connecting ? " connecting" : "",

lib/isc/netmgr/streamdns.c

@@ -131,7 +131,7 @@ streamdns_on_complete_dnsmessage(isc_dnsstream_assembler_t *dnsasm,
 	 */
 	bool stop = sock->client;
 
-	sock->recv_read = false;
+	sock->reading = false;
 	if (sock->recv_cb != NULL) {
 		if (!sock->client) {
 			/*
@@ -271,9 +271,7 @@ static void
 streamdns_call_connect_cb(isc_nmsocket_t *sock, isc_nmhandle_t *handle,
 			  const isc_result_t result) {
 	sock->connecting = false;
-	if (sock->connect_cb == NULL) {
-		return;
-	}
+	INSIST(sock->connect_cb != NULL);
 	sock->connect_cb(handle, result, sock->connect_cbarg);
 	if (result != ISC_R_SUCCESS) {
 		isc__nmsocket_clearcb(handle->sock);
@@ -401,12 +399,6 @@ isc_nm_streamdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local,
 	}
 }
 
-static bool
-streamdns_waiting_for_msg(isc_nmsocket_t *sock) {
-	/* There is an unsatisfied read operation pending */
-	return (sock->recv_read);
-}
-
 bool
 isc__nmsocket_streamdns_timer_running(isc_nmsocket_t *sock) {
 	isc_nmsocket_t *transp_sock;
@@ -464,36 +456,45 @@ isc__nmsocket_streamdns_timer_restart(isc_nmsocket_t *sock) {
 static void
 streamdns_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result,
 			 const bool async) {
-	bool destroy = true;
-
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(result != ISC_R_SUCCESS);
 
-	if (sock->recv_cb != NULL && sock->recv_handle != NULL &&
-	    (streamdns_waiting_for_msg(sock) || result == ISC_R_TIMEDOUT))
-	{
-		isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
+	/* Nobody is reading from the socket yet */
+	if (sock->recv_handle == NULL) {
+		goto destroy;
+	}
 
-		INSIST(VALID_NMHANDLE(sock->recv_handle));
+	if (sock->client && result == ISC_R_TIMEDOUT) {
+		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, false);
+		}
 
-		if (result != ISC_R_TIMEDOUT) {
-			sock->recv_read = false;
-			isc_dnsstream_assembler_clear(sock->streamdns.input);
+		if (isc__nmsocket_timer_running(sock)) {
+			/* Timer was restarted, bail-out */
+			return;
+		}
+
+		isc__nmsocket_clearcb(sock);
+
+		goto destroy;
+	}
+
+	isc_dnsstream_assembler_clear(sock->streamdns.input);
+
+	/* Nobody expects the callback if isc_nm_read() wasn't called */
+	if (!sock->client || sock->reading) {
+		sock->reading = false;
+
+		if (sock->recv_cb != NULL) {
+			isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
 			isc__nmsocket_clearcb(sock);
-		} else if (sock->client) {
-			sock->recv_read = false;
-		}
-		isc__nm_readcb(sock, req, result, async);
-		if (result == ISC_R_TIMEDOUT &&
-		    isc__nmsocket_streamdns_timer_running(sock))
-		{
-			destroy = false;
+			isc__nm_readcb(sock, req, result, async);
 		}
 	}
 
-	if (destroy) {
-		isc__nmsocket_prep_destroy(sock);
-	}
+destroy:
+	isc__nmsocket_prep_destroy(sock);
 }
 
 void
@@ -533,6 +534,10 @@ streamdns_try_close_unused(isc_nmsocket_t *sock) {
 		 * The socket is unused after calling the callback. Let's close
 		 * the underlying connection.
 		 */
+		/* FIXME: call failed_read_cb(?) */
+		if (sock->outerhandle != NULL) {
+			isc_nmhandle_detach(&sock->outerhandle);
+		}
 		isc__nmsocket_prep_destroy(sock);
 	}
 }
@@ -833,7 +838,7 @@ isc__nm_streamdns_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb,
 
 	sock->recv_cb = cb;
 	sock->recv_cbarg = cbarg;
-	sock->recv_read = true;
+	sock->reading = true;
 	isc_nmhandle_attach(handle, &sock->recv_handle);
 
 	/*
@@ -955,29 +960,6 @@ isc__nm_streamdns_stoplistening(isc_nmsocket_t *sock) {
 	isc__nmsocket_stop(sock);
 }
 
-static void
-streamdns_cancelread_cb(void *arg) {
-	isc_nmsocket_t *sock = arg;
-	REQUIRE(VALID_NMSOCK(sock));
-
-	streamdns_failed_read_cb(sock, ISC_R_EOF, false);
-	isc__nmsocket_detach(&sock);
-}
-
-void
-isc__nm_streamdns_cancelread(isc_nmhandle_t *handle) {
-	isc_nmsocket_t *sock = NULL;
-
-	REQUIRE(VALID_NMHANDLE(handle));
-	REQUIRE(VALID_NMSOCK(handle->sock));
-	REQUIRE(handle->sock->type == isc_nm_streamdnssocket);
-
-	sock = handle->sock;
-
-	isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL });
-	isc_async_run(sock->worker->loop, streamdns_cancelread_cb, sock);
-}
-
 void
 isc__nmhandle_streamdns_cleartimeout(isc_nmhandle_t *handle) {
 	isc_nmsocket_t *sock = NULL;

lib/isc/netmgr/tcp.c

@@ -154,16 +154,12 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 	REQUIRE(VALID_UVREQ(req));
 	REQUIRE(VALID_NMHANDLE(req->handle));
 
-	if (sock->timedout) {
+	INSIST(sock->connecting);
+
+	if (sock->timedout || status == UV_ETIMEDOUT) {
+		/* Connection timed-out */
 		result = ISC_R_TIMEDOUT;
 		goto error;
-	} else if (!sock->connecting) {
-		/*
-		 * The connect was cancelled from timeout; just clean up
-		 * the req.
-		 */
-		isc__nm_uvreq_put(&req);
-		return;
 	} else if (isc__nm_closing(worker)) {
 		/* Network manager shutting down */
 		result = ISC_R_SHUTTINGDOWN;
@@ -172,10 +168,6 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 		/* Connection canceled */
 		result = ISC_R_CANCELED;
 		goto error;
-	} else if (status == UV_ETIMEDOUT) {
-		/* Timeout status code here indicates hard error */
-		result = ISC_R_TIMEDOUT;
-		goto error;
 	} else if (status == UV_EADDRINUSE) {
 		/*
 		 * On FreeBSD the TCP connect() call sometimes results in a
@@ -225,7 +217,8 @@ error:
 
 void
 isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
-		  isc_nm_cb_t cb, void *cbarg, unsigned int timeout) {
+		  isc_nm_cb_t connect_cb, void *connect_cbarg,
+		  unsigned int timeout) {
 	isc_result_t result = ISC_R_SUCCESS;
 	isc_nmsocket_t *sock = NULL;
 	isc__nm_uvreq_t *req = NULL;
@@ -238,7 +231,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 	REQUIRE(peer != NULL);
 
 	if (isc__nm_closing(worker)) {
-		cb(NULL, ISC_R_SHUTTINGDOWN, cbarg);
+		connect_cb(NULL, ISC_R_SHUTTINGDOWN, connect_cbarg);
 		return;
 	}
 
@@ -246,7 +239,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 
 	result = isc__nm_socket(sa_family, SOCK_STREAM, 0, &fd);
 	if (result != ISC_R_SUCCESS) {
-		cb(NULL, result, cbarg);
+		connect_cb(NULL, result, connect_cbarg);
 		return;
 	}
 
@@ -258,8 +251,8 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 	sock->client = true;
 
 	req = isc__nm_uvreq_get(sock);
-	req->cb.connect = cb;
-	req->cbarg = cbarg;
+	req->cb.connect = connect_cb;
+	req->cbarg = connect_cbarg;
 	req->peer = *peer;
 	req->local = *local;
 	req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface);
@@ -632,6 +625,7 @@ isc__nm_tcp_stoplistening(isc_nmsocket_t *sock) {
 
 	/* Stop the parent */
 	sock->closed = true;
+
 	isc__nmsocket_prep_destroy(sock);
 }
 
@@ -662,18 +656,12 @@ isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
 	isc__nmsocket_timer_stop(sock);
 	isc__nm_stop_reading(sock);
 
-	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, async);
 	}
 
-destroy:
 	isc__nmsocket_prep_destroy(sock);
 }
 
@@ -694,7 +682,6 @@ isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 
 	sock->recv_cb = cb;
 	sock->recv_cbarg = cbarg;
-	sock->recv_read = true;
 
 	/* Initialize the timer */
 	if (sock->read_timeout == 0) {
@@ -745,7 +732,6 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_tid());
-	REQUIRE(sock->reading);
 	REQUIRE(buf != NULL);
 
 	netmgr = sock->worker->netmgr;
@@ -925,6 +911,10 @@ accept_connection(isc_nmsocket_t *csock) {
 	 */
 	isc_nmhandle_detach(&handle);
 
+	if (csock->statichandle != NULL) {
+		INSIST(csock->recv_cb != NULL);
+	}
+
 	/*
 	 * sock is now attached to the handle.
 	 */
@@ -1184,14 +1174,10 @@ tcp_close_connect_cb(uv_handle_t *handle) {
 
 void
 isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
-	isc__networker_t *worker = NULL;
-
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_tid());
 	REQUIRE(sock->type == isc_nm_tcpsocket);
 
-	worker = sock->worker;
-
 	/*
 	 * If the socket is active, mark it inactive and
 	 * continue. If it isn't active, stop now.
@@ -1201,9 +1187,7 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
 	}
 	sock->active = false;
 
-	if (sock->accepting) {
-		return;
-	}
+	INSIST(!sock->accepting);
 
 	if (sock->connecting) {
 		isc_nmsocket_t *tsock = NULL;
@@ -1212,12 +1196,9 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
 		return;
 	}
 
-	if (sock->statichandle != NULL) {
-		if (isc__nm_closing(worker)) {
-			isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false);
-		} else {
-			isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
-		}
+	/* There's a handle attached to the socket (from accept or connect) */
+	if (sock->statichandle) {
+		isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false);
 		return;
 	}
 
@@ -1243,7 +1224,6 @@ isc__nmhandle_tcp_set_manual_timer(isc_nmhandle_t *handle, const bool manual) {
 	REQUIRE(sock->type == isc_nm_tcpsocket);
 	REQUIRE(sock->tid == isc_tid());
 	REQUIRE(!sock->reading);
-	REQUIRE(!sock->recv_read);
 
 	sock->manual_read_timer = manual;
 }

lib/isc/netmgr/tlsstream.c

@@ -45,6 +45,42 @@
 
 #define TLS_MAX_SEND_BUF_SIZE (UINT16_MAX + UINT16_MAX / 2)
 
+#ifdef ISC_NETMGR_TRACE
+ISC_ATTR_UNUSED static const char *
+tls_status2str(int tls_status) {
+	switch (tls_status) {
+	case SSL_ERROR_NONE:
+		return ("SSL_ERROR_NONE");
+	case SSL_ERROR_ZERO_RETURN:
+		return ("SSL_ERROR_ZERO_RETURN");
+	case SSL_ERROR_WANT_WRITE:
+		return ("SSL_ERROR_WANT_WRITE");
+	case SSL_ERROR_WANT_READ:
+		return ("SSL_ERROR_WANT_READ");
+	case SSL_ERROR_SSL:
+		return ("SSL_ERROR_SSL");
+	default:
+		UNREACHABLE();
+	}
+}
+
+ISC_ATTR_UNUSED static const char *
+state2str(int state) {
+	switch (state) {
+	case TLS_INIT:
+		return ("TLS_INIT");
+	case TLS_HANDSHAKE:
+		return ("TLS_HANDSHAKE");
+	case TLS_IO:
+		return ("TLS_IO");
+	case TLS_CLOSED:
+		return ("TLS_CLOSED");
+	default:
+		UNREACHABLE();
+	}
+}
+#endif /* ISC_NETMGR_TRACE */
+
 static isc_result_t
 tls_error_to_result(const int tls_err, const int tls_state, isc_tls_t *tls) {
 	switch (tls_err) {
@@ -62,6 +98,9 @@ tls_error_to_result(const int tls_err, const int tls_state, isc_tls_t *tls) {
 	}
 }
 
+static void
+tls_read_start(isc_nmsocket_t *restrict sock);
+
 static void
 tls_read_stop(isc_nmsocket_t *sock);
 
@@ -76,9 +115,6 @@ static void
 tls_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
 	   void *cbarg);
 
-static void
-tls_close_direct(void *arg);
-
 static void
 async_tls_do_bio(isc_nmsocket_t *sock);
 
@@ -117,9 +153,7 @@ inactive(isc_nmsocket_t *sock) {
 static void
 tls_call_connect_cb(isc_nmsocket_t *sock, isc_nmhandle_t *handle,
 		    const isc_result_t result) {
-	if (sock->connect_cb == NULL) {
-		return;
-	}
+	INSIST(sock->connect_cb != NULL);
 	sock->connect_cb(handle, result, sock->connect_cbarg);
 	if (result != ISC_R_SUCCESS) {
 		isc__nmsocket_clearcb(handle->sock);
@@ -194,7 +228,12 @@ tls_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 		}
 	}
 
-	if (finish && eresult == ISC_R_SUCCESS && tlssock->reading) {
+	if (finish) {
+		/*
+		 * If wrapping up, call tls_failed_read() - it will care of
+		 * socket de-initialisation and calling the read callback, if
+		 * necessary.
+		 */
 		tls_failed_read_cb(tlssock, ISC_R_EOF);
 	} else if (eresult == ISC_R_SUCCESS) {
 		tls_do_bio(tlssock, NULL, NULL, false);
@@ -218,12 +257,11 @@ tls_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 }
 
 static void
-tls_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result) {
-	bool destroy = true;
-
+tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(result != ISC_R_SUCCESS);
 
+	/* This is TLS counterpart of isc__nm_failed_connect_cb() */
 	if (!sock->tlsstream.server &&
 	    (sock->tlsstream.state == TLS_INIT ||
 	     sock->tlsstream.state == TLS_HANDSHAKE) &&
@@ -235,37 +273,60 @@ tls_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result) {
 		tls_call_connect_cb(sock, handle, result);
 		isc__nmsocket_clearcb(sock);
 		isc_nmhandle_detach(&handle);
-	} else if (sock->reading && sock->recv_cb != NULL &&
-		   sock->statichandle != NULL &&
-		   (sock->recv_read || result == ISC_R_TIMEDOUT))
-	{
-		sock->recv_read = false;
-		sock->recv_cb(sock->statichandle, result, NULL,
-			      sock->recv_cbarg);
-		if (result == ISC_R_TIMEDOUT &&
-		    (sock->outerhandle == NULL ||
-		     isc__nmsocket_timer_running(sock->outerhandle->sock)))
-		{
-			destroy = false;
-		}
+		goto destroy;
 	}
 
-	if (destroy) {
-		isc__nmsocket_prep_destroy(sock);
+	isc__nmsocket_timer_stop(sock);
+
+	/* Nobody is reading from the socket yet */
+	if (sock->statichandle == NULL) {
+		goto destroy;
 	}
+
+	/* This is TLS counterpart of isc__nmsocket_readtimeout_cb() */
+	if (sock->client && result == ISC_R_TIMEDOUT) {
+		INSIST(sock->statichandle != NULL);
+
+		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, false);
+		}
+
+		if (isc__nmsocket_timer_running(sock)) {
+			/* Timer was restarted, bail-out */
+			return;
+		}
+
+		isc__nmsocket_clearcb(sock);
+
+		goto destroy;
+	}
+
+	/*
+	 * We don't need to check for .nsending, as the callbacks will be
+	 * cleared at the time the tls_senddone() tries to call it for the
+	 * second time.
+	 */
+
+	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, false);
+	}
+
+destroy:
+	isc__nmsocket_prep_destroy(sock);
 }
 
 void
 isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
-			   bool async) {
-	UNUSED(async);
-
+			   bool async ISC_ATTR_UNUSED) {
 	if (!inactive(sock) && sock->tlsstream.state == TLS_IO) {
 		tls_do_bio(sock, NULL, NULL, true);
-	} else if (sock->recv_read) {
-		tls_read_stop(sock);
-		tls_failed_read_cb(sock, result);
+		return;
 	}
+
+	tls_failed_read_cb(sock, result);
 }
 
 static void
@@ -459,13 +520,10 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
 	int rv = 0;
 	size_t len = 0;
 	int saved_errno = 0;
-	bool was_reading;
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_tid());
 
-	was_reading = sock->reading;
-
 	/*
 	 * Clear the TLS error queue so that SSL_get_error() and SSL I/O
 	 * routine calls will not get affected by prior error statuses.
@@ -592,7 +650,7 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
 
 		/* Decrypt and pass data from network to client */
 		if (sock->tlsstream.state >= TLS_IO && sock->recv_cb != NULL &&
-		    was_reading && sock->statichandle != NULL && !finish)
+		    sock->statichandle != NULL && sock->reading && !finish)
 		{
 			bool was_new_data = false;
 			uint8_t recv_buf[TLS_BUF_SIZE];
@@ -693,7 +751,6 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
 
 	pending = tls_process_outgoing(sock, finish, send_data);
 	if (pending > 0 && tls_status != SSL_ERROR_SSL) {
-		/* We'll continue in tls_senddone */
 		return;
 	}
 
@@ -717,19 +774,20 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
 		{
 			return;
 		} else if (sock->reading == false &&
-			   sock->tlsstream.state > TLS_HANDSHAKE)
+			   sock->tlsstream.state == TLS_HANDSHAKE)
 		{
-			/* We need to read data when doing handshake even if
-			 * 'sock->reading == false' */
+			/*
+			 * We need to read data when doing handshake even if
+			 * 'sock->reading == false'. It will be stopped when
+			 * handshake is completed.
+			 */
+			tls_read_start(sock);
+			return;
+		} else if (sock->reading == false) {
 			return;
 		}
 
-		INSIST(VALID_NMHANDLE(sock->outerhandle));
-
-		isc_nm_read(sock->outerhandle, tls_readcb, sock);
-		if (!sock->manual_read_timer) {
-			isc__nmsocket_timer_start(sock);
-		}
+		tls_read_start(sock);
 		return;
 	default:
 		result = tls_error_to_result(tls_status, sock->tlsstream.state,
@@ -766,6 +824,7 @@ tls_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
 		return;
 	}
 
+	REQUIRE(handle == tlssock->outerhandle);
 	tls_do_bio(tlssock, region, NULL, false);
 }
 
@@ -1022,16 +1081,29 @@ isc__nm_tls_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 
 	sock->recv_cb = cb;
 	sock->recv_cbarg = cbarg;
-	sock->recv_read = true;
 	sock->reading = true;
 
 	async_tls_do_bio(sock);
 }
 
 static void
-tls_read_stop(isc_nmsocket_t *sock) {
-	sock->reading = false;
+tls_read_start(isc_nmsocket_t *restrict sock) {
+	if (sock->tlsstream.reading) {
+		return;
+	}
+	sock->tlsstream.reading = true;
 
+	INSIST(VALID_NMHANDLE(sock->outerhandle));
+
+	isc_nm_read(sock->outerhandle, tls_readcb, sock);
+	if (!sock->manual_read_timer) {
+		isc__nmsocket_timer_start(sock);
+	}
+}
+
+static void
+tls_read_stop(isc_nmsocket_t *sock) {
+	sock->tlsstream.reading = false;
 	if (sock->outerhandle != NULL) {
 		isc_nm_read_stop(sock->outerhandle);
 	}
@@ -1042,22 +1114,29 @@ isc__nm_tls_read_stop(isc_nmhandle_t *handle) {
 	REQUIRE(VALID_NMHANDLE(handle));
 	REQUIRE(VALID_NMSOCK(handle->sock));
 
+	handle->sock->reading = false;
+
 	tls_read_stop(handle->sock);
 }
 
-static void
-tls_close_direct(void *arg) {
-	isc_nmsocket_t *sock = arg;
+void
+isc__nm_tls_close(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
+	REQUIRE(sock->type == isc_nm_tlssocket);
+	REQUIRE(!sock->closing);
 	REQUIRE(sock->tid == isc_tid());
+	REQUIRE(!sock->closed);
+	REQUIRE(!sock->closing);
+
+	sock->closing = true;
+
 	/*
 	 * At this point we're certain that there are no
 	 * external references, we can close everything.
 	 */
+	tls_read_stop(sock);
 	if (sock->outerhandle != NULL) {
-		sock->reading = false;
 		isc_nm_read_stop(sock->outerhandle);
-
 		isc_nmhandle_close(sock->outerhandle);
 		isc_nmhandle_detach(&sock->outerhandle);
 	}
@@ -1072,22 +1151,6 @@ tls_close_direct(void *arg) {
 	sock->tlsstream.state = TLS_CLOSED;
 }
 
-void
-isc__nm_tls_close(isc_nmsocket_t *sock) {
-	REQUIRE(VALID_NMSOCK(sock));
-	REQUIRE(sock->type == isc_nm_tlssocket);
-	REQUIRE(!sock->closing);
-
-	sock->closing = true;
-
-	if (sock->tid == isc_tid()) {
-		/* no point in attempting to make the call asynchronous */
-		tls_close_direct(sock);
-	} else {
-		isc_async_run(sock->worker->loop, tls_close_direct, sock);
-	}
-}
-
 void
 isc__nm_tls_stoplistening(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
@@ -1103,35 +1166,36 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t result, void *cbarg);
 
 void
 isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
-		  isc_nm_cb_t cb, void *cbarg, isc_tlsctx_t *ctx,
+		  isc_nm_cb_t connect_cb, void *connect_cbarg,
+		  isc_tlsctx_t *ctx,
 		  isc_tlsctx_client_session_cache_t *client_sess_cache,
 		  unsigned int timeout) {
-	isc_nmsocket_t *nsock = NULL;
+	isc_nmsocket_t *sock = NULL;
 	isc__networker_t *worker = &mgr->workers[isc_tid()];
 
 	REQUIRE(VALID_NM(mgr));
 
 	if (isc__nm_closing(worker)) {
-		cb(NULL, ISC_R_SHUTTINGDOWN, cbarg);
+		connect_cb(NULL, ISC_R_SHUTTINGDOWN, connect_cbarg);
 		return;
 	}
 
-	nsock = isc_mem_get(worker->mctx, sizeof(*nsock));
-	isc__nmsocket_init(nsock, worker, isc_nm_tlssocket, local, NULL);
-	nsock->connect_cb = cb;
-	nsock->connect_cbarg = cbarg;
-	nsock->connect_timeout = timeout;
-	isc_tlsctx_attach(ctx, &nsock->tlsstream.ctx);
-	nsock->client = true;
+	sock = isc_mem_get(worker->mctx, sizeof(*sock));
+	isc__nmsocket_init(sock, worker, isc_nm_tlssocket, local, NULL);
+	sock->connect_cb = connect_cb;
+	sock->connect_cbarg = connect_cbarg;
+	sock->connect_timeout = timeout;
+	isc_tlsctx_attach(ctx, &sock->tlsstream.ctx);
+	sock->client = true;
 	if (client_sess_cache != NULL) {
 		INSIST(isc_tlsctx_client_session_cache_getctx(
 			       client_sess_cache) == ctx);
 		isc_tlsctx_client_session_cache_attach(
-			client_sess_cache, &nsock->tlsstream.client_sess_cache);
+			client_sess_cache, &sock->tlsstream.client_sess_cache);
 	}
 
-	isc_nm_tcpconnect(mgr, local, peer, tcp_connected, nsock,
-			  nsock->connect_timeout);
+	isc_nm_tcpconnect(mgr, local, peer, tcp_connected, sock,
+			  sock->connect_timeout);
 }
 
 static void

lib/isc/netmgr/udp.c

@@ -167,7 +167,6 @@ start_udp_child_job(void *arg) {
 		goto done;
 	}
 
-	sock->reading = true;
 done:
 	result = isc_uverr2result(r);
 
@@ -569,7 +568,7 @@ isc__nm_udp_read_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 	req->uvbuf.base = buf->base;
 	req->uvbuf.len = nrecv;
 
-	sock->recv_read = false;
+	sock->reading = false;
 
 	/*
 	 * The client isc_nm_read() expects just a single message, so we need to
@@ -579,6 +578,7 @@ isc__nm_udp_read_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 	if (sock->client) {
 		isc__nmsocket_timer_stop(sock);
 		isc__nm_stop_reading(sock);
+		isc__nmsocket_clearcb(sock);
 	}
 
 	REQUIRE(!sock->processing);
@@ -844,28 +844,6 @@ isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
 	REQUIRE(result != ISC_R_SUCCESS);
 	REQUIRE(sock->tid == isc_tid());
 
-	if (sock->client) {
-		isc__nmsocket_timer_stop(sock);
-		isc__nm_stop_reading(sock);
-
-		/* Nobody expects the callback if isc_nm_read() wasn't called */
-		if (!sock->recv_read) {
-			goto destroy;
-		}
-
-		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, async);
-		}
-
-		sock->recv_read = false;
-
-	destroy:
-		isc__nmsocket_prep_destroy(sock);
-		return;
-	}
-
 	/*
 	 * For UDP server socket, we don't have child socket via
 	 * "accept", so we:
@@ -873,14 +851,26 @@ isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
 	 * - we don't clear the callbacks
 	 * - we don't destroy it (only stoplistening could do that)
 	 */
-	if (!sock->recv_read) {
-		return;
-	}
-	sock->recv_read = false;
 
-	if (sock->recv_cb != NULL) {
-		isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
-		isc__nm_readcb(sock, req, result, async);
+	if (sock->client) {
+		isc__nmsocket_timer_stop(sock);
+		isc__nm_stop_reading(sock);
+	}
+
+	/* Nobody expects the callback if isc_nm_read() wasn't called */
+	if (sock->reading) {
+		sock->reading = false;
+
+		if (sock->recv_cb != NULL) {
+			isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
+			isc__nm_readcb(sock, req, result, async);
+		}
+	}
+
+	if (sock->client) {
+		isc__nmsocket_clearcb(sock);
+		isc__nmsocket_prep_destroy(sock);
+		return;
 	}
 }
 
@@ -896,7 +886,6 @@ isc__nm_udp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->type == isc_nm_udpsocket);
 	REQUIRE(sock->statichandle == handle);
-	REQUIRE(!sock->recv_read);
 	REQUIRE(sock->tid == isc_tid());
 
 	/*
@@ -905,7 +894,7 @@ isc__nm_udp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 	 */
 	sock->recv_cb = cb;
 	sock->recv_cbarg = cbarg;
-	sock->recv_read = true;
+	sock->reading = true;
 
 	if (isc__nm_closing(sock->worker)) {
 		result = ISC_R_SHUTTINGDOWN;
@@ -985,14 +974,10 @@ isc__nm_udp_close(isc_nmsocket_t *sock) {
 
 void
 isc__nm_udp_shutdown(isc_nmsocket_t *sock) {
-	isc__networker_t *worker = NULL;
-
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_tid());
 	REQUIRE(sock->type == isc_nm_udpsocket);
 
-	worker = sock->worker;
-
 	/*
 	 * If the socket is active, mark it inactive and
 	 * continue. If it isn't active, stop now.
@@ -1011,11 +996,7 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock) {
 	 * interested in the callback.
 	 */
 	if (sock->statichandle != NULL) {
-		if (isc__nm_closing(worker)) {
-			isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false);
-		} else {
-			isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
-		}
+		isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false);
 		return;
 	}
 
@@ -1030,28 +1011,3 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock) {
 		isc__nmsocket_prep_destroy(sock->parent);
 	}
 }
-
-static void
-udp_cancelread_cb(void *arg) {
-	isc_nmsocket_t *sock = arg;
-
-	REQUIRE(VALID_NMSOCK(sock));
-	REQUIRE(sock->tid == isc_tid());
-	REQUIRE(sock->client);
-
-	isc__nm_failed_read_cb(sock, ISC_R_EOF, false);
-	isc__nmsocket_detach(&sock);
-}
-
-void
-isc__nm_udp_cancelread(isc_nmhandle_t *handle) {
-	REQUIRE(VALID_NMHANDLE(handle));
-
-	isc_nmsocket_t *sock = handle->sock;
-
-	REQUIRE(VALID_NMSOCK(sock));
-	REQUIRE(sock->type == isc_nm_udpsocket);
-
-	isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL });
-	isc_async_run(sock->worker->loop, udp_cancelread_cb, sock);
-}

lib/isccc/ccmsg.c

@@ -46,42 +46,30 @@ static void
 recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 	  void *arg) {
 	isccc_ccmsg_t *ccmsg = arg;
-	size_t size;
 
-	INSIST(VALID_CCMSG(ccmsg));
+	REQUIRE(VALID_CCMSG(ccmsg));
 
-	switch (eresult) {
-	case ISC_R_SHUTTINGDOWN:
-	case ISC_R_CANCELED:
-	case ISC_R_EOF:
-		ccmsg->result = eresult;
-		goto done;
-	case ISC_R_SUCCESS:
-		if (region == NULL) {
-			ccmsg->result = ISC_R_EOF;
-			goto done;
-		}
-		ccmsg->result = ISC_R_SUCCESS;
-		break;
-	default:
-		ccmsg->result = eresult;
+	REQUIRE(handle == ccmsg->handle);
+	if (eresult != ISC_R_SUCCESS) {
 		goto done;
 	}
 
+	REQUIRE(region != NULL);
+
 	if (!ccmsg->length_received) {
 		if (region->length < sizeof(uint32_t)) {
-			ccmsg->result = ISC_R_UNEXPECTEDEND;
+			eresult = ISC_R_UNEXPECTEDEND;
 			goto done;
 		}
 
 		ccmsg->size = ntohl(*(uint32_t *)region->base);
 
 		if (ccmsg->size == 0) {
-			ccmsg->result = ISC_R_UNEXPECTEDEND;
+			eresult = ISC_R_UNEXPECTEDEND;
 			goto done;
 		}
 		if (ccmsg->size > ccmsg->maxsize) {
-			ccmsg->result = ISC_R_RANGE;
+			eresult = ISC_R_RANGE;
 			goto done;
 		}
 
@@ -100,13 +88,12 @@ recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 
 	/* We have some data in the buffer, read it */
 
-	size = ISC_MIN(isc_buffer_availablelength(ccmsg->buffer),
-		       region->length);
+	size_t size = ISC_MIN(isc_buffer_availablelength(ccmsg->buffer),
+			      region->length);
 	isc_buffer_putmem(ccmsg->buffer, region->base, size);
 	isc_region_consume(region, size);
 
 	if (isc_buffer_usedlength(ccmsg->buffer) == ccmsg->size) {
-		ccmsg->result = ISC_R_SUCCESS;
 		goto done;
 	}
 
@@ -115,7 +102,12 @@ recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 
 done:
 	isc_nm_read_stop(handle);
-	ccmsg->cb(handle, ccmsg->result, ccmsg->cbarg);
+	if (ccmsg->reading) {
+		ccmsg->reading = false;
+		ccmsg->recv_cb(handle, eresult, ccmsg->recv_cbarg);
+	}
+
+	return;
 }
 
 void
@@ -129,9 +121,9 @@ isccc_ccmsg_init(isc_mem_t *mctx, isc_nmhandle_t *handle,
 		.magic = CCMSG_MAGIC,
 		.maxsize = 0xffffffffU, /* Largest message possible. */
 		.mctx = mctx,
-		.handle = handle,
-		.result = ISC_R_UNEXPECTED /* None yet. */
 	};
+
+	isc_nmhandle_attach(handle, &ccmsg->handle);
 }
 
 void
@@ -149,23 +141,42 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg) {
 		isc_buffer_free(&ccmsg->buffer);
 	}
 
-	ccmsg->cb = cb;
-	ccmsg->cbarg = cbarg;
-	ccmsg->result = ISC_R_UNEXPECTED; /* unknown right now */
+	ccmsg->recv_cb = cb;
+	ccmsg->recv_cbarg = cbarg;
 	ccmsg->length_received = false;
 
-	isc_nm_read(ccmsg->handle, recv_data, ccmsg);
 	ccmsg->reading = true;
+	isc_nm_read(ccmsg->handle, recv_data, ccmsg);
+}
+
+static void
+ccmsg_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
+	isccc_ccmsg_t *ccmsg = arg;
+
+	REQUIRE(VALID_CCMSG(ccmsg));
+
+	INSIST(ccmsg->send_cb != NULL);
+	ccmsg->send_cb(handle, eresult, ccmsg->send_cbarg);
+	ccmsg->send_cb = NULL;
+
+	if (eresult != ISC_R_SUCCESS && ccmsg->reading) {
+		recv_data(handle, eresult, NULL, ccmsg);
+	}
+
+	isc_nmhandle_detach(&handle);
 }
 
 void
-isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg) {
+isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region,
+			isc_nm_cb_t cb, void *cbarg) {
 	REQUIRE(VALID_CCMSG(ccmsg));
+	REQUIRE(ccmsg->send_cb == NULL);
 
-	if (ccmsg->reading) {
-		isc_nm_read_stop(ccmsg->handle);
-		ccmsg->reading = false;
-	}
+	ccmsg->send_cb = cb;
+	ccmsg->send_cbarg = cbarg;
+
+	isc_nmhandle_ref(ccmsg->handle);
+	isc_nm_send(ccmsg->handle, region, ccmsg_senddone, ccmsg);
 }
 
 void
@@ -177,4 +188,16 @@ isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) {
 	if (ccmsg->buffer != NULL) {
 		isc_buffer_free(&ccmsg->buffer);
 	}
+	if (ccmsg->handle != NULL) {
+		isc_nmhandle_close(ccmsg->handle);
+		isc_nmhandle_detach(&ccmsg->handle);
+	}
+}
+
+void
+isccc_ccmsg_toregion(isccc_ccmsg_t *ccmsg, isccc_region_t *ccregion) {
+	REQUIRE(VALID_CCMSG(ccmsg));
+
+	ccregion->rstart = isc_buffer_base(ccmsg->buffer);
+	ccregion->rend = isc_buffer_used(ccmsg->buffer);
 }

lib/isccc/include/isccc/ccmsg.h

@@ -38,6 +38,8 @@
 #include <isc/netmgr.h>
 #include <isc/sockaddr.h>
 
+#include <isccc/types.h>
+
 /*% ISCCC Message Structure */
 typedef struct isccc_ccmsg {
 	/* private (don't touch!) */
@@ -48,11 +50,11 @@ typedef struct isccc_ccmsg {
 	unsigned int	maxsize;
 	isc_mem_t      *mctx;
 	isc_nmhandle_t *handle;
-	isc_nm_cb_t	cb;
-	void	       *cbarg;
+	isc_nm_cb_t	recv_cb;
+	void	       *recv_cbarg;
+	isc_nm_cb_t	send_cb;
+	void	       *send_cbarg;
 	bool		reading;
-	/* public (read-only) */
-	isc_result_t result;
 } isccc_ccmsg_t;
 
 ISC_LANG_BEGINDECLS
@@ -109,14 +111,12 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg);
  */
 
 void
-isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg);
+isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region,
+			isc_nm_cb_t cb, void *cbarg);
 /*%
- * Cancel a readmessage() call.  The event will still be posted with a
- * CANCELED result code.
+ * Sends region over the command channel message.
  *
- * Requires:
- *
- *\li	"ccmsg" be valid.
+ * CAVEAT: Only a single send message can be scheduled at the time.
  */
 
 void
@@ -134,4 +134,7 @@ isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg);
  *	sockets, etc.
  */
 
+void
+isccc_ccmsg_toregion(isccc_ccmsg_t *ccmsg, isccc_region_t *ccregion);
+
 ISC_LANG_ENDDECLS

tests/dns/dispatch_test.c

@@ -322,13 +322,14 @@ server_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 
 static void
 nameserver(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
-	   void *cbarg) {
+	   void *cbarg ISC_ATTR_UNUSED) {
 	isc_region_t response1, response2;
 	static unsigned char buf1[16];
 	static unsigned char buf2[16];
 
-	UNUSED(eresult);
-	UNUSED(cbarg);
+	if (eresult != ISC_R_SUCCESS) {
+		return;
+	}
 
 	memmove(buf1, region->base, 12);
 	memset(buf1 + 12, 0, 4);

tests/isc/Makefile.am

@@ -113,6 +113,7 @@ tcp_test_SOURCES =	\
 	tcp_test.c	\
 	netmgr_common.h \
 	netmgr_common.c \
+	stream_shutdown.c \
 	uv_wrap.h
 
 tcpdns_test_CPPFLAGS =	\
@@ -141,6 +142,7 @@ tls_test_SOURCES =	\
 	tls_test.c	\
 	netmgr_common.h \
 	netmgr_common.c \
+	stream_shutdown.c \
 	uv_wrap.h
 
 tlsdns_test_CPPFLAGS =	\

tests/isc/netmgr_common.c

@@ -108,24 +108,13 @@ isc_nm_recv_cb_t connect_readcb = NULL;
 
 int
 setup_netmgr_test(void **state) {
-	char *env_workers = getenv("ISC_TASK_WORKERS");
-	size_t nworkers;
-
 	tcp_connect_addr = (isc_sockaddr_t){ .length = 0 };
 	isc_sockaddr_fromin6(&tcp_connect_addr, &in6addr_loopback, 0);
 
 	tcp_listen_addr = (isc_sockaddr_t){ .length = 0 };
 	isc_sockaddr_fromin6(&tcp_listen_addr, &in6addr_loopback, stream_port);
 
-	if (env_workers != NULL) {
-		workers = atoi(env_workers);
-	} else {
-		workers = isc_os_ncpus();
-	}
-	INSIST(workers > 0);
-	nworkers = ISC_MAX(ISC_MIN(workers, 32), 1);
-
-	esends = NSENDS * nworkers;
+	esends = NSENDS * workers;
 
 	atomic_store(&nsends, esends);
 
@@ -227,7 +216,7 @@ teardown_netmgr_test(void **state ISC_ATTR_UNUSED) {
 	return (0);
 }
 
-static void
+void
 stop_listening(void *arg ISC_ATTR_UNUSED) {
 	isc_nm_stoplistening(listen_sock);
 	isc_nmsocket_close(&listen_sock);
@@ -237,24 +226,23 @@ stop_listening(void *arg ISC_ATTR_UNUSED) {
 /* Callbacks */
 
 void
-noop_recv_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
-	     void *cbarg) {
-	UNUSED(handle);
-	UNUSED(eresult);
-	UNUSED(region);
-	UNUSED(cbarg);
+noop_recv_cb(isc_nmhandle_t *handle ISC_ATTR_UNUSED,
+	     isc_result_t eresult ISC_ATTR_UNUSED,
+	     isc_region_t *region ISC_ATTR_UNUSED,
+	     void *cbarg ISC_ATTR_UNUSED) {
+	F();
 }
 
-unsigned int
-noop_accept_cb(isc_nmhandle_t *handle, unsigned int result, void *cbarg) {
-	UNUSED(handle);
-	UNUSED(cbarg);
+isc_result_t
+noop_accept_cb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, unsigned int eresult,
+	       void *cbarg ISC_ATTR_UNUSED) {
+	F();
 
-	if (result == ISC_R_SUCCESS) {
+	if (eresult == ISC_R_SUCCESS) {
 		(void)atomic_fetch_add(&saccepts, 1);
 	}
 
-	return (0);
+	return (ISC_R_SUCCESS);
 }
 
 void
@@ -278,10 +266,8 @@ connect_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 	case ISC_R_SHUTTINGDOWN:
 	case ISC_R_CANCELED:
 	case ISC_R_CONNECTIONRESET:
-		/* Send failed, we need to stop reading too */
-		if (stream) {
-			isc_nm_read_stop(handle);
-		} else {
+		/* Abort */
+		if (!stream) {
 			isc_nm_cancelread(handle);
 		}
 		break;
@@ -337,6 +323,10 @@ connect_read_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 			return;
 		}
 
+		/* This will initiate one more read callback */
+		if (stream) {
+			isc_nmhandle_close(handle);
+		}
 		break;
 	case ISC_R_TIMEDOUT:
 	case ISC_R_EOF:
@@ -352,10 +342,6 @@ connect_read_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 	}
 
 	isc_refcount_decrement(&active_creads);
-
-	if (stream) {
-		isc_nm_read_stop(handle);
-	}
 	isc_nmhandle_detach(&handle);
 }
 
@@ -399,8 +385,6 @@ listen_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 
 	F();
 
-	isc_refcount_decrement(&active_ssends);
-
 	switch (eresult) {
 	case ISC_R_CANCELED:
 	case ISC_R_CONNECTIONRESET:
@@ -418,6 +402,7 @@ listen_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 		assert_int_equal(eresult, ISC_R_SUCCESS);
 	}
 
+	isc_refcount_decrement(&active_ssends);
 	isc_nmhandle_detach(&sendhandle);
 }
 
@@ -431,11 +416,6 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 	F();
 
 	switch (eresult) {
-	case ISC_R_CANCELED:
-	case ISC_R_CONNECTIONRESET:
-	case ISC_R_EOF:
-	case ISC_R_SHUTTINGDOWN:
-		break;
 	case ISC_R_SUCCESS:
 		memmove(&magic, region->base, sizeof(magic));
 		assert_true(magic == send_magic);
@@ -449,18 +429,21 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 		memmove(&magic, region->base, sizeof(magic));
 		assert_true(magic == send_magic);
 
-		if (magic == send_magic) {
-			if (!noanswer) {
-				isc_nmhandle_t *sendhandle = NULL;
-				isc_nmhandle_attach(handle, &sendhandle);
-				isc_refcount_increment0(&active_ssends);
-				isc_nmhandle_setwritetimeout(sendhandle,
-							     T_IDLE);
-				isc_nm_send(sendhandle, &send_msg,
-					    listen_send_cb, cbarg);
-			}
-			return;
+		if (!noanswer) {
+			/* Answer and continue to listen */
+			isc_nmhandle_t *sendhandle = NULL;
+			isc_nmhandle_attach(handle, &sendhandle);
+			isc_refcount_increment0(&active_ssends);
+			isc_nmhandle_setwritetimeout(sendhandle, T_IDLE);
+			isc_nm_send(sendhandle, &send_msg, listen_send_cb,
+				    cbarg);
 		}
+		/* Continue to listen */
+		return;
+	case ISC_R_CANCELED:
+	case ISC_R_CONNECTIONRESET:
+	case ISC_R_EOF:
+	case ISC_R_SHUTTINGDOWN:
 		break;
 	default:
 		fprintf(stderr, "%s(%p, %s, %p)\n", __func__, handle,
@@ -469,7 +452,6 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 	}
 
 	isc_refcount_decrement(&active_sreads);
-
 	isc_nmhandle_detach(&handle);
 }
 
@@ -488,6 +470,9 @@ listen_accept_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 		do_saccepts_shutdown(loopmgr);
 	}
 
+	isc_nmhandle_attach(handle, &(isc_nmhandle_t *){ NULL });
+	isc_refcount_increment0(&active_sreads);
+
 	return (eresult);
 }
 
@@ -508,6 +493,7 @@ stream_accept_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 	}
 
 	isc_refcount_increment0(&active_sreads);
+
 	isc_nmhandle_attach(handle, &readhandle);
 	isc_nm_read(handle, listen_read_cb, readhandle);
 
@@ -670,11 +656,12 @@ noresponse_readcb(isc_nmhandle_t *handle, isc_result_t eresult,
 	UNUSED(region);
 	UNUSED(cbarg);
 
+	F();
+
 	assert_true(eresult == ISC_R_CANCELED ||
 		    eresult == ISC_R_CONNECTIONRESET || eresult == ISC_R_EOF);
 
 	isc_refcount_decrement(&active_creads);
-
 	isc_nmhandle_detach(&handle);
 
 	isc_loopmgr_shutdown(loopmgr);
@@ -685,6 +672,8 @@ noresponse_sendcb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 	UNUSED(cbarg);
 	UNUSED(eresult);
 
+	F();
+
 	assert_non_null(handle);
 	atomic_fetch_add(&csends, 1);
 	isc_nmhandle_detach(&handle);
@@ -697,8 +686,6 @@ noresponse_connectcb(isc_nmhandle_t *handle, isc_result_t eresult,
 	isc_nmhandle_t *readhandle = NULL;
 	isc_nmhandle_t *sendhandle = NULL;
 
-	UNUSED(handle);
-
 	F();
 
 	isc_refcount_decrement(&active_cconnects);

tests/isc/netmgr_common.h

@@ -201,15 +201,15 @@ extern isc_nm_recv_cb_t connect_readcb;
 	fprintf(stderr, "%s:%s:%d:%s = %" PRId64 "\n", __func__, __FILE__, \
 		__LINE__, #v, atomic_load(&v))
 #define P(v) fprintf(stderr, #v " = %" PRId64 "\n", v)
-#define F()                                                   \
-	fprintf(stderr, "%s(%p, %s, %p)\n", __func__, handle, \
+#define F()                                                                 \
+	fprintf(stderr, "%u:%s(%p, %s, %p)\n", isc_tid(), __func__, handle, \
 		isc_result_totext(eresult), cbarg)
 
-#define isc_loopmgr_shutdown(loopmgr)                                  \
-	{                                                              \
-		fprintf(stderr, "%s:%s:%d:isc_loopmgr_shutdown(%p)\n", \
-			__func__, __FILE__, __LINE__, loopmgr);        \
-		isc_loopmgr_shutdown(loopmgr);                         \
+#define isc_loopmgr_shutdown(loopmgr)                                      \
+	{                                                                  \
+		fprintf(stderr, "%u:%s:%s:%d:isc_loopmgr_shutdown(%p)\n",  \
+			isc_tid(), __func__, __FILE__, __LINE__, loopmgr); \
+		isc_loopmgr_shutdown(loopmgr);                             \
 	}
 #else
 #define X(v)
@@ -234,8 +234,9 @@ void
 noop_recv_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 	     void *cbarg);
 
-unsigned int
-noop_accept_cb(isc_nmhandle_t *handle, unsigned int result, void *cbarg);
+isc_result_t
+noop_accept_cb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, unsigned int result,
+	       void *cbarg ISC_ATTR_UNUSED);
 
 void
 connect_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg);
@@ -325,3 +326,20 @@ int
 stream_recv_send_teardown(void **state ISC_ATTR_UNUSED);
 void
 stream_recv_send_connect(void *arg);
+
+int
+stream_shutdownconnect_setup(void **state ISC_ATTR_UNUSED);
+void
+stream_shutdownconnect(void **state ISC_ATTR_UNUSED);
+int
+stream_shutdownconnect_teardown(void **state ISC_ATTR_UNUSED);
+
+int
+stream_shutdownread_setup(void **state ISC_ATTR_UNUSED);
+void
+stream_shutdownread(void **state ISC_ATTR_UNUSED);
+int
+stream_shutdownread_teardown(void **state ISC_ATTR_UNUSED);
+
+void
+stop_listening(void *arg ISC_ATTR_UNUSED);

tests/isc/stream_shutdown.c

@@ -0,0 +1,171 @@
+/*
+ * 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 <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+/*
+ * As a workaround, include an OpenSSL header file before including cmocka.h,
+ * because OpenSSL 3.1.0 uses __attribute__(malloc), conflicting with a
+ * redefined malloc in cmocka.h.
+ */
+#include <openssl/err.h>
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include "netmgr_common.h"
+
+#include <tests/isc.h>
+
+/*
+ * FIXME: This really needs two network managers, so there's predictable result
+ * when shuttingdown the netmgr - right now there's a race whether the listening
+ * or connecting sockets gets shutdown first
+ */
+
+static void
+shutdownconnect_connectcb(isc_nmhandle_t *handle, isc_result_t eresult,
+			  void *cbarg) {
+	F();
+
+	assert_non_null(handle);
+	assert_int_equal(eresult, ISC_R_SHUTTINGDOWN);
+	assert_null(cbarg);
+
+	isc_refcount_decrement(&active_cconnects);
+
+	atomic_fetch_add(&cconnects, 1);
+}
+
+int
+stream_shutdownconnect_setup(void **state ISC_ATTR_UNUSED) {
+	int r = setup_netmgr_test(state);
+	return (r);
+}
+
+void
+stream_shutdownconnect(void **state ISC_ATTR_UNUSED) {
+	isc_result_t result = stream_listen(stream_accept_cb, NULL, 128, NULL,
+					    &listen_sock);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_loop_teardown(mainloop, stop_listening, listen_sock);
+
+	/* Schedule the shutdown before the connect */
+	isc_loopmgr_shutdown(loopmgr);
+
+	stream_connect(shutdownconnect_connectcb, NULL, T_CONNECT);
+}
+
+int
+stream_shutdownconnect_teardown(void **state ISC_ATTR_UNUSED) {
+	X(cconnects);
+	X(csends);
+	X(creads);
+
+	atomic_assert_int_eq(cconnects, 1);
+	atomic_assert_int_eq(csends, 0);
+	atomic_assert_int_eq(creads, 0);
+
+	return (teardown_netmgr_test(state));
+}
+
+/* Issue the shutdown before reading */
+
+static void
+shutdownread_readcb(isc_nmhandle_t *handle, isc_result_t eresult,
+		    isc_region_t *region, void *cbarg) {
+	F();
+	assert_non_null(handle);
+	assert_true(eresult == ISC_R_SHUTTINGDOWN ||
+		    eresult == ISC_R_CONNECTIONRESET || eresult == ISC_R_EOF);
+	assert_non_null(region);
+	assert_null(cbarg);
+
+	atomic_fetch_add(&creads, 1);
+	isc_nmhandle_detach(&handle);
+	isc_refcount_decrement(&active_creads);
+}
+
+static void
+shutdownread_sendcb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
+	F();
+	assert_non_null(handle);
+	assert_true(eresult == ISC_R_SUCCESS || eresult == ISC_R_SHUTTINGDOWN ||
+		    eresult == ISC_R_CONNECTIONRESET || eresult == ISC_R_EOF);
+	assert_null(cbarg);
+
+	atomic_fetch_add(&csends, 1);
+
+	isc_nmhandle_detach(&handle);
+	isc_refcount_decrement(&active_csends);
+}
+
+static void
+shutdownread_connectcb(isc_nmhandle_t *handle, isc_result_t eresult,
+		       void *cbarg) {
+	F();
+
+	assert_non_null(handle);
+	assert_int_equal(eresult, ISC_R_SUCCESS);
+	assert_null(cbarg);
+
+	isc_refcount_decrement(&active_cconnects);
+
+	atomic_fetch_add(&cconnects, 1);
+
+	/* Schedule the shutdown before read and send */
+	isc_loopmgr_shutdown(loopmgr);
+
+	isc_refcount_increment0(&active_creads);
+	isc_nmhandle_ref(handle);
+	isc_nm_read(handle, shutdownread_readcb, cbarg);
+
+	isc_refcount_increment0(&active_csends);
+	isc_nmhandle_ref(handle);
+	isc_nm_send(handle, (isc_region_t *)&send_msg, shutdownread_sendcb,
+		    cbarg);
+}
+
+int
+stream_shutdownread_setup(void **state ISC_ATTR_UNUSED) {
+	int r = setup_netmgr_test(state);
+	return (r);
+}
+
+void
+stream_shutdownread(void **state ISC_ATTR_UNUSED) {
+	isc_result_t result = stream_listen(stream_accept_cb, NULL, 128, NULL,
+					    &listen_sock);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_loop_teardown(mainloop, stop_listening, listen_sock);
+
+	stream_connect(shutdownread_connectcb, NULL, T_CONNECT);
+}
+
+int
+stream_shutdownread_teardown(void **state ISC_ATTR_UNUSED) {
+	X(cconnects);
+	X(csends);
+	X(creads);
+
+	atomic_assert_int_eq(cconnects, 1);
+	atomic_assert_int_eq(csends, 1);
+	atomic_assert_int_eq(creads, 1);
+
+	return (teardown_netmgr_test(state));
+}

tests/isc/tcp_test.c

@@ -57,6 +57,16 @@ ISC_LOOP_TEST_IMPL(tcp_noresponse) {
 	return;
 }
 
+ISC_LOOP_TEST_IMPL(tcp_shutdownconnect) {
+	stream_shutdownconnect(arg);
+	return;
+}
+
+ISC_LOOP_TEST_IMPL(tcp_shutdownread) {
+	stream_shutdownread(arg);
+	return;
+}
+
 ISC_LOOP_TEST_IMPL(tcp_timeout_recovery) {
 	stream_timeout_recovery(arg);
 	return;
@@ -114,6 +124,10 @@ ISC_TEST_LIST_START
 ISC_TEST_ENTRY_CUSTOM(tcp_noop, stream_noop_setup, stream_noop_teardown)
 ISC_TEST_ENTRY_CUSTOM(tcp_noresponse, stream_noresponse_setup,
 		      stream_noresponse_teardown)
+ISC_TEST_ENTRY_CUSTOM(tcp_shutdownconnect, stream_shutdownconnect_setup,
+		      stream_shutdownconnect_teardown)
+ISC_TEST_ENTRY_CUSTOM(tcp_shutdownread, stream_shutdownread_setup,
+		      stream_shutdownread_teardown)
 ISC_TEST_ENTRY_CUSTOM(tcp_timeout_recovery, stream_timeout_recovery_setup,
 		      stream_timeout_recovery_teardown)
 ISC_TEST_ENTRY_CUSTOM(tcp_recv_one, stream_recv_one_setup,

tests/isc/tcpdns_test.c

@@ -47,13 +47,6 @@
 
 /* TCPDNS */
 
-static void
-stop_listening(void *arg ISC_ATTR_UNUSED) {
-	isc_nm_stoplistening(listen_sock);
-	isc_nmsocket_close(&listen_sock);
-	assert_null(listen_sock);
-}
-
 static void
 start_listening(uint32_t nworkers, isc_nm_accept_cb_t accept_cb,
 		isc_nm_recv_cb_t recv_cb) {

tests/isc/tls_test.c

@@ -55,6 +55,16 @@ ISC_LOOP_TEST_IMPL(tls_noresponse) {
 	return;
 }
 
+ISC_LOOP_TEST_IMPL(tls_shutdownconnect) {
+	stream_shutdownconnect(arg);
+	return;
+}
+
+ISC_LOOP_TEST_IMPL(tls_shutdownread) {
+	stream_shutdownread(arg);
+	return;
+}
+
 ISC_LOOP_TEST_IMPL(tls_timeout_recovery) {
 	stream_timeout_recovery(arg);
 	return;
@@ -109,6 +119,10 @@ ISC_TEST_LIST_START
 ISC_TEST_ENTRY_CUSTOM(tls_noop, stream_noop_setup, stream_noop_teardown)
 ISC_TEST_ENTRY_CUSTOM(tls_noresponse, stream_noresponse_setup,
 		      stream_noresponse_teardown)
+ISC_TEST_ENTRY_CUSTOM(tls_shutdownconnect, stream_shutdownconnect_setup,
+		      stream_shutdownconnect_teardown)
+ISC_TEST_ENTRY_CUSTOM(tls_shutdownread, stream_shutdownread_setup,
+		      stream_shutdownread_teardown)
 ISC_TEST_ENTRY_CUSTOM(tls_timeout_recovery, stream_timeout_recovery_setup,
 		      stream_timeout_recovery_teardown)
 ISC_TEST_ENTRY_CUSTOM(tls_recv_one, stream_recv_one_setup,

tests/isc/tlsdns_test.c

@@ -46,13 +46,6 @@
 
 #include <tests/isc.h>
 
-static void
-stop_listening(void *arg ISC_ATTR_UNUSED) {
-	isc_nm_stoplistening(listen_sock);
-	isc_nmsocket_close(&listen_sock);
-	assert_null(listen_sock);
-}
-
 static void
 start_listening(uint32_t nworkers, isc_nm_accept_cb_t accept_cb,
 		isc_nm_recv_cb_t recv_cb) {

tests/isc/udp_test.c

@@ -134,6 +134,15 @@ mock_recv_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
 	UNUSED(cbarg);
 }
 
+static void
+udp_listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult,
+		   isc_region_t *region, void *cbarg) {
+	if (eresult != ISC_R_SUCCESS) {
+		isc_refcount_increment0(&active_sreads);
+	}
+	listen_read_cb(handle, eresult, region, cbarg);
+}
+
 static void
 connect_nomemory_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 	UNUSED(handle);
@@ -145,13 +154,6 @@ connect_nomemory_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
 	isc_loopmgr_shutdown(loopmgr);
 }
 
-static void
-stop_listening(void *arg ISC_ATTR_UNUSED) {
-	isc_nm_stoplistening(listen_sock);
-	isc_nmsocket_close(&listen_sock);
-	assert_null(listen_sock);
-}
-
 static void
 start_listening(uint32_t nworkers, isc_nm_recv_cb_t cb) {
 	isc_result_t result = isc_nm_listenudp(
@@ -616,6 +618,7 @@ udp_shutdown_read_connect_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 	isc_refcount_increment0(&active_creads);
 	isc_nmhandle_attach(handle, &readhandle);
 	isc_nm_read(handle, udp_shutdown_read_read_cb, cbarg);
+	assert_true(handle->sock->reading);
 
 	/* Send */
 	isc_refcount_increment0(&active_csends);
@@ -712,7 +715,7 @@ udp_cancel_read_read_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 				    udp_cancel_read_send_cb, cbarg);
 		}
 		break;
-	case ISC_R_EOF:
+	case ISC_R_CANCELED:
 		/* The read has been canceled */
 		atomic_fetch_add(&creads, 1);
 		isc_loopmgr_shutdown(loopmgr);
@@ -936,7 +939,7 @@ ISC_TEARDOWN_TEST_IMPL(udp_recv_one) {
 }
 
 ISC_LOOP_TEST_IMPL(udp_recv_one) {
-	start_listening(ISC_NM_LISTEN_ONE, listen_read_cb);
+	start_listening(ISC_NM_LISTEN_ONE, udp_listen_read_cb);
 
 	udp__connect(NULL);
 }
@@ -976,7 +979,7 @@ ISC_TEARDOWN_TEST_IMPL(udp_recv_two) {
 }
 
 ISC_LOOP_TEST_IMPL(udp_recv_two) {
-	start_listening(ISC_NM_LISTEN_ONE, listen_read_cb);
+	start_listening(ISC_NM_LISTEN_ONE, udp_listen_read_cb);
 
 	udp__connect(NULL);
 	udp__connect(NULL);
@@ -1007,7 +1010,7 @@ ISC_TEARDOWN_TEST_IMPL(udp_recv_send) {
 }
 
 ISC_LOOP_TEST_IMPL(udp_recv_send) {
-	start_listening(ISC_NM_LISTEN_ALL, listen_read_cb);
+	start_listening(ISC_NM_LISTEN_ALL, udp_listen_read_cb);
 
 	for (size_t i = 0; i < workers; i++) {
 		isc_async_run(isc_loop_get(loopmgr, i), udp__connect, NULL);

tests/libtest/isc.c

@@ -37,7 +37,7 @@ isc_log_t *lctx = NULL;
 isc_loop_t *mainloop = NULL;
 isc_loopmgr_t *loopmgr = NULL;
 isc_nm_t *netmgr = NULL;
-unsigned int workers = -1;
+unsigned int workers = 0;
 
 static void
 adjustnofile(void) {
@@ -76,9 +76,14 @@ setup_loopmgr(void **state ISC_ATTR_UNUSED) {
 	if (env_workers != NULL) {
 		workers = atoi(env_workers);
 	}
-	if (workers < 2 || workers > 1000) {
+
+	if (workers == 0) {
+		workers = isc_os_ncpus();
+
 		/* We always need at least two loops for some of the tests */
-		workers = isc_os_ncpus() + 1;
+		if (workers < 2) {
+			workers = 2;
+		}
 	}
 
 	isc_loopmgr_create(mctx, workers, &loopmgr);

tests/ns/netmgr_wrap.c

@@ -22,9 +22,9 @@
 #include <ns/client.h>
 
 #if ISC_NETMGR_TRACE
-#define FLARG                                                                  \
-	, const char *file ISC_ATTR_UNUSED, unsigned int line ISC_ATTR_UNUSED, \
-		const char *func ISC_ATTR_UNUSED
+#define FLARG                                                                 \
+	, const char *func ISC_ATTR_UNUSED, const char *file ISC_ATTR_UNUSED, \
+		unsigned int line ISC_ATTR_UNUSED
 #else
 #define FLARG
 #endif
@@ -35,8 +35,13 @@
 atomic_uint_fast32_t client_refs[32];
 atomic_uintptr_t client_addrs[32];
 
+#if ISC_NETMGR_TRACE
 void
-isc__nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp FLARG) {
+isc_nmhandle__attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp FLARG) {
+#else
+void
+isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) {
+#endif
 	ns_client_t *client = (ns_client_t *)source;
 	int i;
 
@@ -58,8 +63,13 @@ isc__nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp FLARG) {
 	return;
 }
 
+#if ISC_NETMGR_TRACE
 void
-isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG) {
+isc_nmhandle__detach(isc_nmhandle_t **handlep FLARG) {
+#else
+void
+isc_nmhandle_detach(isc_nmhandle_t **handlep) {
+#endif
 	isc_nmhandle_t *handle = *handlep;
 	ns_client_t *client = (ns_client_t *)handle;
 	int i;