jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-21 00:10:15 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Windows: FCB cleanup must be done before ObjectInfo

Browse Source 
When processing the cleanup and destruction of a File Control Block
the related ObjectInfoCB is required for proper cleanup.  Reorganize
the AFSPrimaryVolumeWorkerThread logic to ensure that this is true.
This involves dropping the VolumeCB->ObjectInfoTree.TreeLock around
the AFSCleanupFcb() call. While the lock is released it is possible
for the ObjectInfoCB->OpenReferenceCount to change.  Therefore, new
checks must be added after the lock is re-acquired to ensure that
an in-use object is not destroyed.

Change-Id: I6b26fb2fe1ef4077c6edd643ec40715c8e2928ac
Reviewed-on: http://gerrit.openafs.org/7327
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>
This commit is contained in:
Jeffrey Altman 2012-05-03 20:01:22 -04:00 committed by Jeffrey Altman
parent e691a757d6
commit f76cf9a3fb
2 changed files with 62 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
View File

@ -7153,7 +7153,7 @@ AFSCleanupFcb( IN AFSFcb *Fcb,
AFSReleaseResource( &Fcb->NPFcb->Resource);
if( Fcb->OpenReferenceCount == 0)
if( Fcb->OpenReferenceCount <= 0)
{
//

148
src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
View File

@ -1387,15 +1387,38 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
AFSDeleteDirEntry( pCurrentObject,
pCurrentDirEntry);
if( pCurrentChildObject->ObjectReferenceCount <= 0 &&
pCurrentChildObject->Fcb != NULL &&
pCurrentChildObject->FileType == AFS_FILE_TYPE_FILE)
{
//
// We must not hold pVolumeCB->ObjectInfoTree.TreeLock exclusive
// across an AFSCleanupFcb call since it can deadlock with an
// invalidation call from the service.
//
AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
//
// Dropping the TreeLock permits the
// pCurrentObject->ObjectReferenceCount to change
//
AFSCleanupFcb( pCurrentChildObject->Fcb,
TRUE);
AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock,
TRUE);
}
if( pCurrentChildObject->ObjectReferenceCount <= 0)
{
if( pCurrentChildObject->Fcb != NULL)
{
pFcb = (AFSFcb *) InterlockedCompareExchangePointer( (PVOID *)&pCurrentChildObject->Fcb, NULL, (PVOID)pCurrentChildObject->Fcb);
lFileType = pCurrentChildObject->FileType;
AFSRemoveFcb( &pCurrentChildObject->Fcb);
}
if( pCurrentChildObject->FileType == AFS_FILE_TYPE_DIRECTORY &&
@ -1427,33 +1450,6 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
pCurrentDirEntry = pNextDirEntry;
if ( pFcb != NULL)
{
if( lFileType == AFS_FILE_TYPE_FILE)
{
//
// We must not hold pVolumeCB->ObjectInfoTree.TreeLock exclusive
// across an AFSCleanupFcb call since it can deadlock with an
// invalidation call from the service.
//
AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
//
// Dropping the TreeLock permits the
// pCurrentObject->ObjectReferenceCount to change
//
AFSCleanupFcb( pFcb,
TRUE);
AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock,
TRUE);
}
AFSRemoveFcb( &pFcb);
}
}
pCurrentObject->Specific.Directory.DirectoryNodeListHead = NULL;
@ -1536,71 +1532,49 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
else if( pCurrentObject->FileType == AFS_FILE_TYPE_FILE)
{
if( BooleanFlagOn( pCurrentObject->Flags, AFS_OBJECT_FLAGS_DELETED) &&
pCurrentObject->ObjectReferenceCount <= 0 &&
( pCurrentObject->Fcb == NULL ||
pCurrentObject->Fcb->OpenReferenceCount == 0))
AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
if( pCurrentObject->Fcb != NULL)
{
pFcb = (AFSFcb *) InterlockedCompareExchangePointer( (PVOID *)&pCurrentObject->Fcb, NULL, (PVOID)pCurrentObject->Fcb);
if( pFcb != NULL)
{
AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
//
// Dropping the TreeLock permits the
// pCurrentObject->ObjectReferenceCount to change
//
AFSCleanupFcb( pFcb,
TRUE);
AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock,
TRUE);
AFSRemoveFcb( &pFcb);
}
AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
if( AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock,
FALSE))
{
AFSDeleteObjectInfo( pCurrentObject);
AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock);
pCurrentObject = pNextObject;
continue;
}
else
{
bReleaseVolumeLock = FALSE;
break;
}
}
else if( pCurrentObject->Fcb != NULL)
{
AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
//
// Dropping the TreeLock permits the
// pCurrentObject->ObjectReferenceCount to change
//
AFSCleanupFcb( pCurrentObject->Fcb,
FALSE);
AFSAcquireShared( pVolumeCB->ObjectInfoTree.TreeLock,
TRUE);
TRUE);
}
if( !AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock,
FALSE))
{
bReleaseVolumeLock = FALSE;
break;
}
if( BooleanFlagOn( pCurrentObject->Flags, AFS_OBJECT_FLAGS_DELETED) &&
pCurrentObject->ObjectReferenceCount <= 0 &&
( pCurrentObject->Fcb == NULL ||
pCurrentObject->Fcb->OpenReferenceCount == 0))
{
if( pCurrentObject->Fcb != NULL)
{
AFSRemoveFcb( &pCurrentObject->Fcb);
}
AFSDeleteObjectInfo( pCurrentObject);
}
AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock);
pCurrentObject = pNextObject;
continue;
}
pCurrentObject = pNextObject;