[MERGE chakra-core#2172 @pleath] 2 redeferral-related fixes in Recycler

pleath · pleath · commit 95658700e28f · 2016-12-09T16:30:24.000-08:00
Merge pull request chakra-core#2172 from pleath:10017672 1. Do not try to take special mark actions in BackgroundScanStack, only in ScanStack, as memory may be unstable in the background case. 2. In marking, if MarkInternal returns true (indicating no further work is necessary), and if we're required to take special mark actions, detect finalizable object and take the action.
diff --git a/lib/Common/Memory/HeapBlockMap.h b/lib/Common/Memory/HeapBlockMap.h
@@ -188,6 +188,8 @@ class HeapBlockMap32
     template <bool interlocked>
     bool MarkInternal(L2MapChunk * chunk, void * candidate);
 
+    void OnSpecialMark(L2MapChunk * chunk, void * candidate);
+
     template <bool interlocked, bool updateChunk>
     bool MarkInteriorInternal(MarkContext * markContext, L2MapChunk *& chunk, void * originalCandidate, void * realCandidate);
 
diff --git a/lib/Common/Memory/HeapBlockMap.inl b/lib/Common/Memory/HeapBlockMap.inl
@@ -68,6 +68,10 @@ HeapBlockMap32::Mark(void * candidate, MarkContext * markContext)
 
     if (MarkInternal<interlocked>(chunk, candidate))
     {
+        if (doSpecialMark)
+        {
+            this->OnSpecialMark(chunk, candidate);
+        }
         return;
     }
 
@@ -153,6 +157,59 @@ HeapBlockMap32::Mark(void * candidate, MarkContext * markContext)
     }
 }
 
