Simplify netmgr active handles accounting

The active handles accounting was both using atomic counter and ISC_LIST
to keep track of active handles.  Remove the atomic counter that was in
use before the ISC_LIST was added for better tracking of the handles
attached to the socket.

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 6f7fc10a5644567c6e4974494afe21867f6dd267..ef48f89eb0c52fe6bfafbae32b47983cc0efde64 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -991,11 +991,6 @@ struct isc_nmsocket {
         */
        isc_result_t result;
 
-       /*%
-        * Current number of active handles.
-        */
-       atomic_int_fast32_t ah;
-
        /*%
         * This function will be called with handle->sock
         * as the argument whenever a handle's references drop

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index a632a1f715ff84d00bd6a5e777b500895249a5c5..de66cdb9dcdd29a340c8d785bc7198b1170a0734 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -587,7 +587,9 @@ isc___nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target FLARG) {
  * Free all resources inside a socket (including its children if any).
  */
 static void
-nmsocket_cleanup(isc_nmsocket_t *sock) {
+nmsocket_cleanup(void *arg) {
+       isc_nmsocket_t *sock = arg;
+
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(!isc__nmsocket_active(sock));
 
@@ -666,11 +668,26 @@ nmsocket_cleanup(isc_nmsocket_t *sock) {
        isc__networker_detach(&worker);
 }
 
+static bool
+nmsocket_has_active_handles(isc_nmsocket_t *sock) {
+       if (!ISC_LIST_EMPTY(sock->active_handles)) {
+               return (true);
+       }
+
+       if (sock->children != NULL) {
+               for (size_t i = 0; i < sock->nchildren; i++) {
+                       isc_nmsocket_t *csock = &sock->children[i];
+                       if (!ISC_LIST_EMPTY(csock->active_handles)) {
+                               return (true);
+                       }
+               }
+       }
+
+       return (false);
+}
+
 static void
 nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) {
-       int active_handles;
-       bool destroy = false;
-
        NETMGR_TRACE_LOG("%s():%p->references = %" PRIuFAST32 "\n", __func__,
                         sock, isc_refcount_current(&sock->references));
 
@@ -684,39 +701,29 @@ nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) {
                return;
        }
 
-       /*
-        * This is a parent socket (or a standalone). See whether the
-        * children have active handles before deciding whether to
-        * accept destruction.
-        */
        if (atomic_load(&sock->active) || atomic_load(&sock->destroying) ||
            !atomic_load(&sock->closed) || atomic_load(&sock->references) != 0)
        {
                return;
        }
 
-       active_handles = atomic_load(&sock->ah);
-       if (sock->children != NULL) {
-               for (size_t i = 0; i < sock->nchildren; i++) {
-                       active_handles += atomic_load(&sock->children[i].ah);
-               }
-       }
+       NETMGR_TRACE_LOG("%s:%p->statichandle = %p\n", __func__, sock,
+                        sock->statichandle);
 
-       if (active_handles == 0 || sock->statichandle != NULL) {
-               destroy = true;
+       /*
+        * This is a parent socket (or a standalone). See whether the
+        * children have active handles before deciding whether to
+        * accept destruction.
+        */
+       if (sock->statichandle == NULL && nmsocket_has_active_handles(sock)) {
+               return;
        }
 
-       NETMGR_TRACE_LOG("%s:%p->active_handles = %d, .statichandle = %p\n",
-                        __func__, sock, active_handles, sock->statichandle);
-
-       if (destroy) {
-               atomic_store(&sock->destroying, true);
-               if (sock->tid == isc_tid()) {
-                       nmsocket_cleanup(sock);
-               } else {
-                       isc_async_run(sock->worker->loop,
-                                     (isc_job_cb)nmsocket_cleanup, sock);
-               }
+       atomic_store(&sock->destroying, true);
+       if (sock->tid == isc_tid()) {
+               nmsocket_cleanup(sock);
+       } else {
+               isc_async_run(sock->worker->loop, nmsocket_cleanup, sock);
        }
 }
 
@@ -921,7 +928,6 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc__networker_t *worker,
        atomic_init(&sock->listening, 0);
        atomic_init(&sock->closed, 0);
        atomic_init(&sock->destroying, 0);
-       atomic_init(&sock->ah, 0);
        atomic_init(&sock->client, 0);
        atomic_init(&sock->connecting, false);
        atomic_init(&sock->keepalive, false);
@@ -1025,8 +1031,6 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t const *peer,
                handle->local = sock->iface;
        }
 
-       (void)atomic_fetch_add(&sock->ah, 1);
-
        ISC_LIST_APPEND(sock->active_handles, handle, active_link);
 
        switch (sock->type) {
@@ -1088,32 +1092,15 @@ static void
 nmhandle_free(isc_nmsocket_t *sock, isc_nmhandle_t *handle) {
        isc_refcount_destroy(&handle->references);
 
+       handle->magic = 0;
+
        if (handle->dofree != NULL) {
                handle->dofree(handle->opaque);
        }
 
-       *handle = (isc_nmhandle_t){ .magic = 0 };
-
        isc_mem_put(sock->worker->mctx, handle, sizeof(isc_nmhandle_t));
 }
 
-static void
-nmhandle_deactivate(isc_nmsocket_t *sock, isc_nmhandle_t *handle) {
-       uint_fast32_t ah = atomic_fetch_sub(&sock->ah, 1);
-       INSIST(ah > 0);
-
-       ISC_LIST_UNLINK(sock->active_handles, handle, active_link);
-
-#if !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__
-       if (atomic_load(&sock->active)) {
-               ISC_LIST_APPEND(sock->inactive_handles, handle, inactive_link);
-       } else
-#endif /* !__SANITIZE_ADDRESS__ && !__SANITIZE_THREAD__ */
-       {
-               nmhandle_free(sock, handle);
-       }
-}
-
 static void
 isc__nm_closehandle_job(void *arg) {
        isc_nmsocket_t *sock = arg;
@@ -1144,7 +1131,17 @@ nmhandle_destroy(isc_nmhandle_t *handle) {
                sock->statichandle = NULL;
        }
 
-       nmhandle_deactivate(sock, handle);
+       ISC_LIST_UNLINK(sock->active_handles, handle, active_link);
+
+#if defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_THREAD__)
+       nmhandle_free(sock, handle);
+#else
+       if (atomic_load(&sock->active)) {
+               ISC_LIST_APPEND(sock->inactive_handles, handle, inactive_link);
+       } else {
+               nmhandle_free(sock, handle);
+       }
+#endif
 
        /*
         * The handle is gone now. If the socket has a callback configured

diff --git a/lib/isc/netmgr/streamdns.c b/lib/isc/netmgr/streamdns.c
index ef5fd6874ce71dcab1a537e8567b584f1d55b330..1905364ad053f5f069af984d8dff6a419bde4138 100644 (file)
--- a/lib/isc/netmgr/streamdns.c
+++ b/lib/isc/netmgr/streamdns.c
@@ -101,7 +101,9 @@ streamdns_resumeread(isc_nmsocket_t *sock, isc_nmhandle_t *transphandle) {
 static void
 streamdns_readmore(isc_nmsocket_t *sock, isc_nmhandle_t *transphandle) {
        streamdns_resumeread(sock, transphandle);
-       if (sock->streamdns.reading && atomic_load(&sock->ah) == 1) {
+
+       isc_nmhandle_t *handle = ISC_LIST_HEAD(sock->active_handles);
+       if (handle != NULL && ISC_LIST_NEXT(handle, active_link) == NULL) {
                isc__nmsocket_timer_start(sock);
        }
 }