checkpoint

- isc__nmhandle_get() now attaches to the sock in the nmhandle object.
  the caller is responsible for dereferencing the original socket
  pointer when necessary.
- tcpdns listener sockets attach sock->outer to the outer tcp listener
  socket. tcpdns connected sockets attach sock->outerhandle to the handle
  for the tcp connected socket.
- only listener sockets need to be attached/detached directly. connected
  sockets should only be accessed and reference-counted via their
  associated handles.

doc/misc/options

@@ -166,13 +166,16 @@ options {
         dnssec-secure-to-insecure <boolean>;
         dnssec-update-mode ( maintain | no-resign );
         dnssec-validation ( yes | no | auto );
-        dnstap { ( all | auth | client | forwarder | resolver | update ) [
-            ( query | response ) ]; ... };
-        dnstap-identity ( <quoted_string> | none | hostname );
-        dnstap-output ( file | unix ) <quoted_string> [ size ( unlimited |
-            <size> ) ] [ versions ( unlimited | <integer> ) ] [ suffix (
-            increment | timestamp ) ];
-        dnstap-version ( <quoted_string> | none );
+        dnstap { ( all | auth | client | forwarder |
+            resolver | update ) [ ( query | response ) ];
+            ... }; // not configured
+        dnstap-identity ( <quoted_string> | none |
+            hostname ); // not configured
+        dnstap-output ( file | unix ) <quoted_string> [
+            size ( unlimited | <size> ) ] [ versions (
+            unlimited | <integer> ) ] [ suffix ( increment
+            | timestamp ) ]; // not configured
+        dnstap-version ( <quoted_string> | none ); // not configured
         dscp <integer>;
         dual-stack-servers [ port <integer> ] { ( <quoted_string> [ port
             <integer> ] [ dscp <integer> ] | <ipv4_address> [ port
@@ -196,13 +199,13 @@ options {
         forward ( first | only );
         forwarders [ port <integer> ] [ dscp <integer> ] { ( <ipv4_address>
             | <ipv6_address> ) [ port <integer> ] [ dscp <integer> ]; ... };
-        fstrm-set-buffer-hint <integer>;
-        fstrm-set-flush-timeout <integer>;
-        fstrm-set-input-queue-size <integer>;
-        fstrm-set-output-notify-threshold <integer>;
-        fstrm-set-output-queue-model ( mpsc | spsc );
-        fstrm-set-output-queue-size <integer>;
-        fstrm-set-reopen-interval <duration>;
+        fstrm-set-buffer-hint <integer>; // not configured
+        fstrm-set-flush-timeout <integer>; // not configured
+        fstrm-set-input-queue-size <integer>; // not configured
+        fstrm-set-output-notify-threshold <integer>; // not configured
+        fstrm-set-output-queue-model ( mpsc | spsc ); // not configured
+        fstrm-set-output-queue-size <integer>; // not configured
+        fstrm-set-reopen-interval <duration>; // not configured
         geoip-directory ( <quoted_string> | none );
         geoip-use-ecs <boolean>; // obsolete
         glue-cache <boolean>;
@@ -547,8 +550,9 @@ view <string> [ <class> ] {
         dnssec-secure-to-insecure <boolean>;
         dnssec-update-mode ( maintain | no-resign );
         dnssec-validation ( yes | no | auto );
-        dnstap { ( all | auth | client | forwarder | resolver | update ) [
-            ( query | response ) ]; ... };
+        dnstap { ( all | auth | client | forwarder |
+            resolver | update ) [ ( query | response ) ];
+            ... }; // not configured
         dual-stack-servers [ port <integer> ] { ( <quoted_string> [ port
             <integer> ] [ dscp <integer> ] | <ipv4_address> [ port
             <integer> ] [ dscp <integer> ] | <ipv6_address> [ port

lib/isc/include/isc/netmgr.h

@@ -57,26 +57,13 @@ isc_nm_closedown(isc_nm_t *mgr);
 int
 isc_nm_tid(void);
 
-/*
- * isc_nm_freehandle frees a handle, releasing resources
- */
 void
-isc_nm_freehandle(isc_nmhandle_t *handle);
-
-void
-isc_nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target);
+isc_nmsocket_close(isc_nmsocket_t **sockp);
 /*%<
- * isc_nmsocket_attach attaches to a socket, increasing refcount
- */
-
-void
-isc_nmsocket_close(isc_nmsocket_t *sock);
-
-void
-isc_nmsocket_detach(isc_nmsocket_t **socketp);
-/*%<
- * isc_nmsocket_detach detaches from socket, decreasing refcount
- * and possibly destroying the socket if it's no longer referenced.
+ * isc_nmsocket_close() detaches a listening socket that was
+ * created by isc_nm_listenudp(), isc_nm_listentcp(), or
+ * isc_nm_listentcpdns(). Once there are no remaining child
+ * sockets with active handles, the socket will be closed.
  */
 
 void

lib/isc/netmgr/netmgr-int.h

@@ -96,21 +96,23 @@ struct isc_nmhandle {
 	 * the socket.
 	 */
 	isc_nmsocket_t *sock;
-	size_t ah_pos; /* Position in the socket's
-			* 'active handles' array */
+
+	/* Position in the socket's 'active handles' array */
+	size_t ah_pos;
 
 	/*
-	 * The handle is 'inflight' if netmgr is not currently processing
-	 * it in any way - it might mean that e.g. a recursive resolution
-	 * is happening. For an inflight handle we must wait for the
-	 * calling code to finish before we can free it.
+	 * When a handle is 'connected' then it holds an additional reference
+	 * to its associated socket, and an additional reference to itself;
+	 * it cannot be deleted until it is disconnected.
 	 */
-	atomic_bool inflight;
+	atomic_bool connected;
 
 	isc_sockaddr_t peer;
 	isc_sockaddr_t local;
 	isc_nm_opaquecb_t doreset; /* reset extra callback, external */
 	isc_nm_opaquecb_t dofree;  /* free extra callback, external */
+	isc_nm_opaquecb_t dcb;     /* disconnect callback */
+	isc_nm_opaquecb_t dcbarg;  /* disconnect callback argument */
 	void *opaque;
 	char extra[];
 };
@@ -405,6 +407,7 @@ struct isc_nmsocket {
 	int nchildren;
 	isc_nmiface_t *iface;
 	isc_nmhandle_t *tcphandle;
+	isc_nmhandle_t *outerhandle;
 
 	/*% Extra data allocated at the end of each isc_nmhandle_t */
 	size_t extrahandlesize;
@@ -589,6 +592,9 @@ isc__nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer,
  *
  * If 'local' is not NULL, set the handle's local address to 'local',
  * otherwise set it to 'sock->iface->addr'.
+ *
+ * 'sock' will be attached to 'handle->sock'. The caller may need
+ * to detach the socket afterward.
  */
 
 isc__nm_uvreq_t *
@@ -615,6 +621,19 @@ isc__nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
  * and its interface to 'iface'.
  */
 
+void
+isc__nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target);
+/*%<
+ * Attach to a socket, increasing refcount
+ */
+
+void
+isc__nmsocket_detach(isc_nmsocket_t **socketp);
+/*%<
+ * Detach from socket, decreasing refcount and possibly destroying the
+ * socket if it's no longer referenced.
+ */
+
 void
 isc__nmsocket_prep_destroy(isc_nmsocket_t *sock);
 /*%<
@@ -787,3 +806,15 @@ isc__nm_decstats(isc_nm_t *mgr, isc_statscounter_t counterid);
 /*%<
  * Decrement socket-related statistics counters.
  */
+
+const char *
+isc__nm_socket_type(isc_nmsocket_type type);
+/*%<
+ * Returns socket type as a string for logging purposes.
+ */
+
+void
+isc__nmhandle_connected(isc_nmhandle_t *handle,
+			isc_nm_opaquecb_t disconnect_cb, void *dcbarg);
+void
+isc__nmhandle_disconnect(isc_nmhandle_t *handle);

lib/isc/netmgr/netmgr.c

@@ -129,6 +129,27 @@ async_cb(uv_async_t *handle);
 static void
 process_queue(isc__networker_t *worker, isc_queue_t *queue);
 
+const char *
+isc__nm_socket_type(isc_nmsocket_type type) {
+	switch(type) {
+	case isc_nm_udpsocket:
+		return("udpsocket");
+	case isc_nm_udplistener:
+		return("udplistener");
+	case isc_nm_tcpsocket:
+		return("tcpsocket");
+	case isc_nm_tcplistener:
+		return("tcplistener");
+	case isc_nm_tcpdnssocket:
+		return("tcpdnssocket");
+	case isc_nm_tcpdnslistener:
+		return("tcpdnslistener");
+	default:
+		INSIST(0);
+		ISC_UNREACHABLE();
+	}
+}
+
 int
 isc_nm_tid(void) {
 	return (isc__nm_tid_v);
@@ -686,14 +707,16 @@ isc__nmsocket_active(isc_nmsocket_t *sock) {
 }
 
 void
-isc_nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target) {
+isc__nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(target != NULL && *target == NULL);
 
 	if (sock->parent != NULL) {
 		INSIST(sock->parent->parent == NULL); /* sanity check */
+fprintf(stderr, "attach sock %p (%s) (%p) to %lu\n", sock, isc__nm_socket_type(sock->type), sock->parent, sock->parent->references+1);
 		isc_refcount_increment0(&sock->parent->references);
 	} else {
+fprintf(stderr, "attach sock %p (%s) to %lu\n", sock, isc__nm_socket_type(sock->type), sock->references+1);
 		isc_refcount_increment0(&sock->references);
 	}
 
@@ -736,9 +759,16 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree) {
 		isc__nm_decstats(sock->mgr, sock->statsindex[STATID_ACTIVE]);
 	}
 
-	if (sock->tcphandle != NULL) {
-		isc_nmhandle_unref(sock->tcphandle);
-		sock->tcphandle = NULL;
+	sock->tcphandle = NULL;
+
+	if (sock->outerhandle != NULL) {
+		isc_nmhandle_unref(sock->outerhandle);
+fprintf(stderr, "sock %p outerhandle %p, clearing?\n", sock, sock->outerhandle);
+		sock->outerhandle = NULL;
+	}
+
+	if (sock->outer != NULL) {
+		isc__nmsocket_detach(&sock->outer);
 	}
 
 	while ((handle = isc_astack_pop(sock->inactivehandles)) != NULL) {
@@ -878,7 +908,7 @@ isc__nmsocket_prep_destroy(isc_nmsocket_t *sock) {
 }
 
 void
-isc_nmsocket_detach(isc_nmsocket_t **sockp) {
+isc__nmsocket_detach(isc_nmsocket_t **sockp) {
 	REQUIRE(sockp != NULL && *sockp != NULL);
 	REQUIRE(VALID_NMSOCK(*sockp));
 
@@ -891,8 +921,10 @@ isc_nmsocket_detach(isc_nmsocket_t **sockp) {
 	 */
 	if (sock->parent != NULL) {
 		rsock = sock->parent;
+fprintf(stderr, "detach sock %p (%s) (%p) to %lu\n", sock, isc__nm_socket_type(sock->type), rsock, rsock->references-1);
 		INSIST(rsock->parent == NULL); /* Sanity check */
 	} else {
+fprintf(stderr, "detach sock %p (%s) to %lu\n", sock, isc__nm_socket_type(sock->type), sock->references-1);
 		rsock = sock;
 	}
 
@@ -901,6 +933,17 @@ isc_nmsocket_detach(isc_nmsocket_t **sockp) {
 	}
 }
 
+void
+isc_nmsocket_close(isc_nmsocket_t **sockp) {
+	REQUIRE(sockp != NULL);
+	REQUIRE(VALID_NMSOCK(*sockp));
+	REQUIRE((*sockp)->type == isc_nm_udplistener ||
+		(*sockp)->type == isc_nm_tcplistener ||
+		(*sockp)->type == isc_nm_tcpdnslistener);
+
+	isc__nmsocket_detach(sockp);
+}
+
 void
 isc__nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
 		   isc_nmiface_t *iface) {
@@ -911,6 +954,7 @@ isc__nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
 	REQUIRE(iface != NULL);
 
 	family = iface->addr.type.sa.sa_family;
+fprintf(stderr, "init sock %p (%s)\n", sock, isc__nm_socket_type(type));
 
 	*sock = (isc_nmsocket_t){ .type = type,
 				  .iface = iface,
@@ -1039,7 +1083,8 @@ isc__nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t *peer,
 		isc_refcount_increment0(&handle->references);
 	}
 
-	handle->sock = sock;
+	isc__nmsocket_attach(sock, &handle->sock);
+
 	if (peer != NULL) {
 		memcpy(&handle->peer, peer, sizeof(isc_sockaddr_t));
 	} else {
@@ -1094,6 +1139,7 @@ void
 isc_nmhandle_ref(isc_nmhandle_t *handle) {
 	REQUIRE(VALID_NMHANDLE(handle));
 
+fprintf(stderr, "ref %p (%p %s) to %lu\n", handle, handle->sock, isc__nm_socket_type(handle->sock->type), handle->references+1);
 	isc_refcount_increment(&handle->references);
 }
 
@@ -1149,27 +1195,31 @@ nmhandle_deactivate(isc_nmsocket_t *sock, isc_nmhandle_t *handle) {
 
 void
 isc_nmhandle_unref(isc_nmhandle_t *handle) {
-	isc_nmsocket_t *sock = NULL, *tmp = NULL;
+	isc_nmsocket_t *sock = NULL;
 
 	REQUIRE(VALID_NMHANDLE(handle));
 
+fprintf(stderr, "unref %p (%p %s) to %lu\n", handle, handle->sock, isc__nm_socket_type(handle->sock->type), handle->references-1);
 	if (isc_refcount_decrement(&handle->references) > 1) {
 		return;
 	}
 
-	sock = handle->sock;
+	/*
+	 * If connected, we should still have at least one reference.
+	 */
+	INSIST(!atomic_load(&handle->connected));
+
+	/*
+	 * Reference the socket to ensure that it can't be deleted by
+	 * another thread while we're deactivating the handle.
+	 */
+	isc__nmsocket_attach(handle->sock, &sock);
 	handle->sock = NULL;
 
 	if (handle->doreset != NULL) {
 		handle->doreset(handle->opaque);
 	}
 
-	/*
-	 * Temporarily reference the socket to ensure that it can't
-	 * be deleted by another thread while we're deactivating the
-	 * handle.
-	 */
-	isc_nmsocket_attach(sock, &tmp);
 	nmhandle_deactivate(sock, handle);
 
 	/*
@@ -1184,16 +1234,42 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) {
 			isc__netievent_closecb_t *event = isc__nm_get_ievent(
 				sock->mgr, netievent_closecb);
 			/*
-			 * The socket will be finally detached by the closecb
-			 * event handler.
+			 * The socket will be detached by both the closecb
+			 * event handler and the closehandle callback.
 			 */
-			isc_nmsocket_attach(sock, &event->sock);
+			isc__nmsocket_attach(sock, &event->sock);
 			isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
 					       (isc__netievent_t *)event);
 		}
 	}
+	isc__nmsocket_detach(&sock);
+}
 
-	isc_nmsocket_detach(&tmp);
+void
+isc__nmhandle_connected(isc_nmhandle_t *handle,
+			isc_nm_opaquecb_t disconnect_cb, void *dcbarg) {
+	isc_nmsocket_t *tsock = NULL;
+fprintf(stderr, "connect handle %p (%p %s)\n", handle, handle->sock, isc__nm_socket_type(handle->sock->type));
+	isc_nmhandle_ref(handle);
+	isc__nmsocket_attach(handle->sock, &tsock);
+	atomic_store(&handle->connected, true);
+	handle->dcb = disconnect_cb;
+	handle->dcbarg = dcbarg;
+}
+
+void
+isc__nmhandle_disconnect(isc_nmhandle_t *handle) {
+	isc_nmsocket_t *tsock = handle->sock;
+fprintf(stderr, "disconnect handle %p (%p %s)\n", handle, handle->sock, isc__nm_socket_type(handle->sock->type));
+	isc__nmsocket_detach(&tsock);
+	atomic_store(&handle->connected, false);
+
+	if (handle->dcb != NULL) {
+		handle->dcb(handle->dcbarg);
+		handle->dcb = NULL;
+		handle->dcbarg = NULL;
+	}
+	isc_nmhandle_unref(handle);
 }
 
 void *
@@ -1260,7 +1336,7 @@ isc__nm_uvreq_get(isc_nm_t *mgr, isc_nmsocket_t *sock) {
 
 	*req = (isc__nm_uvreq_t){ .magic = 0 };
 	req->uv_req.req.data = req;
-	isc_nmsocket_attach(sock, &req->sock);
+	isc__nmsocket_attach(sock, &req->sock);
 	req->magic = UVREQ_MAGIC;
 
 	return (req);
@@ -1297,7 +1373,7 @@ isc__nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock) {
 		isc_nmhandle_unref(handle);
 	}
 
-	isc_nmsocket_detach(&sock);
+	isc__nmsocket_detach(&sock);
 }
 
 isc_result_t
@@ -1386,7 +1462,7 @@ isc__nm_async_closecb(isc__networker_t *worker, isc__netievent_t *ev0) {
 	UNUSED(worker);
 
 	ievent->sock->closehandle_cb(ievent->sock);
-	isc_nmsocket_detach(&ievent->sock);
+	isc__nmsocket_detach(&ievent->sock);
 }
 
 static void

lib/isc/netmgr/tcp.c

@@ -125,14 +125,15 @@ static void
 tcp_connect_cb(uv_connect_t *uvreq, int status) {
 	isc__nm_uvreq_t *req = (isc__nm_uvreq_t *)uvreq->data;
 	isc_nmsocket_t *sock = NULL;
+
 	sock = uv_handle_get_data((uv_handle_t *)uvreq->handle);
 
 	REQUIRE(VALID_UVREQ(req));
 
 	if (status == 0) {
 		isc_result_t result;
-		isc_nmhandle_t *handle = NULL;
 		struct sockaddr_storage ss;
+		isc_nmhandle_t *handle = NULL;
 
 		isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CONNECT]);
 		uv_tcp_getpeername(&sock->uv_handle.tcp, (struct sockaddr *)&ss,
@@ -143,6 +144,19 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 
 		handle = isc__nmhandle_get(sock, NULL, NULL);
 		req->cb.connect(handle, ISC_R_SUCCESS, req->cbarg);
+
+		isc__nm_uvreq_put(&req, sock);
+
+		/*
+		 * The sock is now attached to the handle.
+		 */
+		isc__nmsocket_detach(&sock);
+
+		/*
+		 * If the connect callback wants to hold on to the handle,
+		 * it needs to attach to it.
+		 */
+		isc_nmhandle_unref(handle);
 	} else {
 		/*
 		 * TODO:
@@ -151,9 +165,8 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) {
 		isc__nm_incstats(sock->mgr,
 				 sock->statsindex[STATID_CONNECTFAIL]);
 		req->cb.connect(NULL, isc__nm_uverr2result(status), req->cbarg);
+		isc__nm_uvreq_put(&req, sock);
 	}
-
-	isc__nm_uvreq_put(&req, sock);
 }
 
 isc_result_t
@@ -167,8 +180,8 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb,
 
 	nsock = isc_mem_get(mgr->mctx, sizeof(*nsock));
 	isc__nmsocket_init(nsock, mgr, isc_nm_tcplistener, iface);
-	nsock->rcb.accept = cb;
-	nsock->rcbarg = cbarg;
+	nsock->accept_cb.accept = cb;
+	nsock->accept_cbarg = cbarg;
 	nsock->extrahandlesize = extrahandlesize;
 	nsock->backlog = backlog;
 	nsock->result = ISC_R_SUCCESS;
@@ -205,7 +218,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb,
 		return (ISC_R_SUCCESS);
 	} else {
 		isc_result_t result = nsock->result;
-		isc_nmsocket_detach(&nsock);
+		isc__nmsocket_detach(&nsock);
 		return (result);
 	}
 }
@@ -375,14 +388,24 @@ isc__nm_async_tcpchildaccept(isc__networker_t *worker, isc__netievent_t *ev0) {
 		goto error;
 	}
 
-	isc_nmsocket_attach(ssock, &csock->server);
+	isc__nmsocket_attach(ssock, &csock->server);
 
 	handle = isc__nmhandle_get(csock, NULL, &local);
 
-	INSIST(ssock->rcb.accept != NULL);
+	INSIST(ssock->accept_cb.accept != NULL);
 	csock->read_timeout = ssock->mgr->init;
-	ssock->rcb.accept(handle, ISC_R_SUCCESS, ssock->rcbarg);
-	isc_nmsocket_detach(&csock);
+	ssock->accept_cb.accept(handle, ISC_R_SUCCESS, ssock->accept_cbarg);
+
+	/*
+	 * csock is now attached to the handle.
+	 */
+	isc__nmsocket_detach(&csock);
+
+	/*
+	 * If the accept callback wants to hold on to the handle,
+	 * it needs to attach to it.
+	 */
+	isc_nmhandle_unref(handle);
 	return;
 
 error:
@@ -401,7 +424,7 @@ error:
 	/*
 	 * Detach the socket properly to make sure uv_close() is called.
 	 */
-	isc_nmsocket_detach(&csock);
+	isc__nmsocket_detach(&csock);
 }
 
 void
@@ -412,7 +435,7 @@ isc__nm_tcp_stoplistening(isc_nmsocket_t *sock) {
 	REQUIRE(!isc__nm_in_netthread());
 
 	ievent = isc__nm_get_ievent(sock->mgr, netievent_tcpstop);
-	isc_nmsocket_attach(sock, &ievent->sock);
+	isc__nmsocket_attach(sock, &ievent->sock);
 	isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
 			       (isc__netievent_t *)ievent);
 }
@@ -458,7 +481,7 @@ tcp_listenclose_cb(uv_handle_t *handle) {
 	sock->pquota = NULL;
 	UNLOCK(&sock->lock);
 
-	isc_nmsocket_detach(&sock);
+	isc__nmsocket_detach(&sock);
 }
 
 static void
@@ -693,7 +716,7 @@ isc__nm_async_tcpaccept(isc__networker_t *worker, isc__netievent_t *ev0) {
 	/*
 	 * The socket was attached just before we called isc_quota_attach_cb().
 	 */
-	isc_nmsocket_detach(&ievent->sock);
+	isc__nmsocket_detach(&ievent->sock);
 }
 
 /*
@@ -733,11 +756,11 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 		 * We need to attach to ssock, because it might be queued
 		 * waiting for a TCP quota slot.  If so, then we'll detach it
 		 * later when the connection is accepted. (XXX: This may be
-		 * suboptimal, it might be better to attach unless
+		 * suboptimal, it might be better not to attach unless
 		 * we need to.)
 		 */
 		isc_nmsocket_t *tsock = NULL;
-		isc_nmsocket_attach(ssock, &tsock);
+		isc__nmsocket_attach(ssock, &tsock);
 		isc_quota_cb_init(&ssock->quotacb, quota_accept_cb, tsock);
 		result = isc_quota_attach_cb(ssock->pquota, &quota,
 					     &ssock->quotacb);
@@ -752,7 +775,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 		 * clear the quota callback and and detach the socket.
 		 */
 		isc_quota_cb_init(&ssock->quotacb, NULL, NULL);
-		isc_nmsocket_detach(&tsock);
+		isc__nmsocket_detach(&tsock);
 	}
 
 	isc__nm_incstats(ssock->mgr, ssock->statsindex[STATID_ACCEPT]);
@@ -902,6 +925,7 @@ tcp_close_cb(uv_handle_t *uvhandle) {
 
 	REQUIRE(VALID_NMSOCK(sock));
 
+fprintf(stderr, "close tcp socket %p\n", sock);
 	isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CLOSE]);
 	atomic_store(&sock->closed, true);
 	isc__nmsocket_prep_destroy(sock);
@@ -913,7 +937,9 @@ timer_close_cb(uv_handle_t *uvhandle) {
 
 	REQUIRE(VALID_NMSOCK(sock));
 
-	isc_nmsocket_detach(&sock->server);
+	if (sock->server != NULL) {
+		isc__nmsocket_detach(&sock->server);
+	}
 	uv_close(&sock->uv_handle.handle, tcp_close_cb);
 }
 
@@ -931,7 +957,7 @@ tcp_close_direct(isc_nmsocket_t *sock) {
 		uv_close((uv_handle_t *)&sock->timer, timer_close_cb);
 	} else {
 		if (sock->server != NULL) {
-			isc_nmsocket_detach(&sock->server);
+			isc__nmsocket_detach(&sock->server);
 		}
 		uv_close(&sock->uv_handle.handle, tcp_close_cb);
 	}
@@ -974,5 +1000,8 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
 	    sock->rcb.recv != NULL)
 	{
 		sock->rcb.recv(sock->tcphandle, NULL, sock->rcbarg);
+		if (atomic_load(&sock->tcphandle->connected)) {
+			isc__nmhandle_disconnect(sock->tcphandle);
+		}
 	}
 }

lib/isc/netmgr/tcpdns.c

@@ -82,7 +82,7 @@ static void
 timer_close_cb(uv_handle_t *handle) {
 	isc_nmsocket_t *sock = (isc_nmsocket_t *)uv_handle_get_data(handle);
 	INSIST(VALID_NMSOCK(sock));
-	isc_nmsocket_detach(&sock);
+fprintf(stderr, "timer close cb, sock %p\n", sock);
 }
 
 static void
@@ -95,14 +95,21 @@ dnstcp_readtimeout(uv_timer_t *timer) {
 	tcpdns_close_direct(sock);
 }
 
+static void
+disconnect_cb(void *arg) {
+	isc_nmsocket_t *sock = (isc_nmsocket_t *)arg;
+	isc__nmsocket_detach(&sock);
+}
+
 /*
  * Accept callback for TCP-DNS connection.
  */
 static void
 dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	isc_nmsocket_t *dnslistensock = (isc_nmsocket_t *)cbarg;
-	isc_nmsocket_t *dnssock = NULL;
+	isc_nmsocket_t *dnssock = NULL, *tsock = NULL;
 
+fprintf(stderr, "dnslisten_acceptcb %p %s\n", handle, isc_result_totext(result));
 	REQUIRE(VALID_NMSOCK(dnslistensock));
 	REQUIRE(dnslistensock->type == isc_nm_tcpdnslistener);
 
@@ -122,8 +129,14 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 			   handle->sock->iface);
 
 	dnssock->extrahandlesize = dnslistensock->extrahandlesize;
-	isc_nmsocket_attach(dnslistensock, &dnssock->listener);
-	isc_nmsocket_attach(handle->sock, &dnssock->outer);
+	isc__nmsocket_attach(dnslistensock, &dnssock->listener);
+
+	dnssock->outerhandle = handle;
+	isc_nmhandle_ref(dnssock->outerhandle);
+
+	isc__nmsocket_attach(dnssock, &tsock);
+	isc__nmhandle_connected(dnssock->outerhandle, disconnect_cb, dnssock);
+
 	dnssock->peer = handle->sock->peer;
 	dnssock->read_timeout = handle->sock->mgr->init;
 	dnssock->tid = isc_nm_tid();
@@ -136,7 +149,11 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	uv_timer_start(&dnssock->timer, dnstcp_readtimeout,
 		       dnssock->read_timeout, 0);
 
-	isc_nm_read(handle, dnslisten_readcb, dnssock);
+	isc_nmhandle_ref(handle);
+	result = isc_nm_read(handle, dnslisten_readcb, dnssock);
+	if (result != ISC_R_SUCCESS) {
+		isc_nmhandle_unref(handle);
+	}
 }
 
 /*
@@ -188,6 +205,11 @@ processbuffer(isc_nmsocket_t *dnssock, isc_nmhandle_t **handlep) {
 				dnssock->buf_len);
 		}
 
+		/*
+		 * dnssock is now attached to dnshandle.
+		 */
+		isc__nmsocket_detach(&dnssock);
+
 		*handlep = dnshandle;
 		return (ISC_R_SUCCESS);
 	}
@@ -210,6 +232,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 	REQUIRE(VALID_NMHANDLE(handle));
 	REQUIRE(dnssock->tid == isc_nm_tid());
 
+fprintf(stderr, "dnslisten_readcb %p (reg %p), sock %p (%s)\n", handle, handle->sock, region, handle->sock ?  isc__nm_socket_type(handle->sock->type) : "nil");
 	if (region == NULL) {
 		/* Connection closed */
 		isc__nm_tcpdns_close(dnssock);
@@ -234,6 +257,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 		isc_nmhandle_t *dnshandle = NULL;
 
 		result = processbuffer(dnssock, &dnshandle);
+fprintf(stderr, "processbuffer sock %p with handle %p, result %s\n", dnssock, dnshandle, isc_result_totext(result));
 		if (result != ISC_R_SUCCESS) {
 			/*
 			 * There wasn't anything in the buffer to process.
@@ -244,7 +268,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 		/*
 		 * We have a packet: stop timeout timers
 		 */
-		atomic_store(&dnssock->outer->processing, true);
+		atomic_store(&dnssock->outerhandle->sock->processing, true);
 		if (dnssock->timer_initialized) {
 			uv_timer_stop(&dnssock->timer);
 		}
@@ -255,7 +279,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 			 * one packet, so we're done until the next read
 			 * completes.
 			 */
-			isc_nm_pauseread(dnssock->outer);
+			isc_nm_pauseread(dnssock->outerhandle->sock);
 			done = true;
 		} else {
 			/*
@@ -267,7 +291,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 			 */
 			if (atomic_load(&dnssock->ah) >=
 			    TCPDNS_CLIENTS_PER_CONN) {
-				isc_nm_pauseread(dnssock->outer);
+				isc_nm_pauseread(dnssock->outerhandle->sock);
 				done = true;
 			}
 		}
@@ -310,7 +334,7 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
 		return (ISC_R_SUCCESS);
 	} else {
 		atomic_store(&dnslistensock->closed, true);
-		isc_nmsocket_detach(&dnslistensock);
+		isc__nmsocket_detach(&dnslistensock);
 		return (result);
 	}
 }
@@ -326,8 +350,8 @@ isc__nm_tcpdns_stoplistening(isc_nmsocket_t *sock) {
 	sock->rcbarg = NULL;
 
 	if (sock->outer != NULL) {
-		isc_nm_stoplistening(sock->outer);
-		isc_nmsocket_detach(&sock->outer);
+		isc__nm_tcp_stoplistening(sock->outer);
+		isc__nmsocket_detach(&sock->outer);
 	}
 }
 
@@ -336,7 +360,8 @@ isc_nm_tcpdns_sequential(isc_nmhandle_t *handle) {
 	REQUIRE(VALID_NMHANDLE(handle));
 
 	if (handle->sock->type != isc_nm_tcpdnssocket ||
-	    handle->sock->outer == NULL) {
+	    handle->sock->outerhandle == NULL)
+	{
 		return;
 	}
 
@@ -348,7 +373,7 @@ isc_nm_tcpdns_sequential(isc_nmhandle_t *handle) {
 	 * closehandle_cb callback, called whenever a handle
 	 * is released.
 	 */
-	isc_nm_pauseread(handle->sock->outer);
+	isc_nm_pauseread(handle->sock->outerhandle->sock);
 	atomic_store(&handle->sock->sequential, true);
 }
 
@@ -357,12 +382,13 @@ isc_nm_tcpdns_keepalive(isc_nmhandle_t *handle) {
 	REQUIRE(VALID_NMHANDLE(handle));
 
 	if (handle->sock->type != isc_nm_tcpdnssocket ||
-	    handle->sock->outer == NULL) {
+	    handle->sock->outerhandle == NULL)
+	{
 		return;
 	}
 
 	atomic_store(&handle->sock->keepalive, true);
-	atomic_store(&handle->sock->outer->keepalive, true);
+	atomic_store(&handle->sock->outerhandle->sock->keepalive, true);
 }
 
 typedef struct tcpsend {
@@ -378,17 +404,16 @@ static void
 resume_processing(void *arg) {
 	isc_nmsocket_t *sock = (isc_nmsocket_t *)arg;
 	isc_result_t result;
+	bool resumed = false;
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
-
-	if (sock->type != isc_nm_tcpdnssocket || sock->outer == NULL) {
-		return;
-	}
+	REQUIRE(sock->type == isc_nm_tcpdnssocket);
+	REQUIRE(sock->outerhandle != NULL);
 
 	if (atomic_load(&sock->ah) == 0) {
 		/* Nothing is active; sockets can timeout now */
-		atomic_store(&sock->outer->processing, false);
+		atomic_store(&sock->outerhandle->sock->processing, false);
 		if (sock->timer_initialized) {
 			uv_timer_start(&sock->timer, dnstcp_readtimeout,
 				       sock->read_timeout, 0);
@@ -403,17 +428,23 @@ resume_processing(void *arg) {
 		isc_nmhandle_t *handle = NULL;
 
 		result = processbuffer(sock, &handle);
+fprintf(stderr, "processed sequential sock %p with handle %p, result %s\n", sock, handle, isc_result_totext(result));
 		if (result == ISC_R_SUCCESS) {
-			atomic_store(&sock->outer->processing, true);
+			atomic_store(&sock->outerhandle->sock->processing,
+				     true);
 			if (sock->timer_initialized) {
 				uv_timer_stop(&sock->timer);
 			}
 			isc_nmhandle_unref(handle);
-		} else if (sock->outer != NULL) {
-			isc_nm_resumeread(sock->outer);
+		} else {
+			/*
+			 * Nothing in the buffer; resume reading.
+			 */
+			isc_nm_resumeread(sock->outerhandle->sock);
+			resumed = true;
 		}
 
-		return;
+		goto done;
 	}
 
 	/*
@@ -424,36 +455,46 @@ resume_processing(void *arg) {
 		isc_nmhandle_t *dnshandle = NULL;
 
 		result = processbuffer(sock, &dnshandle);
-		if (result != ISC_R_SUCCESS) {
+fprintf(stderr, "processed nonsequentially sock %p with handle %p, result %s\n", sock, dnshandle, isc_result_totext(result));
+		if (result == ISC_R_SUCCESS) {
+			if (sock->timer_initialized) {
+				uv_timer_stop(&sock->timer);
+			}
+			atomic_store(&sock->outerhandle->sock->processing, true);
+			isc_nmhandle_unref(dnshandle);
+		} else {
 			/*
 			 * Nothing in the buffer; resume reading.
 			 */
-			if (sock->outer != NULL) {
-				isc_nm_resumeread(sock->outer);
+			if (sock->outerhandle != NULL) {
+				isc_nm_resumeread(sock->outerhandle->sock);
+				resumed = true;
 			}
 
 			break;
 		}
-
-		if (sock->timer_initialized) {
-			uv_timer_stop(&sock->timer);
-		}
-		atomic_store(&sock->outer->processing, true);
-		isc_nmhandle_unref(dnshandle);
 	} while (atomic_load(&sock->ah) < TCPDNS_CLIENTS_PER_CONN);
+
+done:
+	if (!resumed && sock->outerhandle != NULL) {
+		isc__nmhandle_disconnect(sock->outerhandle);
+	}
+
+	isc__nmsocket_detach(&sock);
 }
 
 static void
 tcpdnssend_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	tcpsend_t *ts = (tcpsend_t *)cbarg;
 
-	UNUSED(handle);
-
+fprintf(stderr, "tcpdnssend_cb %p %s\n", handle, isc_result_totext(result));
 	ts->cb(ts->orighandle, result, ts->cbarg);
 	isc_mem_put(ts->mctx, ts->region.base, ts->region.length);
 
 	isc_nmhandle_unref(ts->orighandle);
 	isc_mem_putanddetach(&ts->mctx, ts, sizeof(*ts));
+
+	isc_nmhandle_unref(handle);
 }
 
 /*
@@ -471,7 +512,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->type == isc_nm_tcpdnssocket);
 
-	if (sock->outer == NULL) {
+	if (sock->outerhandle == NULL) {
 		/* The socket is closed */
 		return (ISC_R_NOTCONNECTED);
 	}
@@ -480,7 +521,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
 	*t = (tcpsend_t){
 		.cb = cb,
 		.cbarg = cbarg,
-		.handle = handle->sock->outer->tcphandle,
+		.handle = handle->sock->outerhandle,
 	};
 
 	isc_mem_attach(sock->mgr->mctx, &t->mctx);
@@ -500,6 +541,8 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
 static void
 tcpdns_close_direct(isc_nmsocket_t *sock) {
 	REQUIRE(sock->tid == isc_nm_tid());
+
+fprintf(stderr, "close sock %p (tcpdns)\n", sock);
 	/* We don't need atomics here, it's all in single network thread */
 	if (sock->timer_initialized) {
 		/*
@@ -509,21 +552,23 @@ tcpdns_close_direct(isc_nmsocket_t *sock) {
 		 */
 		sock->timer_initialized = false;
 		uv_timer_stop(&sock->timer);
+fprintf(stderr, "closing timer\n");
 		uv_close((uv_handle_t *)&sock->timer, timer_close_cb);
+fprintf(stderr, "...done\n");
 	} else {
 		/*
 		 * At this point we're certain that there are no external
 		 * references, we can close everything.
 		 */
-		if (sock->outer != NULL) {
-			sock->outer->rcb.recv = NULL;
-			isc_nmsocket_detach(&sock->outer);
-		}
 		if (sock->listener != NULL) {
-			isc_nmsocket_detach(&sock->listener);
+fprintf(stderr, "clearing listener\n");
+			isc__nmsocket_detach(&sock->listener);
 		}
 		atomic_store(&sock->closed, true);
+fprintf(stderr, "prep socket destruction\n");
+		isc__nmsocket_prep_destroy(sock);
 	}
+fprintf(stderr, "closed\n");
 }
 
 void

lib/isc/netmgr/udp.c

@@ -152,7 +152,8 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
 #endif
 	uv_udp_init_ex(&worker->loop, &sock->uv_handle.udp, uv_init_flags);
 	uv_handle_set_data(&sock->uv_handle.handle, NULL);
-	isc_nmsocket_attach(sock, (isc_nmsocket_t **)&sock->uv_handle.udp.data);
+	isc__nmsocket_attach(sock,
+			     (isc_nmsocket_t **)&sock->uv_handle.udp.data);
 
 	r = uv_udp_open(&sock->uv_handle.udp, sock->fd);
 	if (r == 0) {
@@ -186,7 +187,7 @@ udp_close_cb(uv_handle_t *handle) {
 	isc_nmsocket_t *sock = uv_handle_get_data(handle);
 	atomic_store(&sock->closed, true);
 
-	isc_nmsocket_detach((isc_nmsocket_t **)&sock->uv_handle.udp.data);
+	isc__nmsocket_detach((isc_nmsocket_t **)&sock->uv_handle.udp.data);
 }
 
 static void
@@ -316,12 +317,17 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 	isc_result_t result;
 	isc_nmhandle_t *nmhandle = NULL;
 	isc_sockaddr_t sockaddr;
-	isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)handle);
+	isc_nmsocket_t *sock = NULL;
 	isc_region_t region;
 	uint32_t maxudp;
 	bool free_buf = true;
 
-	REQUIRE(VALID_NMSOCK(sock));
+	/*
+	 * Even though destruction of the socket can only happen from the
+	 * network thread that we're in, we still attach to the socket here
+	 * to ensure it won't be destroyed by the recv callback.
+	 */
+	isc__nmsocket_attach(uv_handle_get_data((uv_handle_t *)handle), &sock);
 
 #ifdef UV_UDP_MMSG_CHUNK
 	free_buf = ((flags & UV_UDP_MMSG_CHUNK) == 0);
@@ -337,6 +343,7 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 		if (free_buf) {
 			isc__nm_free_uvbuf(sock, buf);
 		}
+		isc__nmsocket_detach(&sock);
 		return;
 	}
 
