Fix rgTail corruption in rare scavenging cases [Desktop port]. (dotnet/coreclr#11327)

Commit migrated from https://github.com/dotnet/coreclr/commit/363206f312a73bbf6c2093d78acad58c060ca0c7

diff --git a/src/coreclr/src/gc/handletablecore.cpp b/src/coreclr/src/gc/handletablecore.cpp
index 00ab6a2..228b8bf 100644 (file)
--- a/src/coreclr/src/gc/handletablecore.cpp
+++ b/src/coreclr/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