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 dfb4b0154b40c5b527a6898f703a10b3e3f8ad5e..106e0b1f5cc9ff01ed81a406b0b0f9c1ddc96b53 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]
+
 5368.  [bug]           Named failed to restart if 'rndc addzone' names
                        contained special characters (e.g. '/'). [GL #1655]
 

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 481818857001f8b1a2284257f475eed65128b768..227527a4e1e8be806381b255cc05661b63ef6518 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -214,6 +214,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;
@@ -239,8 +240,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 9f49dfbd9fbfd75f57f3d4834e09496b0d5cc00f..874494ba2a9a21a01238137cace8b94355dda596 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);
 }
 
@@ -1331,8 +1328,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);
 }