allow tcpdns sockets to self-reference while connected

A TCPDNS socket creates a handle for each complete DNS message.

Previously, when all the handles were disconnected, the socket
would be closed, but the wrapped TCP socket might still have
more to read.

Now, when a connection is established, the TCPDNS socket creates
a reference to itself by attaching itself to sock->self. This
reference isn't cleared until the connection is closed via
EOF, timeout, or server shutdown. This allows the socket to remain
open even when there are no active handles for it.

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index d066198bd012f2c72a18545cad997e607674f960..2a38b5b53257c7aeda85d80c3bc95a2e47badd37 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/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

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 1a4a82c831a102caaeefe10b49a7d456d3606c7c..a1e6988710875e48eaa1b3cdac5e2ad22919c776 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -1184,11 +1184,6 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) {
                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.
-        */
        nmhandle_deactivate(sock, handle);
 
        /*

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index d879fceeafb2fb6eaf7a020fe6b905726a495086..f2b487d2962ec3f03fdbbdcabf5cf02e44ead3d3 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -511,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

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 6ae5c111cf5725fa71216bffa4f25b1f6add9a7e..c7d23e96a99cff01ab78256766ac468dda893324 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -82,6 +82,8 @@ 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));
+       atomic_store(&sock->closed, true);
+       tcpdns_close_direct(sock);
 }
 
 static void
@@ -91,7 +93,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;
 }
 
 /*
@@ -123,6 +127,8 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
        dnssock->extrahandlesize = dnslistensock->extrahandlesize;
        isc__nmsocket_attach(dnslistensock, &dnssock->listener);
 
+       isc__nmsocket_attach(dnssock, &dnssock->self);
+
        dnssock->outerhandle = handle;
        isc_nmhandle_ref(dnssock->outerhandle);
 
@@ -137,12 +143,12 @@ 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_nmhandle_ref(handle);
        result = isc_nm_read(handle, dnslisten_readcb, dnssock);
        if (result != ISC_R_SUCCESS) {
                isc_nmhandle_unref(handle);
        }
+       isc__nmsocket_detach(&dnssock);
 }
 
 /*
@@ -194,11 +200,6 @@ 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);
        }
@@ -223,7 +224,10 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) {
 
        if (region == NULL) {
                /* Connection closed */
-               isc__nm_tcpdns_close(dnssock);
+               isc_nmhandle_unref(handle);
+               if (dnssock->self != NULL) {
+                       isc__nmsocket_detach(&dnssock->self);
+               }
                return;
        }
 
@@ -500,6 +504,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
        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),
@@ -514,6 +519,7 @@ 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());
+
        /* We don't need atomics here, it's all in single network thread */
        if (sock->timer_initialized) {
                /*
@@ -524,6 +530,8 @@ tcpdns_close_direct(isc_nmsocket_t *sock) {
                sock->timer_initialized = false;
                uv_timer_stop(&sock->timer);
                uv_close((uv_handle_t *)&sock->timer, timer_close_cb);
+       } else if (sock->self != NULL) {
+               isc__nmsocket_detach(&sock->self);
        } else {
                /*
                 * At this point we're certain that there are no external