Don't detach self until we're closing

- 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

@@ -371,6 +371,8 @@ struct isc_nmsocket {
 	isc_nmsocket_t *parent;
 	/*% Listener socket this connection was accepted on */
 	isc_nmsocket_t *listener;
+	/*% Self, for self-contained unreferenced sockets (tcpdns) */
+	isc_nmsocket_t *self;
 
 	/*%
 	 * quota is the TCP client, attached when a TCP connection
@@ -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,9 @@ 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.
+ */

lib/isc/netmgr/netmgr.c

@@ -32,6 +32,10 @@
 
 #include "netmgr-int.h"
 #include "uv-compat.h"
+#include <execinfo.h>
+
+#define BACKTRACE() do { void *arr[4]; backtrace_symbols_fd(arr, backtrace(arr, 4), 2); fprintf(stderr, "\n"); } while(0)
+
 
 /*%
  * How many isc_nmhandles and isc_nm_uvreqs will we be
@@ -129,6 +133,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,16 +711,19 @@ 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);
 	}
+	BACKTRACE();
 
 	*target = sock;
 }
@@ -736,9 +764,17 @@ 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);
+	BACKTRACE();
+		sock->outerhandle = NULL;
+	}
+
+	if (sock->outer != NULL) {
+		isc__nmsocket_detach(&sock->outer);
 	}
 
 	while ((handle = isc_astack_pop(sock->inactivehandles)) != NULL) {
@@ -878,7 +914,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,16 +927,30 @@ 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;
 	}
+	BACKTRACE();
 
 	if (isc_refcount_decrement(&rsock->references) == 1) {
 		isc__nmsocket_prep_destroy(rsock);
 	}
 }
 
+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 +961,8 @@ 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));
+	BACKTRACE();
 
 	*sock = (isc_nmsocket_t){ .type = type,
 				  .iface = iface,
@@ -1039,7 +1091,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 +1147,8 @@ 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);
+	BACKTRACE();
 	isc_refcount_increment(&handle->references);
 }
 
@@ -1149,29 +1204,32 @@ 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);
+	BACKTRACE();
 	if (isc_refcount_decrement(&handle->references) > 1) {
 		return;
 	}
 	/* We need an acquire memory barrier here */
 	(void)isc_refcount_current(&handle->references);
 
+	/*
+	 * XXX: Do we need to reference the socket to ensure that it
+	 * can't be deleted by another thread while we're deactivating
+	 * the handle?
+	 */
 	sock = handle->sock;
 	handle->sock = NULL;
+fprintf(stderr, "handle %p sock %p set to NULL\n", handle, sock);
+	BACKTRACE();
 
 	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);
 
 	/*
@@ -1189,13 +1247,13 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) {
 			 * The socket will be finally detached by the closecb
 			 * event handler.
 			 */
-			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(&tmp);
+	isc__nmsocket_detach(&sock);
 }
 
 void *
@@ -1262,7 +1320,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);
@@ -1299,7 +1357,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
@@ -1388,7 +1446,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

@@ -128,14 +128,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,
@@ -146,6 +147,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:
@@ -154,9 +168,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
@@ -170,8 +183,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;
@@ -209,7 +222,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);
 	}
 }
@@ -379,14 +392,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:
@@ -405,7 +428,7 @@ error:
 	/*
 	 * Detach the socket properly to make sure uv_close() is called.
 	 */
-	isc_nmsocket_detach(&csock);
+	isc__nmsocket_detach(&csock);
 }
 
 void
@@ -416,7 +439,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);
 }
@@ -462,7 +485,7 @@ tcp_listenclose_cb(uv_handle_t *handle) {
 	sock->pquota = NULL;
 	UNLOCK(&sock->lock);
 
-	isc_nmsocket_detach(&sock);
+	isc__nmsocket_detach(&sock);
 }
 
 static void
@@ -488,7 +511,9 @@ readtimeout_cb(uv_timer_t *handle) {
 	if (sock->quota) {
 		isc_quota_detach(&sock->quota);
 	}
-	sock->rcb.recv(sock->tcphandle, NULL, sock->rcbarg);
+	if (sock->rcb.recv != NULL) {
+		sock->rcb.recv(sock->tcphandle, NULL, sock->rcbarg);
+	}
 }
 
 isc_result_t
@@ -697,7 +722,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);
 }
 
 /*
@@ -741,7 +766,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 		 * we need to - but we risk a race then.)
 		 */
 		isc_nmsocket_t *tsock = NULL;
-		isc_nmsocket_attach(ssock, &tsock);
+		isc__nmsocket_attach(ssock, &tsock);
 		result = isc_quota_attach_cb(ssock->pquota, &quota,
 					     &ssock->quotacb);
 		if (result == ISC_R_QUOTA) {
@@ -754,7 +779,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 		 * We're under quota, so there's no need to wait;
 		 * Detach the socket.
 		 */
-		isc_nmsocket_detach(&tsock);
+		isc__nmsocket_detach(&tsock);
 	}
 
 	isc__nm_incstats(ssock->mgr, ssock->statsindex[STATID_ACCEPT]);
