From 173c3524520b28f966e2d80cf98338befb13a708 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Tue, 27 Sep 2022 15:20:33 +0200
Subject: [PATCH 1/2] Call the isc__nm_udp_send() callbacks asynchronously on
 shutdown

The isc__nm_udp_send() callback would be called synchronously when
shutting down or when the socket has been closed.  This could lead to
double locking in the calling code and thus those callbacks needs to be
called asynchronously.
---
 lib/isc/netmgr/udp.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 9b715d9b6c..587e368203 100644
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -685,6 +685,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, const isc_region_t *region,
 	isc__networker_t *worker = NULL;
 	uint32_t maxudp;
 	int r;
+	isc_result_t result;
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->type == isc_nm_udpsocket);
@@ -706,16 +707,6 @@ isc__nm_udp_send(isc_nmhandle_t *handle, const isc_region_t *region,
 		return;
 	}
 
-	if (isc__nm_closing(worker)) {
-		cb(handle, ISC_R_SHUTTINGDOWN, cbarg);
-		return;
-	}
-
-	if (isc__nmsocket_closing(sock)) {
-		cb(handle, ISC_R_CANCELED, cbarg);
-		return;
-	}
-
 	uvreq = isc__nm_uvreq_get(sock->worker, sock);
 	uvreq->uvbuf.base = (char *)region->base;
 	uvreq->uvbuf.len = region->length;
@@ -725,6 +716,16 @@ isc__nm_udp_send(isc_nmhandle_t *handle, const isc_region_t *region,
 	uvreq->cb.send = cb;
 	uvreq->cbarg = cbarg;
 
+	if (isc__nm_closing(worker)) {
+		result = ISC_R_SHUTTINGDOWN;
+		goto fail;
+	}
+
+	if (isc__nmsocket_closing(sock)) {
+		result = ISC_R_CANCELED;
+		goto fail;
+	}
+
 	/*
 	 * We used uv_udp_connect(), so the peer address has to be
 	 * set to NULL or else uv_udp_send() could fail or assert,
@@ -738,8 +739,12 @@ isc__nm_udp_send(isc_nmhandle_t *handle, const isc_region_t *region,
 			&uvreq->uvbuf, 1, sa, udp_send_cb);
 	if (r < 0) {
 		isc__nm_incstats(sock, STATID_SENDFAIL);
-		isc__nm_failed_send_cb(sock, uvreq, isc_uverr2result(r));
+		result = isc_uverr2result(r);
+		goto fail;
 	}
+	return;
+fail:
+	isc__nm_failed_send_cb(sock, uvreq, result);
 }
 
 static isc_result_t

From b4a43bf2f24e642e9c78a984b8c1aa998ef19c77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Thu, 29 Sep 2022 11:12:15 +0200
Subject: [PATCH 2/2] Add developer documentation on the netmgr callbacks

Extra care must be taken when executing the callbacks to prevent the
deadlocks on the caller's side.  Add a paragraph that addresses when we
can and when we cannot call the callbacks directly.
---
 doc/dev/loopmgr.md | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/doc/dev/loopmgr.md b/doc/dev/loopmgr.md
index 9c4518154c..15831e6236 100644
--- a/doc/dev/loopmgr.md
+++ b/doc/dev/loopmgr.md
@@ -104,3 +104,13 @@ functions MUST be called from the thread that created the network manager
 socket.
 
 The ``isc_nm_listen*()`` functions MUST be called from the ``main`` loop.
+
+The general design of Network Manager is based on callbacks.  An extra care must
+be taken when implementing new functions because the callbacks MUST be called
+asynchronously because the caller might be inside a lock and the same lock must
+be acquired in the callback.  This doesn't mean that the callback must be always
+called asynchronously, because sometimes we are already in the libuv callback
+and thus we can just call the callback directly, but in other places, especially
+when returning an error, the control hasn't been returned to the caller yet and
+in such case, the callback must be scheduled onto the event loop instead of
+executing it directly.