+inline
+void
+HeapBlockMap32::OnSpecialMark(L2MapChunk * chunk, void * candidate)
+{
+    uint id2 = GetLevel2Id(candidate);
+    HeapBlock::HeapBlockType blockType = chunk->blockInfo[id2].blockType;
+
+    Assert(blockType == HeapBlock::HeapBlockType::FreeBlockType || chunk->map[id2]->GetHeapBlockType() == blockType);
+
+    unsigned char attributes = ObjectInfoBits::NoBit;
+    bool success = false;
+
+    // If the block is finalizable, we may still have to take special mark action.
+    switch (blockType)
+    {
+    case HeapBlock::HeapBlockType::SmallFinalizableBlockType:
+#ifdef RECYCLER_WRITE_BARRIER
+    case HeapBlock::HeapBlockType::SmallFinalizableBlockWithBarrierType:
+#endif
+    {
+        SmallFinalizableHeapBlock *smallBlock = (SmallFinalizableHeapBlock*)chunk->map[id2];
+        success = smallBlock->TryGetAttributes(candidate, &attributes);
+        break;
+    }
+
+    case HeapBlock::HeapBlockType::MediumFinalizableBlockType:
+#ifdef RECYCLER_WRITE_BARRIER
+    case HeapBlock::HeapBlockType::MediumFinalizableBlockWithBarrierType:
+#endif
+    {
+        MediumFinalizableHeapBlock *mediumBlock = (MediumFinalizableHeapBlock*)chunk->map[id2];
+        success = mediumBlock->TryGetAttributes(candidate, &attributes);
+        break;
+    }
+
+    case HeapBlock::HeapBlockType::LargeBlockType:
+    {
+        LargeHeapBlock *largeBlock = (LargeHeapBlock*)chunk->map[id2];
+        success = largeBlock->TryGetAttributes(candidate, &attributes);
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    if (success && (attributes & FinalizeBit))
+    {   
+        FinalizableObject *trackedObject = (FinalizableObject*)candidate;
+        trackedObject->OnMark();
+    }
+}
+
 template <bool interlocked, bool largeBlockType>
 inline
 bool
diff --git a/lib/Common/Memory/LargeHeapBlock.cpp b/lib/Common/Memory/LargeHeapBlock.cpp
@@ -378,6 +378,44 @@ LargeHeapBlock::IsFreeObject(void * objectAddress)
     return ((char *)header >= this->address && header->objectIndex < this->allocCount && this->GetHeader(header->objectIndex) == nullptr);
 }
 #endif
+
+bool
+LargeHeapBlock::TryGetAttributes(void* objectAddress, unsigned char * pAttr)
+{
+    return this->TryGetAttributes(GetHeader(objectAddress), pAttr);
+}
+
+bool
+LargeHeapBlock::TryGetAttributes(LargeObjectHeader * header, unsigned char * pAttr)
+{
+    if ((char *)header < this->address)
+    {
+        return false;
+    }
+
+    uint index = header->objectIndex;
+
+    if (index >= this->allocCount)
+    {
+        // Not allocated yet.
+        return false;
+    }
+
+    if (this->HeaderList()[index] != header)
+    {
+        // header doesn't match, not a real object
+        return false;
+    }
+
+    if (this->InPageHeapMode())
+    {
+        this->VerifyPageHeapPattern();
+    }
+
+    *pAttr = header->GetAttributes(this->heapInfo->recycler->Cookie);
+    return true;
+}
+
 size_t
 LargeHeapBlock::GetPagesNeeded(size_t size, bool multiplyRequest)
 {
@@ -563,33 +601,14 @@ LargeHeapBlock::Mark(void* objectAddress, MarkContext * markContext)
 {
     LargeObjectHeader * header = GetHeader(objectAddress);
 
-    if ((char *)header < this->address)
+    unsigned char attributes = ObjectInfoBits::NoBit;
+    if (!this->TryGetAttributes(header, &attributes))
     {
         return;
     }
 
-    uint index = header->objectIndex;
-
-    if (index >= this->allocCount)
-    {
-        // Not allocated yet.
-        return;
-    }
-
-    if (this->HeaderList()[index] != header)
-    {
-        // header doesn't match, not a real object
-        return;
-    }
-
-    if (this->InPageHeapMode())
-    {
-        this->VerifyPageHeapPattern();
-    }
-
     DUMP_OBJECT_REFERENCE(markContext->GetRecycler(), objectAddress);
 
-    unsigned char attributes = header->GetAttributes(this->heapInfo->recycler->Cookie);
     size_t objectSize = header->objectSize;
     if (this->InPageHeapMode())
     {
diff --git a/lib/Common/Memory/LargeHeapBlock.h b/lib/Common/Memory/LargeHeapBlock.h
@@ -149,6 +149,9 @@ class LargeHeapBlock sealed : public HeapBlock
     char* GetBeginAddress() const { return address; }
     char* GetEndAddress() const { return addressEnd; }
 
+    bool TryGetAttributes(void* objectAddress, unsigned char * pAttr);
+    bool TryGetAttributes(LargeObjectHeader *objectHeader, unsigned char * pAttr);
+
     char * Alloc(DECLSPEC_GUARD_OVERFLOW size_t size, ObjectInfoBits attributes);
     char * TryAllocFromFreeList(DECLSPEC_GUARD_OVERFLOW size_t size, ObjectInfoBits attributes);
 
diff --git a/lib/Common/Memory/Recycler.cpp b/lib/Common/Memory/Recycler.cpp
@@ -4974,15 +4974,7 @@ Recycler::BackgroundScanStack()
     if (stackTop != nullptr)
     {
         size_t size = (char *)stackBase - stackTop;
-        bool doSpecialMark = collectionWrapper->DoSpecialMarkOnScanStack();
-        if (doSpecialMark)
-        {
-            ScanMemoryInline<true>((void **)stackTop, size);
-        }
-        else
-        {
-            ScanMemoryInline<false>((void **)stackTop, size);
-        }
+        ScanMemoryInline<false>((void **)stackTop, size);
         return size;
     }
 
diff --git a/lib/Common/Memory/SmallFinalizableHeapBlock.cpp b/lib/Common/Memory/SmallFinalizableHeapBlock.cpp
@@ -104,20 +104,45 @@ SmallFinalizableHeapBlockT<TBlockAttributes>::SetAttributes(void * address, unsi
 }
 
 template <class TBlockAttributes>
-template <bool doSpecialMark>
-_NOINLINE
-void
-SmallFinalizableHeapBlockT<TBlockAttributes>::ProcessMarkedObject(void* objectAddress, MarkContext * markContext)
+bool
+SmallFinalizableHeapBlockT<TBlockAttributes>::TryGetAttributes(void* objectAddress, unsigned char * pAttr)
+{
+    unsigned char * attributes = nullptr;
+    if (this->TryGetAddressOfAttributes(objectAddress, &attributes))
+    {
+        *pAttr = *attributes;
+        return true;
+    }
+    return false;
+}
+
+template <class TBlockAttributes>
+bool
+SmallFinalizableHeapBlockT<TBlockAttributes>::TryGetAddressOfAttributes(void* objectAddress, unsigned char ** ppAttrs)
 {
     ushort objectIndex = this->GetAddressIndex(objectAddress);
 
     if (objectIndex == SmallHeapBlockT<TBlockAttributes>::InvalidAddressBit)
     {
         // Not a valid offset within the block.  No further processing necessary.
-        return;
+        return false;
     }
 
-    unsigned char * attributes = &this->ObjectInfo(objectIndex);
+    *ppAttrs = &this->ObjectInfo(objectIndex);
+    return true;
+}
+
+template <class TBlockAttributes>
+template <bool doSpecialMark>
+_NOINLINE
+void
+SmallFinalizableHeapBlockT<TBlockAttributes>::ProcessMarkedObject(void* objectAddress, MarkContext * markContext)
+{
+    unsigned char * attributes = nullptr;
+    if (!this->TryGetAddressOfAttributes(objectAddress, &attributes))
+    {
+        return;
+    }
 
     if (!this->template UpdateAttributesOfMarkedObjects<doSpecialMark>(markContext, objectAddress, this->objectSize, *attributes,
         [&](unsigned char _attributes) { *attributes = _attributes; }))
diff --git a/lib/Common/Memory/SmallFinalizableHeapBlock.h b/lib/Common/Memory/SmallFinalizableHeapBlock.h
@@ -30,6 +30,9 @@ class SmallFinalizableHeapBlockT : public SmallNormalHeapBlockT<TBlockAttributes
     }
     void SetNextBlock(SmallFinalizableHeapBlockT * next) { Base::SetNextBlock(next); }
 
+    bool TryGetAddressOfAttributes(void* objectAddress, unsigned char **ppAttr);
+    bool TryGetAttributes(void* objectAddress, unsigned char *pAttr);
+
     template <bool doSpecialMark>
     void ProcessMarkedObject(void* candidate, MarkContext * markContext);
 

Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,9 @@ class SmallFinalizableHeapBlockT : public SmallNormalHeapBlockT<TBlockAttributes`
`30`	`30`	`}`
`31`	`31`	`void SetNextBlock(SmallFinalizableHeapBlockT * next) { Base::SetNextBlock(next); }`
`32`	`32`
	`33`	`+ bool TryGetAddressOfAttributes(void* objectAddress, unsigned char **ppAttr);`
	`34`	`+ bool TryGetAttributes(void* objectAddress, unsigned char *pAttr);`
	`35`	`+`
`33`	`36`	`template <bool doSpecialMark>`
`34`	`37`	`void ProcessMarkedObject(void* candidate, MarkContext * markContext);`
`35`	`38`