@@ -915,7 +940,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);
 }
 
@@ -933,7 +960,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);
 	}

lib/isc/netmgr/tcpdns.c

@@ -82,7 +82,9 @@ 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);
+	atomic_store(&sock->closed, true);
+	tcpdns_close_direct(sock);
 }
 
 static void
@@ -92,7 +94,9 @@ dnstcp_readtimeout(uv_timer_t *timer) {
 
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
-	tcpdns_close_direct(sock);
+	/* Close the TCP connection, it's closing should fire 'our' closing */
+	isc_nmhandle_unref(sock->outerhandle);
+	sock->outerhandle = NULL;
 }
 
 /*
@@ -103,6 +107,7 @@ 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;
 
+fprintf(stderr, "dnslisten_acceptcb %p %s\n", handle, isc_result_totext(result));
 	REQUIRE(VALID_NMSOCK(dnslistensock));
 	REQUIRE(dnslistensock->type == isc_nm_tcpdnslistener);
 
@@ -122,8 +127,13 @@ 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);
+
+	isc__nmsocket_attach(dnssock, &dnssock->self);
+
+	dnssock->outerhandle = handle;
+	isc_nmhandle_ref(dnssock->outerhandle);
+
 	dnssock->peer = handle->sock->peer;
 	dnssock->read_timeout = handle->sock->mgr->init;
 	dnssock->tid = isc_nm_tid();
@@ -135,8 +145,13 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	dnssock->timer_initialized = true;
 	uv_timer_start(&dnssock->timer, dnstcp_readtimeout,
 		       dnssock->read_timeout, 0);
-
-	isc_nm_read(handle, dnslisten_readcb, dnssock);
+	fprintf(stderr, "dnssock->refs %lu\n", dnssock->references);
+	isc_nmhandle_ref(handle);
+	result = isc_nm_read(handle, dnslisten_readcb, dnssock);
+	if (result != ISC_R_SUCCESS) {
+		isc_nmhandle_unref(handle);
+	}
+	isc__nmsocket_detach(&dnssock);
 }
 
 /*
@@ -210,9 +225,13 @@ 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);
+		isc_nmhandle_unref(handle);
+		if (dnssock->self != NULL) {
+			isc__nmsocket_detach(&dnssock->self);
+		};
 		return;
 	}
 
@@ -234,6 +253,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 +264,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 +275,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 +287,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 +330,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 +346,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 +356,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 +369,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 +378,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 {
@@ -382,13 +404,13 @@ resume_processing(void *arg) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
 
-	if (sock->type != isc_nm_tcpdnssocket || sock->outer == NULL) {
+	if (sock->type != isc_nm_tcpdnssocket || sock->outerhandle == NULL) {
 		return;
 	}
 
 	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,14 +425,17 @@ 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 if (sock->outerhandle != NULL) {
+			isc_nm_resumeread(sock->outerhandle->sock);
+			
 		}
 
 		return;
@@ -424,12 +449,13 @@ resume_processing(void *arg) {
 		isc_nmhandle_t *dnshandle = NULL;
 
 		result = processbuffer(sock, &dnshandle);
+fprintf(stderr, "processed nonsequentially sock %p with handle %p, result %s\n", sock, dnshandle, isc_result_totext(result));
 		if (result != ISC_R_SUCCESS) {
 			/*
 			 * 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);
 			}
 
 			break;
@@ -438,22 +464,25 @@ resume_processing(void *arg) {
 		if (sock->timer_initialized) {
 			uv_timer_stop(&sock->timer);
 		}
-		atomic_store(&sock->outer->processing, true);
+		atomic_store(&sock->outerhandle->sock->processing, true);
 		isc_nmhandle_unref(dnshandle);
 	} while (atomic_load(&sock->ah) < TCPDNS_CLIENTS_PER_CONN);
+
+
 }
 
 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 +500,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,12 +509,13 @@ 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);
 	t->orighandle = handle;
 	isc_nmhandle_ref(t->orighandle);
+	isc_nmhandle_ref(t->handle);
 
 	t->region = (isc_region_t){ .base = isc_mem_get(t->mctx,
 							region->length + 2),
@@ -500,6 +530,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) %lu\n", sock, sock->references);
 	/* We don't need atomics here, it's all in single network thread */
 	if (sock->timer_initialized) {
 		/*
@@ -509,21 +541,31 @@ 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 if (sock->self != NULL) {
+		isc__nmsocket_detach(&sock->self);
 	} 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->outerhandle != NULL) {
+fprintf(stderr, "clearing outerhandle\n");
+			sock->outerhandle->sock->rcb.recv = NULL;
+			isc_nmhandle_unref(sock->outerhandle);
+  			sock->outerhandle = NULL;
 		}
 		if (sock->listener != NULL) {
-			isc_nmsocket_detach(&sock->listener);
+fprintf(stderr, "clearing listener\n");
+			isc__nmsocket_detach(&sock->listener);
 		}
 		atomic_store(&sock->closed, true);
+		isc__nmsocket_prep_destroy(sock);
+fprintf(stderr, "prep socket destruction\n");
 	}
+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);