Parcourir la source

fix throwing new exception before previous being caught

Previously I did a change to temporarily save (keep alive) an exception
object reference in threadContext before it being caught and handled.
However it does not address a scenario that before the previous exception
being caught, we might need to throw another exception (e.g., leave script
event handler might throw/catch on the same thread).

To support this, expand previous one exception registry into a list.
Since we don't want to allocate memory at this point, add list support into
exception object itself (add field "next").

Modified wrapper `JavascriptException` to be more robust in clearing
the registry in case one missed explicit GetAndClear().
Jianchun Xu il y a 9 ans
Parent
commit
2391a623a9

+ 40 - 13
lib/Common/Exceptions/JavascriptException.h

@@ -4,9 +4,19 @@
 //-------------------------------------------------------------------------------------------------------
 #pragma once
 
-namespace Js {
+class ThreadContext;
+
+namespace Js
+{
     class JavascriptExceptionObject;
 
+    // Implemented by runtime to temporarily save (keep alive during throw) / clear
+    // an exception object.
+    void SaveTempUncaughtException(
+        ThreadContext* threadContext, Js::JavascriptExceptionObject* exception);
+    void ClearTempUncaughtException(
+        ThreadContext* threadContext, Js::JavascriptExceptionObject* exception);
+
     //
     // JavascriptException wraps a runtime JavascriptExceptionObject. To ensure
     // a thrown JavascriptExceptionObject is kept alive before being caught by
@@ -18,26 +28,43 @@ namespace Js {
     class JavascriptException : public ExceptionBase
     {
     private:
-        Field(JavascriptExceptionObject*)* const addressOfException;
+        ThreadContext* const threadContext;
+        mutable Js::JavascriptExceptionObject* exception;
 
     public:
-        // Caller should have stored the JavascriptExceptionObject reference in
-        // thread context data. addressOfException should be the thread context data
-        // address.
-        JavascriptException(Field(JavascriptExceptionObject*)* addressOfException)
-            : addressOfException(addressOfException)
+        JavascriptException(ThreadContext* threadContext,
+                            Js::JavascriptExceptionObject* exception = nullptr)
+            : threadContext(threadContext), exception(exception)
         {
-            Assert(addressOfException && *addressOfException);
+            SaveTempUncaughtException(threadContext, exception);
         }
 
-        JavascriptExceptionObject* GetAndClear() const
+        JavascriptException(const JavascriptException& other)
+            : threadContext(other.threadContext), exception(other.exception)
+        {
+            other.exception = nullptr;  // transfer
+        }
+
+        ~JavascriptException()
         {
-            Assert(*addressOfException);
+            if (exception)
+            {
+                GetAndClear();
+            }
+        }
 
-            JavascriptExceptionObject* exceptionObject = *addressOfException;
-            *addressOfException = nullptr;
-            return exceptionObject;
+        JavascriptExceptionObject* GetAndClear() const
+        {
+            JavascriptExceptionObject* tmp = exception;
+            if (exception)
+            {
+                ClearTempUncaughtException(threadContext, exception);
+                exception = nullptr;
+            }
+            return tmp;
         }
+
+        PREVENT_ASSIGN(JavascriptException);
     };
 
 } // namespace Js

+ 7 - 6
lib/Runtime/Base/ThreadContext.h

@@ -1361,15 +1361,16 @@ public:
     Js::JavascriptExceptionObject* GetUnhandledExceptionObject() const  { return recyclableData->unhandledExceptionObject; };
 
     // To temporarily keep throwing exception object alive (thrown but not yet caught)
-    Field(Js::JavascriptExceptionObject*)* SaveTempUncaughtException(Js::JavascriptExceptionObject* exceptionObject)
+    void SaveTempUncaughtException(Js::JavascriptExceptionObject* exceptionObject)
     {
-        // Previous save should have been caught and cleared
-        Assert(recyclableData->tempUncaughtException == nullptr);
-
-        recyclableData->tempUncaughtException = exceptionObject;
-        return &recyclableData->tempUncaughtException;
+        Js::JavascriptExceptionObject::Insert(&recyclableData->tempUncaughtException, exceptionObject);
+    }
+    void ClearTempUncaughtException(Js::JavascriptExceptionObject* exceptionObject)
+    {
+        Js::JavascriptExceptionObject::Remove(&recyclableData->tempUncaughtException, exceptionObject);
     }
 
+public:
     bool HasCatchHandler() const { return hasCatchHandler; }
     void SetHasCatchHandler(bool hasCatchHandler) { this->hasCatchHandler = hasCatchHandler; }
 

+ 38 - 0
lib/Runtime/Language/JavascriptExceptionObject.cpp

@@ -227,4 +227,42 @@ namespace Js
         }
     }
 #endif
