Windows: reorg open handle counts and Fcb->NPFcb->Resource

Reorganize when open handle counts are decremented in order
to avoid a race with worker threads performing garbage collection.

Change-Id: I07c1c5e80fad48cd3439dbc9c85bd6dff9b9bf44
Reviewed-on: http://gerrit.openafs.org/6504
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Peter Scott <pscott@kerneldrivers.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp
index 69096ec..70ceb02 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp
@@ -162,14 +162,6 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
 
                 ASSERT( pFcb->OpenHandleCount != 0);
 
-                InterlockedDecrement( &pFcb->OpenHandleCount);
-
-                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                              AFS_TRACE_LEVEL_VERBOSE,
-                              "AFSCleanup (IOCtl) Decrement handle count on Fcb %08lX Cnt %d\n",
-                              pFcb,
-                              pFcb->OpenHandleCount);
-
                 //
                 // Decrement the open child handle count
                 //
@@ -187,6 +179,14 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                                   pObjectInfo->ParentObjectInformation->Specific.Directory.ChildOpenHandleCount);
                 }
 
+                InterlockedDecrement( &pFcb->OpenHandleCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSCleanup (IOCtl) Decrement handle count on Fcb %08lX Cnt %d\n",
+                              pFcb,
+                              pFcb->OpenHandleCount);
+
                 //
                 // And finally, release the Fcb if we acquired it.
                 //
@@ -291,14 +291,6 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
 
                 ASSERT( pFcb->OpenHandleCount != 0);
 
-                InterlockedDecrement( &pFcb->OpenHandleCount);
-
-                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                              AFS_TRACE_LEVEL_VERBOSE,
-                              "AFSCleanup (File) Decrement handle count on Fcb %08lX Cnt %d\n",
-                              pFcb,
-                              pFcb->OpenHandleCount);
-
                 if( pFcb->ObjectInformation->ParentObjectInformation != NULL)
                 {
 
@@ -354,11 +346,12 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                 }
 
                 //
-                // If the count has dropped to zero and there is a pending delete
-                // then delete the node
+                // If the count has dropped to one and there is a pending delete
+                // then delete the node.  The final count will be decremented just
+                // before the Fcb->NPFcb->Resource is released.
                 //
 
-                if( pFcb->OpenHandleCount == 0 &&
+                if( pFcb->OpenHandleCount == 1 &&
                     BooleanFlagOn( pCcb->DirectoryCB->Flags, AFS_DIR_ENTRY_PENDING_DELETE))
                 {
 
@@ -520,7 +513,7 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                                          &pCcb->AuthGroup);
                     }
 
-                    if( pFcb->OpenHandleCount == 0)
+                    if( pFcb->OpenHandleCount == 1)
                     {
 
                         //
@@ -590,6 +583,14 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                                   pObjectInfo->ParentObjectInformation->Specific.Directory.ChildOpenHandleCount);
                 }
 
+                InterlockedDecrement( &pFcb->OpenHandleCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSCleanup (File) Decrement handle count on Fcb %08lX Cnt %d\n",
+                              pFcb,
+                              pFcb->OpenHandleCount);
+
                 //
                 // And finally, release the Fcb if we acquired it.
                 //
@@ -639,14 +640,6 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
 
                 ASSERT( pFcb->OpenHandleCount != 0);
 
-                InterlockedDecrement( &pFcb->OpenHandleCount);
-
-                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                              AFS_TRACE_LEVEL_VERBOSE,
-                              "AFSCleanup (Dir) Decrement handle count on Fcb %08lX Cnt %d\n",
-                              pFcb,
-                              pFcb->OpenHandleCount);
-
                 if( pFcb->ObjectInformation->ParentObjectInformation != NULL)
                 {
 
@@ -686,11 +679,12 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                 }
 
                 //
-                // If the count has dropped to zero and there is a pending delete
-                // then delete the node
+                // If the count has dropped to one and there is a pending delete
+                // then delete the node.  The final count will be decremented just
+                // before the Fcb->NPFcb->Resource is released.
                 //
 
-                if( pFcb->OpenHandleCount == 0 &&
+                if( pFcb->OpenHandleCount == 1 &&
                     BooleanFlagOn( pCcb->DirectoryCB->Flags, AFS_DIR_ENTRY_PENDING_DELETE))
                 {
 
@@ -871,6 +865,14 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                                   pObjectInfo->ParentObjectInformation->Specific.Directory.ChildOpenHandleCount);
                 }
 
+                InterlockedDecrement( &pFcb->OpenHandleCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSCleanup (Dir) Decrement handle count on Fcb %08lX Cnt %d\n",
+                              pFcb,
+                              pFcb->OpenHandleCount);
+
                 //
                 // And finally, release the Fcb if we acquired it.
                 //
