[1.6>1.7] [MERGE #3479 @tcare] Port AutoDisableInterrupt updates from…

… internal branch Merge pull request #3479 from tcare:autodisable I made use of Akrosh's auto interrupt disable class, and moved it to ThreadContext to replace the existing one which has less functionality.
chakra-core · Aug 4, 2017 · d7fba37 · d7fba37
2 parents 1cadbb3 + 9929d4b
commit d7fba37
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 56 deletions.
diff --git a/lib/Runtime/Base/ScriptContext.cpp b/lib/Runtime/Base/ScriptContext.cpp
@@ -3011,7 +3011,7 @@ namespace Js
         DEBUGGER_ATTACHDETACH_FATAL_ERROR_IF_FAILED(hr);
 
         // Disable QC while functions are re-parsed as this can be time consuming
-        AutoDisableInterrupt autoDisableInterrupt(this->threadContext->GetInterruptPoller(), true);
+        AutoDisableInterrupt autoDisableInterrupt(this->threadContext, false /* explicitCompletion */);
 
         hr = this->GetDebugContext()->RundownSourcesAndReparse(shouldPerformSourceRundown, /*shouldReparseFunctions*/ true);
 
@@ -3107,7 +3107,7 @@ namespace Js
         this->GetDebugContext()->SetDebuggerMode(Js::DebuggerMode::SourceRundown);
 
         // Disable QC while functions are re-parsed as this can be time consuming
-        AutoDisableInterrupt autoDisableInterrupt(this->threadContext->GetInterruptPoller(), true);
+        AutoDisableInterrupt autoDisableInterrupt(this->threadContext, false /* explicitCompletion */);
 
         // Force a reparse so that indirect function caches are updated.
         hr = this->GetDebugContext()->RundownSourcesAndReparse(/*shouldPerformSourceRundown*/ false, /*shouldReparseFunctions*/ true);

diff --git a/lib/Runtime/Base/ThreadContext.h b/lib/Runtime/Base/ThreadContext.h
@@ -82,30 +82,6 @@ class InterruptPoller _ABSTRACT
     bool isDisabled;
 };
 
-class AutoDisableInterrupt
-{
-private:
-    InterruptPoller* interruptPoller;
-    bool previousState;
-public:
-    AutoDisableInterrupt(InterruptPoller* interruptPoller, bool disable)
-        : interruptPoller(interruptPoller)
-    {
-        if (interruptPoller != nullptr)
-        {
-            previousState = interruptPoller->IsDisabled();
-            interruptPoller->SetDisabled(disable);
-        }
-    }
-    ~AutoDisableInterrupt()
-    {
-        if (interruptPoller != nullptr)
-        {
-            interruptPoller->SetDisabled(previousState);
-        }
-    }
-};
-
 // This function is called before we step out of script (currently only for WinRT callout).
 // Debugger would put a breakpoint on this function if they want to detect the point at which we step
 // over the boundary.
@@ -1800,6 +1776,42 @@ class ThreadContext sealed :
 
 extern void(*InitializeAdditionalProperties)(ThreadContext *threadContext);
 
+// This is for protecting a region of code, where we can't recover and be consistent upon failures (mainly due to OOM and SO).
+// FailFast on that. 
+class AutoDisableInterrupt
+{
+public:
+    AutoDisableInterrupt::AutoDisableInterrupt(ThreadContext *threadContext, bool explicitCompletion = true)
+        : m_operationCompleted(false), m_interruptDisableState(false), m_threadContext(threadContext), m_explicitCompletion(explicitCompletion)
+    {
+        if (m_threadContext->HasInterruptPoller())
+        {
+            m_interruptDisableState = m_threadContext->GetInterruptPoller()->IsDisabled();
+            m_threadContext->GetInterruptPoller()->SetDisabled(true);
+        }
+    }
+    AutoDisableInterrupt::~AutoDisableInterrupt()
+    {
+        if (m_threadContext->HasInterruptPoller())
+        {
+            m_threadContext->GetInterruptPoller()->SetDisabled(m_interruptDisableState);
+        }
+
+        if (m_explicitCompletion && !m_operationCompleted)
+        {
+            AssertOrFailFast(false);
+        }
+    }
+
+    void Completed() { m_operationCompleted = true; }
+
+private:
+    ThreadContext * m_threadContext;
+    bool m_operationCompleted;
+    bool m_interruptDisableState;
+    bool m_explicitCompletion;
+};
+
 #if ENABLE_JS_REENTRANCY_CHECK
 class JsReentLock
 {

diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp
@@ -1601,12 +1601,11 @@ namespace Js
         }
 #endif
 
