Changeset 279924 in webkit

Timestamp:

Jul 14, 2021 3:10:35 PM (3 years ago)

Author:

mark.lam@apple.com

Message:

Speculative fix for failed scope.releaseAssertNoException() after calls to JSMap::create().
​https://bugs.webkit.org/show_bug.cgi?id=227964
rdar://78013960

Reviewed by Yusuke Suzuki.

Source/JavaScriptCore:

There have been reports of flaky failures on the scope.releaseAssertNoException()
after the call to JSMap::create() in JSModuleLoader::finishCreation().

The scope.releaseAssertNoException() says that we don't expect the JSMap::create()
to ever throw an exception. If the assertion is true, the only way that we can
see an exception there is if we're throwing an asynchronous TerminationException.

Since JSModuleLoader::finishCreation() does not have any long running loops, we can
just DeferTerminationForAWhile and let the next exception check site throw the
asynchronous TerminationException. We don't want to just use DeferTermination
because it will throw the TerminationException right at the end of
JSModuleLoader::finishCreation(), and the caller of JSModuleLoader::finishCreation()
may be similarly not expecting an exception to be thrown there.

Also apply the same treatment to AbstractModuleRecord::finishCreation(), and
getBackingMap() in WebCore for the same reason. Other than those, other sites that
call JSMap::create() already check for exceptions. So, those sites do not need to
DeferTerminationForAWhile.

• runtime/AbstractModuleRecord.cpp:

(JSC::AbstractModuleRecord::finishCreation):

• runtime/JSModuleLoader.cpp:

(JSC::JSModuleLoader::finishCreation):

Source/WebCore:

• bindings/js/JSDOMMapLike.cpp:

(WebCore::getBackingMap):

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/runtime/AbstractModuleRecord.cpp (modified) (2 diffs)
• JavaScriptCore/runtime/JSModuleLoader.cpp (modified) (2 diffs)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/JSDOMMapLike.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-07-14  Mark Lam  <mark.lam@apple.com>
+
+        Speculative fix for failed scope.releaseAssertNoException() after calls to JSMap::create().
+        https://bugs.webkit.org/show_bug.cgi?id=227964
+        rdar://78013960
+
+        Reviewed by Yusuke Suzuki.
+
+        There have been reports of flaky failures on the scope.releaseAssertNoException()
+        after the call to JSMap::create() in JSModuleLoader::finishCreation().
+
+        The scope.releaseAssertNoException() says that we don't expect the JSMap::create()
+        to ever throw an exception.  If the assertion is true, the only way that we can
+        see an exception there is if we're throwing an asynchronous TerminationException.
+
+        Since JSModuleLoader::finishCreation() does not have any long running loops, we can
+        just DeferTerminationForAWhile and let the next exception check site throw the
+        asynchronous TerminationException.  We don't want to just use DeferTermination
+        because it will throw the TerminationException right at the end of
+        JSModuleLoader::finishCreation(), and the caller of JSModuleLoader::finishCreation()
+        may be similarly not expecting an exception to be thrown there.
+
+        Also apply the same treatment to AbstractModuleRecord::finishCreation(), and
+        getBackingMap() in WebCore for the same reason.  Other than those, other sites that
+        call JSMap::create() already check for exceptions.  So, those sites do not need to
+        DeferTerminationForAWhile.
+
+        * runtime/AbstractModuleRecord.cpp:
+        (JSC::AbstractModuleRecord::finishCreation):
+        * runtime/JSModuleLoader.cpp:
+        (JSC::JSModuleLoader::finishCreation):
+
 2021-07-14  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp

@@ -34 +34 @@
 #include "JSModuleNamespaceObject.h"
 #include "JSModuleRecord.h"
+#include "VMTrapsInlines.h"
 #include "WebAssemblyModuleRecord.h"
 
@@ -51 +52 @@
 void AbstractModuleRecord::finishCreation(JSGlobalObject* globalObject, VM& vm)
 {
-    DeferTermination deferScope(vm);
-    auto scope = DECLARE_THROW_SCOPE(vm);
+    DeferTerminationForAWhile deferScope(vm);
+    auto scope = DECLARE_CATCH_SCOPE(vm);
 
     Base::finishCreation(vm);

trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp

@@ -42 +42 @@
 #include "Parser.h"
 #include "ParserError.h"
+#include "VMTrapsInlines.h"
 
 namespace JSC {
@@ -99 +100 @@
 void JSModuleLoader::finishCreation(JSGlobalObject* globalObject, VM& vm)
 {
+    DeferTerminationForAWhile deferScope(vm);
     auto scope = DECLARE_CATCH_SCOPE(vm);
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-07-14  Mark Lam  <mark.lam@apple.com>
+
+        Speculative fix for failed scope.releaseAssertNoException() after calls to JSMap::create().
+        https://bugs.webkit.org/show_bug.cgi?id=227964
+        rdar://78013960
+
+        Reviewed by Yusuke Suzuki.
+
+        * bindings/js/JSDOMMapLike.cpp:
+        (WebCore::getBackingMap):
+
 2021-07-14  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/bindings/js/JSDOMMapLike.cpp

@@ -41 +41 @@
         return { false, *JSC::asObject(backingMap) };
 
-    JSC::DeferTermination deferScope(vm);
+    JSC::DeferTerminationForAWhile deferScope(vm);
     auto scope = DECLARE_CATCH_SCOPE(vm);