clear expirableObjectDisposeList on DisposeObjects

to avoid linear searching the list to do the removal which can cause busy hang this is a more completed fix for chakra-core#2739
leirocks · Apr 6, 2017 · 925076a · 925076a
1 parent 25942bc
commit 925076a
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 16 deletions.
diff --git a/lib/Common/Memory/Recycler.cpp b/lib/Common/Memory/Recycler.cpp
@@ -3275,6 +3275,7 @@ Recycler::DisposeObjects()
 #ifdef FAULT_INJECTION
     this->collectionWrapper->DisposeScriptContextByFaultInjectionCallBack();
 #endif
+    this->collectionWrapper->PreDisposeObjectsCallBack();
 
     // Scope timestamp to just dispose
     {

diff --git a/lib/Common/Memory/Recycler.h b/lib/Common/Memory/Recycler.h
@@ -354,6 +354,7 @@ class RecyclerCollectionWrapper
     virtual void DisposeScriptContextByFaultInjectionCallBack() = 0;
 #endif
     virtual void DisposeObjects(Recycler * recycler) = 0;
+    virtual void PreDisposeObjectsCallBack() = 0;
 #ifdef ENABLE_PROJECTION
     virtual void MarkExternalWeakReferencedObjects(bool inPartialCollect) = 0;
     virtual void ResolveExternalWeakReferencedObjects() = 0;
@@ -402,6 +403,7 @@ class DefaultRecyclerCollectionWrapper : public RecyclerCollectionWrapper
     virtual void DisposeScriptContextByFaultInjectionCallBack() override {};
 #endif
     virtual void DisposeObjects(Recycler * recycler) override;
+    virtual void PreDisposeObjectsCallBack() override {};
 
 #ifdef ENABLE_PROJECTION
     virtual void MarkExternalWeakReferencedObjects(bool inPartialCollect) override {};

diff --git a/lib/Runtime/Base/ExpirableObject.cpp b/lib/Runtime/Base/ExpirableObject.cpp
@@ -26,11 +26,6 @@ void ExpirableObject::Finalize(bool isShutdown)
 
 void ExpirableObject::Dispose(bool isShutdown)
 {
-    if (!isShutdown && this->registrationHandle == nullptr)
-    {
-        ThreadContext* threadContext = ThreadContext::GetContextForCurrentThread();
-        threadContext->DisposeExpirableObject(this);
-    }
 }
 
 void ExpirableObject::EnterExpirableCollectMode()

diff --git a/lib/Runtime/Base/ThreadContext.cpp b/lib/Runtime/Base/ThreadContext.cpp
@@ -1838,7 +1838,6 @@ void ThreadContext::DisposeOnLeaveScript()
     if (this->callDispose && this->recycler->NeedDispose())
     {
         this->recycler->FinishDisposeObjectsNow<FinishDispose>();
-        this->expirableObjectDisposeList->Clear();
     }
 }
 
@@ -2843,6 +2842,12 @@ ThreadContext::UpdateRedeferralState()
     }
 }
 
+void
+ThreadContext::PreDisposeObjectsCallBack()
+{
+    this->expirableObjectDisposeList->Clear();
+}
+
 #ifdef FAULT_INJECTION
 void
 ThreadContext::DisposeScriptContextByFaultInjectionCallBack()
@@ -3017,15 +3022,6 @@ ThreadContext::UnregisterExpirableObject(ExpirableObject* object)
     numExpirableObjects--;
 }
 
-void
-ThreadContext::DisposeExpirableObject(ExpirableObject* object)
-{
-    Assert(object->registrationHandle == nullptr);
-
-    //expirableObjectDisposeList will be cleared after finished disposing all objects
-
-    OUTPUT_VERBOSE_TRACE(Js::ExpirableCollectPhase, _u("Disposed 0x%p\n"), object);
-}
 #pragma endregion
 
 void

diff --git a/lib/Runtime/Base/ThreadContext.h b/lib/Runtime/Base/ThreadContext.h
@@ -1575,6 +1575,7 @@ class ThreadContext sealed :
     virtual void DisposeScriptContextByFaultInjectionCallBack() override;
 #endif
     virtual void DisposeObjects(Recycler * recycler) override;
+    virtual void PreDisposeObjectsCallBack() override;
 
     typedef DList<ExpirableObject*, ArenaAllocator> ExpirableObjectList;
     ExpirableObjectList* expirableObjectList;
@@ -1588,7 +1589,6 @@ class ThreadContext sealed :
     void TryExitExpirableCollectMode();
     void RegisterExpirableObject(ExpirableObject* object);
     void UnregisterExpirableObject(ExpirableObject* object);
-    void DisposeExpirableObject(ExpirableObject* object);
 
     void * GetDynamicObjectEnumeratorCache(Js::DynamicType const * dynamicType);
     void AddDynamicObjectEnumeratorCache(Js::DynamicType const * dynamicType, void * cache);