@@ -346,6 +353,7 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 	 */
 	maxudp = atomic_load(&sock->mgr->maxudp);
 	if (maxudp != 0 && (uint32_t)nrecv > maxudp) {
+		isc__nmsocket_detach(&sock);
 		return;
 	}
 
@@ -361,6 +369,11 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 		isc__nm_free_uvbuf(sock, buf);
 	}
 
+	/*
+	 * The sock is now attached to the handle, we can detach our ref.
+	 */
+	isc__nmsocket_detach(&sock);
+
 	/*
 	 * If the recv callback wants to hold on to the handle,
 	 * it needs to attach to it.

lib/isc/win32/libisc.def.in

@@ -464,7 +464,7 @@ isc_nm_tcp_settimeouts
 isc_nm_tcpdns_keepalive
 isc_nm_tcpdns_sequential
 isc_nm_tid
-isc_nmsocket_detach
+isc_nmsocket_close
 isc__nm_acquire_interlocked
 isc__nm_drop_interlocked
 isc__nm_acquire_interlocked_force

lib/ns/interfacemgr.c

@@ -553,11 +553,11 @@ void
 ns_interface_shutdown(ns_interface_t *ifp) {
 	if (ifp->udplistensocket != NULL) {
 		isc_nm_stoplistening(ifp->udplistensocket);
-		isc_nmsocket_detach(&ifp->udplistensocket);
+		isc_nmsocket_close(&ifp->udplistensocket);
 	}
 	if (ifp->tcplistensocket != NULL) {
 		isc_nm_stoplistening(ifp->tcplistensocket);
-		isc_nmsocket_detach(&ifp->tcplistensocket);
+		isc_nmsocket_close(&ifp->tcplistensocket);
 	}
 	if (ifp->clientmgr != NULL) {
 		ns_clientmgr_destroy(&ifp->clientmgr);