-        // Grow the segments
-
         // Code below has potential to throw due to OOM or SO. Just FailFast on those cases
-        AutoFailFastOnError failFastError;
-
         ScriptContext *scriptContext = intArray->GetScriptContext();
+        AutoDisableInterrupt failFastError(scriptContext->GetThreadContext());
+
+        // Grow the segments
         Recycler *recycler = scriptContext->GetRecycler();
         SparseArraySegmentBase *seg, *nextSeg, *prevSeg = nullptr;
         for (seg = intArray->head; seg; seg = nextSeg)
@@ -1884,7 +1883,7 @@ namespace Js
         SparseArraySegmentBase *seg, *nextSeg, *prevSeg = nullptr;
 
         // Code below has potential to throw due to OOM or SO. Just FailFast on those cases
-        AutoFailFastOnError failFastError;
+        AutoDisableInterrupt failFastError(scriptContext->GetThreadContext());
 
         for (seg = intArray->head; seg; seg = nextSeg)
         {
@@ -2074,7 +2073,7 @@ namespace Js
         SparseArraySegmentBase *seg, *nextSeg, *prevSeg = nullptr;
 
         // Code below has potential to throw due to OOM or SO. Just FailFast on those cases
-        AutoFailFastOnError failFastError;
+        AutoDisableInterrupt failFastError(scriptContext->GetThreadContext());
 
         for (seg = fArray->head; seg; seg = nextSeg)
         {
@@ -7377,7 +7376,7 @@ namespace Js
         if (newArr && isBuiltinArrayCtor && len == pArr->length)
         {
             // Code below has potential to throw due to OOM or SO. Just FailFast on those cases
-            AutoFailFastOnError failFastOnError;
+            AutoDisableInterrupt failFastOnError(scriptContext->GetThreadContext());
 
             // Array has a single segment (need not start at 0) and splice start lies in the range
             // of that segment we optimize splice - Fast path.
@@ -12708,14 +12707,6 @@ namespace Js
         return static_cast<JavascriptNativeFloatArray *>(RecyclableObject::FromVar(aValue));
     }
 
-    AutoFailFastOnError::~AutoFailFastOnError()
-    {
-        if (!m_operationCompleted)
-        {
-            AssertOrFailFast(false);
-        }
-    }
-
     template int   Js::JavascriptArray::GetParamForIndexOf<unsigned int>(unsigned int, Js::Arguments const&, void*&, unsigned int&, Js::ScriptContext*);
     template bool  Js::JavascriptArray::ArrayElementEnumerator::MoveNext<void*>();
     template void  Js::JavascriptArray::SetArrayLiteralItem<void*>(unsigned int, void*);

diff --git a/lib/Runtime/Library/JavascriptArray.h b/lib/Runtime/Library/JavascriptArray.h
@@ -1267,19 +1267,4 @@ namespace Js
     template <>
     inline uint32 JavascriptArray::ConvertToIndex<uint32, uint32>(uint32 idxDest, ScriptContext* scriptContext) { return idxDest; }
 
-    // This is for protecting a region of code, where we can't recover and be consistent upon failures (mainly due to OOM and SO).
-    // FailFast on that. 
-    class AutoFailFastOnError
-    {
-    public:
-        AutoFailFastOnError() : m_operationCompleted(false) { }
-        ~AutoFailFastOnError();
-
-        void Completed() { m_operationCompleted = true; }
-
-    private:
-        bool m_operationCompleted;
-    };
-
-
 } // namespace Js