@@ -905,14 +907,6 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
 
                 ASSERT( pFcb->OpenHandleCount != 0);
 
-                InterlockedDecrement( &pFcb->OpenHandleCount);
-
-                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                              AFS_TRACE_LEVEL_VERBOSE,
-                              "AFSCleanup (MP/SL) Decrement handle count on Fcb %08lX Cnt %d\n",
-                              pFcb,
-                              pFcb->OpenHandleCount);
-
                 if( pFcb->ObjectInformation->ParentObjectInformation != NULL)
                 {
 
@@ -952,11 +946,12 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                 }
 
                 //
-                // If the count has dropped to zero and there is a pending delete
-                // then delete the node
+                // If the count has dropped to one and there is a pending delete
+                // then delete the node.  The final count will be decremented just
+                // before the Fcb->NPFcb->Resource is released.
                 //
 
-                if( pFcb->OpenHandleCount == 0 &&
+                if( pFcb->OpenHandleCount == 1 &&
                     BooleanFlagOn( pCcb->DirectoryCB->Flags, AFS_DIR_ENTRY_PENDING_DELETE))
                 {
 
@@ -1129,6 +1124,14 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                                   pObjectInfo->ParentObjectInformation->Specific.Directory.ChildOpenHandleCount);
                 }
 
+                InterlockedDecrement( &pFcb->OpenHandleCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSCleanup (Share) Decrement handle count on Fcb %08lX Cnt %d\n",
+                              pFcb,
+                              pFcb->OpenHandleCount);
+
                 //
                 // And finally, release the Fcb if we acquired it.
                 //
@@ -1152,14 +1155,6 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
 
                 ASSERT( pFcb->OpenHandleCount != 0);
 
-                InterlockedDecrement( &pFcb->OpenHandleCount);
-
-                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                              AFS_TRACE_LEVEL_VERBOSE,
-                              "AFSCleanup (Share) Decrement handle count on Fcb %08lX Cnt %d\n",
-                              pFcb,
-                              pFcb->OpenHandleCount);
-
                 //
                 // Decrement the open child handle count
                 //
@@ -1177,6 +1172,14 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                                   pObjectInfo->ParentObjectInformation->Specific.Directory.ChildOpenHandleCount);
                 }
 
+                InterlockedDecrement( &pFcb->OpenHandleCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSCleanup (MP/SL) Decrement handle count on Fcb %08lX Cnt %d\n",
+                              pFcb,
+                              pFcb->OpenHandleCount);
+
                 //
                 // And finally, release the Fcb if we acquired it.
                 //

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
index b39ade7..129b9ac 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
@@ -1102,6 +1102,18 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                 if( pCurrentObject->Fcb != NULL)
                                 {
 
+                                    //
+                                    // Acquire and drop the Fcb resource to synchronize
+                                    // with a potentially active AFSCleanup() which sets
+                                    // the OpenReferenceCount to zero while holding the
+                                    // resource.
+                                    //
+
+                                    AFSAcquireExcl( &pCurrentObject->Fcb->NPFcb->Resource,
+                                                    TRUE);
+
+                                    AFSReleaseResource( &pCurrentObject->Fcb->NPFcb->Resource);
+
                                     AFSRemoveFcb( pCurrentObject->Fcb);
                                 }
 
@@ -1300,6 +1312,18 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                                                TRUE);
                                             }
 
+                                            //
+                                            // Acquire and drop the Fcb resource to synchronize
+                                            // with a potentially active AFSCleanup() which sets
+                                            // the OpenReferenceCount to zero while holding the
+                                            // resource.
+                                            //
+
+                                            AFSAcquireExcl( &pCurrentChildObject->Fcb->NPFcb->Resource,
+                                                            TRUE);
+
+                                            AFSReleaseResource( &pCurrentChildObject->Fcb->NPFcb->Resource);
+
                                             AFSRemoveFcb( pCurrentChildObject->Fcb);
                                         }
 
@@ -1429,6 +1453,9 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                 if( pCurrentObject->Fcb != NULL)
                                 {
 
+                                    AFSCleanupFcb( pCurrentObject->Fcb,
+                                                   TRUE);
+
                                     //
                                     // Acquire and drop the Fcb resource to synchronize
                                     // with a potentially active AFSCleanup() which sets
@@ -1441,9 +1468,6 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
 
                                     AFSReleaseResource( &pCurrentObject->Fcb->NPFcb->Resource);
 
-                                    AFSCleanupFcb( pCurrentObject->Fcb,
-                                                   TRUE);
-
                                     AFSRemoveFcb( pCurrentObject->Fcb);
                                 }