diff options
author | Aditya Mandaleeka <adityamandaleeka@users.noreply.github.com> | 2017-05-01 22:03:25 -0700 |
---|---|---|
committer | Jan Kotas <jkotas@microsoft.com> | 2017-05-01 22:03:25 -0700 |
commit | 363206f312a73bbf6c2093d78acad58c060ca0c7 (patch) | |
tree | f7d8af0202beafbc028c6fe3ec791232fdfdc49b /src/gc | |
parent | 9501a3b59923bf9c1fea780dc97a342692a8c2ea (diff) | |
download | coreclr-363206f312a73bbf6c2093d78acad58c060ca0c7.tar.gz coreclr-363206f312a73bbf6c2093d78acad58c060ca0c7.tar.bz2 coreclr-363206f312a73bbf6c2093d78acad58c060ca0c7.zip |
Fix rgTail corruption in rare scavenging cases [Desktop port]. (#11327)
Diffstat (limited to 'src/gc')
-rw-r--r-- | src/gc/handletablecore.cpp | 26 |
1 files changed, 26 insertions, 0 deletions
diff --git a/src/gc/handletablecore.cpp b/src/gc/handletablecore.cpp index 00ab6a24b9..228b8bfa09 100644 --- a/src/gc/handletablecore.cpp +++ b/src/gc/handletablecore.cpp @@ -1592,6 +1592,7 @@ void SegmentResortChains(TableSegment *pSegment) // clear the sort flag for this segment pSegment->fResortChains = FALSE; + BOOL fScavengingOccurred = FALSE; // first, do we need to scavenge any blocks? if (pSegment->fNeedsScavenging) @@ -1599,6 +1600,8 @@ void SegmentResortChains(TableSegment *pSegment) // clear the scavenge flag pSegment->fNeedsScavenging = FALSE; + fScavengingOccurred = TRUE; + // we may need to explicitly scan the user data chain too BOOL fCleanupUserData = FALSE; @@ -1758,6 +1761,21 @@ void SegmentResortChains(TableSegment *pSegment) if (pSegment->rgBlockType[pSegment->rgHint[uType]] != uType) pSegment->rgHint[uType] = bBlock; } + else + { + // No blocks of this type were found in the rgBlockType array, meaning either there were no + // such blocks on entry to this function (in which case the associated tail is guaranteed + // to already be marked invalid) OR that there were blocks but all of them were reclaimed + // by the scavenging logic above (in which case the associated tail is guaranteed to point + // to one of the scavenged blocks). In the latter case, the tail is currently "stale" + // and therefore needs to be manually updated. + if (pSegment->rgTail[uType] != BLOCK_INVALID) + { + _ASSERTE(fScavengingOccurred); + pSegment->rgTail[uType] = BLOCK_INVALID; + pSegment->rgHint[uType] = BLOCK_INVALID; + } + } } // store the new free list head @@ -2549,6 +2567,14 @@ uint32_t BlockFreeHandles(TableSegment *pSegment, uint32_t uBlock, OBJECTHANDLE if (fAllMasksWeTouchedAreFree) { // is the block unlocked? + // NOTE: This check is incorrect and defeats the intended purpose of scavenging. If the + // current block is locked and has just been emptied, then it cannot be removed right now + // and therefore will nominally need to be scavenged. The only code that triggers + // scavenging is in SegmentRemoveFreeBlocks, and setting the flag is the only way to + // trigger a call into SegmentRemoveFreeBlocks call. As a result, by NOT setting the flag + // this code is generally PREVENTING scavenging in exactly the cases where scavenging is + // needed. The code is not being changed because it has always been this way and scavenging + // itself generally has extremely low value. if (!BlockIsLocked(pSegment, uBlock)) { // tell the caller it might be a good idea to scan for free blocks |