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.

(cherry picked from commit cd79b4953800daf87b6bfcea0b6f3ce3bec29988)

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 4b54995c3fd97eb9c1b8143214ba1d3782fe48ea..f6d8417f6d8113f12fe7a5394caedd93d7f55675 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 5dd3dee4b070d66151bba7879298dabd53572f33..3f23eff056441ee8364d7c904b3001b9c469388a 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 9c4b5d93177a1c39f5768389654bc8e65fb80669..b23860f5b5d5233c42a462ff719f7c5966572a96 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 a52ef1268064ac9a0f9b91388bdcf5f0504fc63c..15e3ced2fa1b6d20f7a00166418a372473923df5 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