Deactivate the handle before sending the async close callback.

We could have a race between handle closing and processing async
callback. Deactivate the handle before issuing the callback - we
have the socket referenced anyway so it's not a problem.

diff --git a/CHANGES b/CHANGES
index 19d0f29adefe32e575bd6d1e3db770868fbfa0d9..2ad5684af73a3031c46c11672d509ffccabcf2a5 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,8 @@
+5370.  [bug]           Deactivation of a netmgr handle associated with a
+                       socket could be skipped in some circumstances.
+                       Fixed by deactivating the netmgr handle before
+                       scheduling the asynchronous close routine. [GL #1700]
+
 5369.  [func]          Add the ability to specify whether or not to wait
                        for nameserver domain names to be looked up, with
                        a new RPZ modifying directive 'nsdname-wait-recurse'.

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 6472115009bdb0717e68857c365ec05bc0e6e1d3..c825a5854dba28e45223b0bee3b92154ad2214bf 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -216,6 +216,7 @@ typedef isc__netievent__socket_t isc__netievent_tcpclose_t;
 typedef isc__netievent__socket_t isc__netievent_tcpdnsclose_t;
 typedef isc__netievent__socket_t isc__netievent_startread_t;
 typedef isc__netievent__socket_t isc__netievent_pauseread_t;
+typedef isc__netievent__socket_t isc__netievent_closecb_t;
 
 typedef struct isc__netievent__socket_req {
        isc__netievent_type type;
@@ -241,8 +242,6 @@ typedef struct isc__netievent__socket_handle {
        isc_nmhandle_t *handle;
 } isc__netievent__socket_handle_t;
 
-typedef isc__netievent__socket_handle_t isc__netievent_closecb_t;
-
 typedef struct isc__netievent_udpsend {
        isc__netievent_type type;
        isc_nmsocket_t *sock;

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 55e75254363af68ac8edecfcf439284ab6790e44..f4b784862fabdb92472b8976cad64e3673f4923a 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -1164,7 +1164,15 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) {
        }
 
        /*
-        * The handle is closed. If the socket has a callback configured
+        * 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);
+
+       /*
+        * The handle is gone now. If the socket has a callback configured
         * for that (e.g., to perform cleanup after request processing),
         * call it now, or schedule it to run asynchronously.
         */
@@ -1174,27 +1182,16 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) {
                } else {
                        isc__netievent_closecb_t *event = isc__nm_get_ievent(
                                sock->mgr, netievent_closecb);
+                       /*
+                        * The socket will be finally detached by the closecb
+                        * event handler.
+                        */
                        isc_nmsocket_attach(sock, &event->sock);
-                       event->handle = handle;
                        isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
                                               (isc__netievent_t *)event);
-
-                       /*
-                        * If we're doing this asynchronously, then the
-                        * async event will take care of cleaning up the
-                        * handle and closing the socket.
-                        */
-                       return;
                }
        }
 
-       /*
-        * 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);
        isc_nmsocket_detach(&tmp);
 }
 
@@ -1387,8 +1384,6 @@ isc__nm_async_closecb(isc__networker_t *worker, isc__netievent_t *ev0) {
 
        UNUSED(worker);
 
-       nmhandle_deactivate(ievent->sock, ievent->handle);
-
        ievent->sock->closehandle_cb(ievent->sock);
        isc_nmsocket_detach(&ievent->sock);
 }