Windows: PrimaryVolumeWorker ObjectInfoLock deadlock

Patchset eaad522651a81f20eac4966a55a731e0e59e39dd inadvertently
introduced a deadlock with invalidation requests from the service.
It is not safe to hold the ObjectInfoLock resource across calls
to AFSCleanupFcb().  Instead of holding the lock obtain a reference
to the ObjectInformationCB.

Change-Id: I048401ec3e432c05c8a72251ef1e32442974256d
Reviewed-on: http://gerrit.openafs.org/8308
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
index cf28e9f..fa8d080 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
@@ -1398,10 +1398,9 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                     // with an invalidation call from the service during AFSCleanupFcb
                                     //
 
-                                    AFSAcquireShared( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock,
-                                                      TRUE);
+                                    lCount = AFSObjectInfoIncrement( pCurrentChildObject);
 
-                                    if( pCurrentChildObject->ObjectReferenceCount <= 0 &&
+                                    if( lCount == 1 &&
                                         pCurrentChildObject->Fcb != NULL &&
                                         pCurrentChildObject->FileType == AFS_FILE_TYPE_FILE)
                                     {
@@ -1415,6 +1414,10 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                         AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
 
                                         //
+                                        // Cannot hold a TreeLock across an AFSCleanupFcb call
+                                        // as it can deadlock with an invalidation ioctl initiated
+                                        // from the service.
+                                        //
                                         // Dropping the TreeLock permits the
                                         // pCurrentObject->ObjectReferenceCount to change
                                         //
@@ -1432,7 +1435,7 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                                         TRUE);
                                     }
 
-                                    AFSReleaseResource( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock);
+                                    lCount = AFSObjectInfoDecrement( pCurrentChildObject);
 
                                     AFSAcquireExcl( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock,
                                                     TRUE);
@@ -1566,7 +1569,10 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
 
                         AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
 
-                        if( pCurrentObject->Fcb != NULL)
+                        lCount = AFSObjectInfoIncrement( pCurrentObject);
+
+                        if( lCount == 0 &&
+                            pCurrentObject->Fcb != NULL)
                         {
 
                             //
@@ -1584,6 +1590,8 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                             }
                         }
 
+                        lCount = AFSObjectInfoDecrement( pCurrentObject);
+
                         if( !AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock,
                                              FALSE))
                         {