Merge branch 'each-rndc-netmgr-pt1.5' into 'main'

Fix socket closing races.

Closes #1660

See merge request isc-projects/bind9!3781

lib/isc/include/isc/netmgr.h

@@ -12,17 +12,19 @@
 #pragma once
 
 #include <isc/mem.h>
+#include <isc/region.h>
 #include <isc/result.h>
 #include <isc/types.h>
 
+/*
+ * Replacement for isc_sockettype_t provided by socket.h.
+ */
 typedef enum {
-	NMEV_READ,
+	isc_socktype_tcp = 1,
-	NMEV_WRITE,
+	isc_socktype_udp = 2,
-	NMEV_ACCEPT,
+	isc_socktype_unix = 3,
-	NMEV_CONNECTED,
+	isc_socktype_raw = 4
-	NMEV_CANCELLED,
+} isc_socktype_t;
-	NMEV_SHUTDOWN
-} isc_nm_eventtype;
 
 typedef void (*isc_nm_recv_cb_t)(isc_nmhandle_t *handle, isc_result_t eresult,
 				 isc_region_t *region, void *cbarg);

lib/isc/netmgr/netmgr-int.h

@@ -79,8 +79,10 @@ typedef struct isc__networker {
  * connections we have peer address here, so both TCP and UDP can be
  * handled with a simple send-like function
  */
-#define NMHANDLE_MAGIC	  ISC_MAGIC('N', 'M', 'H', 'D')
+#define NMHANDLE_MAGIC ISC_MAGIC('N', 'M', 'H', 'D')
-#define VALID_NMHANDLE(t) ISC_MAGIC_VALID(t, NMHANDLE_MAGIC)
+#define VALID_NMHANDLE(t)                      \
+	(ISC_MAGIC_VALID(t, NMHANDLE_MAGIC) && \
+	 atomic_load(&(t)->references) > 0)
 
 typedef void (*isc__nm_closecb)(isc_nmhandle_t *);
 

lib/isc/netmgr/netmgr.c

@@ -877,10 +877,10 @@ isc__nmsocket_prep_destroy(isc_nmsocket_t *sock) {
 		switch (sock->type) {
 		case isc_nm_tcpsocket:
 			isc__nm_tcp_close(sock);
-			break;
+			return;
 		case isc_nm_tcpdnssocket:
 			isc__nm_tcpdns_close(sock);
-			break;
+			return;
 		default:
 			break;
 		}
@@ -1068,8 +1068,8 @@ isc__nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer,
 	if (handle == NULL) {
 		handle = alloc_handle(sock);
 	} else {
-		INSIST(VALID_NMHANDLE(handle));
 		isc_refcount_increment0(&handle->references);
+		INSIST(VALID_NMHANDLE(handle));
 	}
 
 	isc__nmsocket_attach(sock, &handle->sock);

lib/isc/netmgr/tcp.c

@@ -195,9 +195,10 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 isc_result_t
 isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 		  isc_nm_cb_t cb, void *cbarg, size_t extrahandlesize) {
-	isc_nmsocket_t *nsock = NULL;
+	isc_nmsocket_t *nsock = NULL, *tmp = NULL;
 	isc__netievent_tcpconnect_t *ievent = NULL;
 	isc__nm_uvreq_t *req = NULL;
+	isc_result_t result = ISC_R_SUCCESS;
 
 	REQUIRE(VALID_NM(mgr));
 
@@ -215,6 +216,12 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 	ievent->sock = nsock;
 	ievent->req = req;
 
+	/*
+	 * Async callbacks can dereference the socket in the meantime,
+	 * we need to hold an additional reference to it.
+	 */
+	isc__nmsocket_attach(nsock, &tmp);
+
 	if (isc__nm_in_netthread()) {
 		nsock->tid = isc_nm_tid();
 		isc__nm_async_tcpconnect(&mgr->workers[nsock->tid],
@@ -234,12 +241,13 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
 	}
 
 	if (nsock->result != ISC_R_SUCCESS) {
-		isc_result_t result = nsock->result;
+		result = nsock->result;
 		isc__nmsocket_detach(&nsock);
-		return (result);
 	}
 
-	return (ISC_R_SUCCESS);
+	isc__nmsocket_detach(&tmp);
+
+	return (result);
 }
 
 isc_result_t

lib/isc/win32/libisc.def.in

@@ -454,8 +454,10 @@ isc_nm_closedown
 isc_nm_destroy
 isc_nm_detach
 isc_nm_listentcpdns
+isc_nm_listentcp
 isc_nm_listenudp
 isc_nm_maxudp
+isc_nm_read
 isc_nm_send
 isc_nm_setstats
 isc_nm_start