+
+    void JavascriptExceptionObject::Insert(
+        Field(JavascriptExceptionObject*)* head, JavascriptExceptionObject* item)
+    {
+        Assert(!item->next);
+        item->next = *head;
+        *head = item;
+    }
+
+    void JavascriptExceptionObject::Remove(
+        Field(JavascriptExceptionObject*)* head, JavascriptExceptionObject* item)
+    {
+        // Typically Insert/Remove happens in reversed order and item should be
+        // the front one. Loop the whole list to prevent unexpected order messup.
+        for (auto p = head; *p; p = &(*p)->next)
+        {
+            if (*p == item)
+            {
+                *p = item->next;
+                item->next = nullptr;
+                return;
+            }
+        }
+
+        Assert(false);  // item not in list unexpected
+    }
+
+    //
+    // Support JavascriptException implementation
+    //
+    void SaveTempUncaughtException(ThreadContext* threadContext, Js::JavascriptExceptionObject* exceptionObject)
+    {
+        threadContext->SaveTempUncaughtException(exceptionObject);
+    }
+    void ClearTempUncaughtException(ThreadContext* threadContext, Js::JavascriptExceptionObject* exceptionObject)
+    {
+        threadContext->ClearTempUncaughtException(exceptionObject);
+    }
 }

+ 13 - 2
lib/Runtime/Language/JavascriptExceptionObject.h

@@ -4,7 +4,8 @@
 //-------------------------------------------------------------------------------------------------------
 #pragma once
 
-namespace Js {
+namespace Js
+{
     const DWORD  ExceptionParameters = 1;
     const int    ExceptionObjectIndex = 0;
 
@@ -18,7 +19,8 @@ namespace Js {
         JavascriptExceptionObject(Var object, ScriptContext * scriptContext, JavascriptExceptionContext* exceptionContextIn, bool isPendingExceptionObject = false) :
             thrownObject(object), isPendingExceptionObject(isPendingExceptionObject),
             scriptContext(scriptContext), tag(true), isDebuggerSkip(false), byteCodeOffsetAfterDebuggerSkip(Constants::InvalidByteCodeOffset), hasDebuggerLogged(false),
-            isFirstChance(false), isExceptionCaughtInNonUserCode(false), ignoreAdvanceToNextStatement(false), hostWrapperCreateFunc(nullptr), isGeneratorReturnException(false)
+            isFirstChance(false), isExceptionCaughtInNonUserCode(false), ignoreAdvanceToNextStatement(false), hostWrapperCreateFunc(nullptr), isGeneratorReturnException(false),
+            next(nullptr)
         {
             if (exceptionContextIn)
             {
@@ -160,6 +162,11 @@ namespace Js {
             return isGeneratorReturnException;
         }
 
+    private:
+        friend class ::ThreadContext;
+        static void Insert(Field(JavascriptExceptionObject*)* head, JavascriptExceptionObject* item);
+        static void Remove(Field(JavascriptExceptionObject*)* head, JavascriptExceptionObject* item);
+
     private:
         Field(Var)      thrownObject;
         Field(ScriptContext *) scriptContext;
@@ -184,6 +191,10 @@ namespace Js {
         static const int StackToSkip = 2;
         static const int StackTraceDepth = 30;
 #endif
+
+        Field(JavascriptExceptionObject*) next;  // to temporarily store list of throwing exceptions
+
+        PREVENT_COPY(JavascriptExceptionObject)
     };
 
     class GeneratorReturnExceptionObject : public JavascriptExceptionObject

+ 1 - 4
lib/Runtime/Language/JavascriptExceptionOperators.cpp

@@ -869,11 +869,8 @@ namespace Js
     {
         ThreadContext* threadContext = scriptContext? scriptContext->GetThreadContext() : ThreadContext::GetContextForCurrentThread();
 
-        // Temporarily keep throwing exception object alive (thrown but not yet caught)
-        Field(JavascriptExceptionObject*)* addr = threadContext->SaveTempUncaughtException(exceptionObject);
-
         // Throw a wrapper JavascriptException. catch handler must GetAndClear() the exception object.
-        throw JavascriptException(addr);
+        throw JavascriptException(threadContext, exceptionObject);
     }
 
     void JavascriptExceptionOperators::DoThrowCheckClone(JavascriptExceptionObject* exceptionObject, ScriptContext* scriptContext)