From f7bea476c7f6e8489372e138dc60ebcdd92c40c1 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Sun, 25 Mar 2012 21:29:40 -0400 Subject: [PATCH 1/1] windows: ObjectInformationCB.ObjectReferenceCount The ObjectInformationCB.ObjectReferenceCount is protected by the VolumeCB->ObjectInfoTree.TreeLock. When the TreeLock is dropped the reference count can change. Hold the TreeLock across both ObjectReferenceCount == 0 tests and the associated tear down or repeat the ObjectReferenceCount == 0 test after the TreeLock is reacquired. Change-Id: I069c22ae8f3a93fad3ef9a662df5b4903b317897 Reviewed-on: http://gerrit.openafs.org/6959 Tested-by: BuildBot Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman --- src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp | 1 + src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp | 11 +++-- src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp | 65 ++++++++++++++++++++---------- 3 files changed, 50 insertions(+), 27 deletions(-) diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp index af4576b..77bf7e9 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp @@ -644,6 +644,7 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject, if( BooleanFlagOn( pFcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE)) { InterlockedIncrement( &pObjectInfo->ObjectReferenceCount); + ClearFlag( pFcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); AFSPerformObjectInvalidate( pObjectInfo, diff --git a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp index 4bd9ea6..fbdfa31 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp @@ -1958,6 +1958,9 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB) try_return( ntStatus); } + AFSAcquireShared( pVolumeCB->ObjectInfoTree.TreeLock, + TRUE); + if ( AFSIsVolumeFID( &InvalidateCB->FileID)) { @@ -1966,9 +1969,6 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB) else { - AFSAcquireShared( pVolumeCB->ObjectInfoTree.TreeLock, - TRUE); - lCount = InterlockedDecrement( &pVolumeCB->VolumeReferenceCount); AFSDbgLogMsg( AFS_SUBSYSTEM_VOLUME_REF_COUNTING, @@ -1982,9 +1982,6 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB) ntStatus = AFSLocateHashEntry( pVolumeCB->ObjectInfoTree.TreeHead, ullIndex, (AFSBTreeEntry **)&pObjectInfo); - - AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); - } if( pObjectInfo != NULL) @@ -2003,6 +2000,8 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB) lCount); } + AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); + if( !NT_SUCCESS( ntStatus) || pObjectInfo == NULL) { diff --git a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp index 6ad1031..168d060 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp @@ -1184,40 +1184,49 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context) AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); + // + // Dropping the TreeLock permits the + // pCurrentObject->ObjectReferenceCount to change + // + if( AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock, FALSE)) { - if( pCurrentObject->Fcb != NULL) + if ( pCurrentObject->ObjectReferenceCount == 0) { - AFSRemoveFcb( &pCurrentObject->Fcb); - } + if( pCurrentObject->Fcb != NULL) + { - if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB != NULL) - { + AFSRemoveFcb( &pCurrentObject->Fcb); + } - if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb != NULL) + if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB != NULL) { - AFSRemoveFcb( &pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb); - } + if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb != NULL) + { - AFSDeleteObjectInfo( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation); + AFSRemoveFcb( &pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb); + } - ExDeleteResourceLite( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged->Lock); + AFSDeleteObjectInfo( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation); - AFSExFreePool( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged); + ExDeleteResourceLite( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged->Lock); - AFSExFreePool( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB); - } + AFSExFreePool( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged); - AFSDbgLogMsg( AFS_SUBSYSTEM_CLEANUP_PROCESSING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSPrimaryVolumeWorkerThread Deleting deleted object %08lX\n", - pCurrentObject); + AFSExFreePool( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB); + } - AFSDeleteObjectInfo( pCurrentObject); + AFSDbgLogMsg( AFS_SUBSYSTEM_CLEANUP_PROCESSING, + AFS_TRACE_LEVEL_VERBOSE, + "AFSPrimaryVolumeWorkerThread Deleting deleted object %08lX\n", + pCurrentObject); + + AFSDeleteObjectInfo( pCurrentObject); + } AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock); @@ -1431,6 +1440,11 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context) AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); + // + // Dropping the TreeLock permits the + // pCurrentObject->ObjectReferenceCount to change + // + AFSCleanupFcb( pFcb, TRUE); @@ -1462,7 +1476,6 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context) pCurrentObject->FileId.Vnode, pCurrentObject->FileId.Unique); - // // Clear our enumerated flag on this object so we retrieve info again on next access // @@ -1470,6 +1483,8 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context) ClearFlag( pCurrentObject->Flags, AFS_OBJECT_FLAGS_DIRECTORY_ENUMERATED); AFSReleaseResource( pCurrentObject->Specific.Directory.DirectoryNodeHdr.TreeLock); + + AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock); } else { @@ -1480,8 +1495,6 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context) break; } - - AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock); } else { @@ -1536,6 +1549,11 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context) AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); + // + // Dropping the TreeLock permits the + // pCurrentObject->ObjectReferenceCount to change + // + AFSCleanupFcb( pFcb, TRUE); @@ -1572,6 +1590,11 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context) AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); + // + // Dropping the TreeLock permits the + // pCurrentObject->ObjectReferenceCount to change + // + AFSCleanupFcb( pCurrentObject->Fcb, FALSE); -- 1.9.4