Title: [286913] trunk/Source
- Revision
- 286913
- Author
- [email protected]
- Date
- 2021-12-11 08:59:05 -0800 (Sat, 11 Dec 2021)
Log Message
Automatically forbid JS execution when we throw a TerminationException.
https://bugs.webkit.org/show_bug.cgi?id=234188
Reviewed by Yusuke Suzuki.
Source/_javascript_Core:
For Worker threads, we throw a TerminationException when Worker.terminate() is
called. Once the TerminationException is thrown, we expect to completely unwind
out of any JS frames on the stack, and we also expect the client to never call
into JS again. Previously, WebCore will call VM:setExecutionForbidden() to flag
that we should not re-enter the VM anymore. On JSC side, this executionForbidden()
is used to prevent micro-tasks from firing. On WebCore side, it is used to prevent
many things from running, including firing events.
Previously, we reply on WebCore side to catch the TerminationException, determine
that it is the TerminationException, and then call VM:setExecutionForbidden().
This is tedious and error prone as there may be places in WebCore that should call
VM:setExecutionForbidden() but is missed. This has been the source of some bugs
with the handling of the Worker termination in the past.
In this patch, we change VM to setExecutionForbidden() immediately when we throw
the TerminationException, but only if VM::m_executionForbiddenOnTermination is set.
Currently, we'll only set VM:m_executionForbiddenOnTermination for Workers because
for legacy reasons, other clients of JSC has the ability to re-enter the VM after
a TerminationException unwinds out (which is ok to do when used under some
controlled conditions). Until we can determine that it is safe to adopt this
"execution forbidden on termination" behavior universally, we'll adopt it only for
workers.
In a subsequent patch, we can also look into removing all the places in WebCore
that checks for TerminationException in order to call VM:setExecutionForbidden().
We'll leave those in place for now though they should be redundant after this patch.
Also add some ASSERTs to document invariants regarding states used in the handing
of TerminationException.
* runtime/VM.cpp:
(JSC::VM::setException):
(JSC::VM::throwTerminationException):
* runtime/VM.h:
(JSC::VM::forbidExecutionOnTermination):
Source/WebCore:
Enable "execution forbidden on termination" behavior for workers.
* workers/WorkerOrWorkletScriptController.cpp:
(WebCore::WorkerOrWorkletScriptController::WorkerOrWorkletScriptController):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (286912 => 286913)
--- trunk/Source/_javascript_Core/ChangeLog 2021-12-11 16:55:07 UTC (rev 286912)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-12-11 16:59:05 UTC (rev 286913)
@@ -1,3 +1,46 @@
+2021-12-11 Mark Lam <[email protected]>
+
+ Automatically forbid JS execution when we throw a TerminationException.
+ https://bugs.webkit.org/show_bug.cgi?id=234188
+
+ Reviewed by Yusuke Suzuki.
+
+ For Worker threads, we throw a TerminationException when Worker.terminate() is
+ called. Once the TerminationException is thrown, we expect to completely unwind
+ out of any JS frames on the stack, and we also expect the client to never call
+ into JS again. Previously, WebCore will call VM:setExecutionForbidden() to flag
+ that we should not re-enter the VM anymore. On JSC side, this executionForbidden()
+ is used to prevent micro-tasks from firing. On WebCore side, it is used to prevent
+ many things from running, including firing events.
+
+ Previously, we reply on WebCore side to catch the TerminationException, determine
+ that it is the TerminationException, and then call VM:setExecutionForbidden().
+ This is tedious and error prone as there may be places in WebCore that should call
+ VM:setExecutionForbidden() but is missed. This has been the source of some bugs
+ with the handling of the Worker termination in the past.
+
+ In this patch, we change VM to setExecutionForbidden() immediately when we throw
+ the TerminationException, but only if VM::m_executionForbiddenOnTermination is set.
+ Currently, we'll only set VM:m_executionForbiddenOnTermination for Workers because
+ for legacy reasons, other clients of JSC has the ability to re-enter the VM after
+ a TerminationException unwinds out (which is ok to do when used under some
+ controlled conditions). Until we can determine that it is safe to adopt this
+ "execution forbidden on termination" behavior universally, we'll adopt it only for
+ workers.
+
+ In a subsequent patch, we can also look into removing all the places in WebCore
+ that checks for TerminationException in order to call VM:setExecutionForbidden().
+ We'll leave those in place for now though they should be redundant after this patch.
+
+ Also add some ASSERTs to document invariants regarding states used in the handing
+ of TerminationException.
+
+ * runtime/VM.cpp:
+ (JSC::VM::setException):
+ (JSC::VM::throwTerminationException):
+ * runtime/VM.h:
+ (JSC::VM::forbidExecutionOnTermination):
+
2021-12-10 Yusuke Suzuki <[email protected]>
[JSC] Wasm catch thunk should be JIT code to use ExceptionHandlerPtrTag
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (286912 => 286913)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2021-12-11 16:55:07 UTC (rev 286912)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2021-12-11 16:59:05 UTC (rev 286913)
@@ -839,6 +839,7 @@
void VM::setException(Exception* exception)
{
+ ASSERT(!exception || !isTerminationException(exception) || terminationInProgress());
m_exception = exception;
m_lastException = exception;
if (exception)
@@ -847,8 +848,11 @@
void VM::throwTerminationException()
{
+ ASSERT(terminationInProgress());
ASSERT(!m_traps.isDeferringTermination());
setException(terminationException());
+ if (m_executionForbiddenOnTermination)
+ setExecutionForbidden();
}
Exception* VM::throwException(JSGlobalObject* globalObject, Exception* exceptionToThrow)
Modified: trunk/Source/_javascript_Core/runtime/VM.h (286912 => 286913)
--- trunk/Source/_javascript_Core/runtime/VM.h 2021-12-11 16:55:07 UTC (rev 286912)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2021-12-11 16:59:05 UTC (rev 286913)
@@ -294,6 +294,13 @@
bool executionForbidden() const { return m_executionForbidden; }
void setExecutionForbidden() { m_executionForbidden = true; }
+ // Setting this means that the VM can never recover from a TerminationException.
+ // Currently, we'll only set this for worker threads. Ideally, we want this
+ // to always be true. However, we're only limiting it to workers for now until
+ // we can be sure that clients using the JSC watchdog (which uses termination)
+ // isn't broken by this change.
+ void forbidExecutionOnTermination() { m_executionForbiddenOnTermination = true; }
+
JS_EXPORT_PRIVATE Exception* ensureTerminationException();
Exception* terminationException() const
{
@@ -1113,6 +1120,7 @@
bool m_terminationInProgress { false };
bool m_executionForbidden { false };
+ bool m_executionForbiddenOnTermination { false };
Lock m_loopHintExecutionCountLock;
HashMap<const Instruction*, std::pair<unsigned, std::unique_ptr<uintptr_t>>> m_loopHintExecutionCounts;
Modified: trunk/Source/WebCore/ChangeLog (286912 => 286913)
--- trunk/Source/WebCore/ChangeLog 2021-12-11 16:55:07 UTC (rev 286912)
+++ trunk/Source/WebCore/ChangeLog 2021-12-11 16:59:05 UTC (rev 286913)
@@ -1,5 +1,17 @@
2021-12-11 Mark Lam <[email protected]>
+ Automatically forbid JS execution when we throw a TerminationException.
+ https://bugs.webkit.org/show_bug.cgi?id=234188
+
+ Reviewed by Yusuke Suzuki.
+
+ Enable "execution forbidden on termination" behavior for workers.
+
+ * workers/WorkerOrWorkletScriptController.cpp:
+ (WebCore::WorkerOrWorkletScriptController::WorkerOrWorkletScriptController):
+
+2021-12-11 Mark Lam <[email protected]>
+
WebCore::createDOMException() should abort early if termination is pending.
https://bugs.webkit.org/show_bug.cgi?id=234190
Modified: trunk/Source/WebCore/workers/WorkerOrWorkletScriptController.cpp (286912 => 286913)
--- trunk/Source/WebCore/workers/WorkerOrWorkletScriptController.cpp 2021-12-11 16:55:07 UTC (rev 286912)
+++ trunk/Source/WebCore/workers/WorkerOrWorkletScriptController.cpp 2021-12-11 16:59:05 UTC (rev 286913)
@@ -77,6 +77,7 @@
{
JSLockHolder lock(m_vm.get());
m_vm->ensureTerminationException();
+ m_vm->forbidExecutionOnTermination();
}
JSVMClientData::initNormalWorld(m_vm.get(), type);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
