Diff
Modified: trunk/LayoutTests/ChangeLog (189008 => 189009)
--- trunk/LayoutTests/ChangeLog 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/LayoutTests/ChangeLog 2015-08-27 02:49:52 UTC (rev 189009)
@@ -1,3 +1,14 @@
+2015-08-26 Mark Lam <mark....@apple.com>
+
+ watchdog m_didFire state erroneously retained.
+ https://bugs.webkit.org/show_bug.cgi?id=131082
+
+ Reviewed by Geoffrey Garen.
+
+ * fast/workers/worker-terminate-forever-expected.txt:
+ * fast/workers/worker-terminate-forever.html:
+ - Updated to check if the worker actually did terminate.
+
2015-08-26 Andy Estes <aes...@apple.com>
REGRESSION (r188987): imported/mozilla/svg/filters/feConvolveMatrix-1.svg fails
Modified: trunk/LayoutTests/fast/workers/worker-terminate-forever-expected.txt (189008 => 189009)
--- trunk/LayoutTests/fast/workers/worker-terminate-forever-expected.txt 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/LayoutTests/fast/workers/worker-terminate-forever-expected.txt 2015-08-27 02:49:52 UTC (rev 189009)
@@ -1 +1,3 @@
+CONSOLE MESSAGE: line 15: Worker was started
+CONSOLE MESSAGE: line 34: Worker was terminated
Test Worker.terminate() for a worker that tries to run forever.
Modified: trunk/LayoutTests/fast/workers/worker-terminate-forever.html (189008 => 189009)
--- trunk/LayoutTests/fast/workers/worker-terminate-forever.html 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/LayoutTests/fast/workers/worker-terminate-forever.html 2015-08-27 02:49:52 UTC (rev 189009)
@@ -1,11 +1,52 @@
<body>
<p>Test Worker.terminate() for a worker that tries to run forever.</p>
<script>
-if (window.testRunner)
+if (window.testRunner) {
testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
var worker = new Worker('resources/worker-run-forever.js');
-worker.terminate();
+
+function waitForWorkerToStart() {
+ var startTime = Date.now();
+ function checkIfWorkerStarted() {
+ if (internals.workerThreadCount == 1) {
+ console.log("Worker was started");
+ worker.terminate();
+ setTimeout(waitForWorkerToStop, 0);
+
+ } else if (Date.now() - startTime < 5000) {
+ setTimeout(checkIfWorkerStarted, 0);
+
+ } else {
+ console.log("Worker did not show up");
+ testRunner.notifyDone();
+ }
+ }
+ setTimeout(checkIfWorkerStarted, 0);
+}
+
+function waitForWorkerToStop() {
+ var startTime = Date.now();
+ function checkIfWorkerStopped() {
+ if (internals.workerThreadCount == 0) {
+ console.log("Worker was terminated");
+ testRunner.notifyDone();
+
+ } else if (Date.now() - startTime < 5000) {
+ setTimeout(checkIfWorkerStopped, 0);
+
+ } else {
+ console.log("Did not see worker terminate");
+ testRunner.notifyDone();
+ }
+ }
+ setTimeout(checkIfWorkerStopped, 0);
+}
+
+window.setTimeout(waitForWorkerToStart, 0);
+
</script>
</body>
</html>
Modified: trunk/Source/_javascript_Core/API/JSContextRef.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/API/JSContextRef.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/API/JSContextRef.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -99,9 +99,9 @@
Watchdog& watchdog = vm.ensureWatchdog();
if (callback) {
void* callbackPtr = reinterpret_cast<void*>(callback);
- watchdog.setTimeLimit(vm, std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)), internalScriptTimeoutCallback, callbackPtr, callbackData);
+ watchdog.setTimeLimit(std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)), internalScriptTimeoutCallback, callbackPtr, callbackData);
} else
- watchdog.setTimeLimit(vm, std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)));
+ watchdog.setTimeLimit(std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)));
}
void JSContextGroupClearExecutionTimeLimit(JSContextGroupRef group)
@@ -109,7 +109,7 @@
VM& vm = *toJS(group);
JSLockHolder locker(&vm);
if (vm.watchdog)
- vm.watchdog->setTimeLimit(vm, Watchdog::noTimeLimit);
+ vm.watchdog->setTimeLimit(Watchdog::noTimeLimit);
}
// From the API's perspective, a global context remains alive iff it has been JSGlobalContextRetained.
Modified: trunk/Source/_javascript_Core/API/tests/ExecutionTimeLimitTest.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/API/tests/ExecutionTimeLimitTest.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/API/tests/ExecutionTimeLimitTest.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -83,6 +83,22 @@
const char* optionsStr;
};
+static void testResetAfterTimeout(bool& failed)
+{
+ JSValueRef v = nullptr;
+ JSValueRef exception = nullptr;
+ const char* reentryScript = "100";
+ JSStringRef script = JSStringCreateWithUTF8CString(reentryScript);
+ v = JSEvaluateScript(context, script, nullptr, nullptr, 1, &exception);
+ if (exception) {
+ printf("FAIL: Watchdog timeout was not reset.\n");
+ failed = true;
+ } else if (!JSValueIsNumber(context, v) || JSValueToNumber(context, v, nullptr) != 100) {
+ printf("FAIL: Script result is not as expected.\n");
+ failed = true;
+ }
+}
+
int testExecutionTimeLimit()
{
static const TierOptions tierOptionsList[] = {
@@ -152,6 +168,8 @@
printf("FAIL: %s TerminatedExecutionException was not thrown.\n", tierOptions.tier);
failed = true;
}
+
+ testResetAfterTimeout(failed);
}
/* Test the script timeout's TerminatedExecutionException should NOT be catchable: */
@@ -187,6 +205,8 @@
printf("FAIL: %s TerminatedExecutionException was caught.\n", tierOptions.tier);
failed = true;
}
+
+ testResetAfterTimeout(failed);
}
/* Test script timeout with no callback: */
@@ -222,6 +242,8 @@
printf("FAIL: %s TerminatedExecutionException was not thrown.\n", tierOptions.tier);
failed = true;
}
+
+ testResetAfterTimeout(failed);
}
/* Test script timeout cancellation: */
Modified: trunk/Source/_javascript_Core/ChangeLog (189008 => 189009)
--- trunk/Source/_javascript_Core/ChangeLog 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-08-27 02:49:52 UTC (rev 189009)
@@ -1,3 +1,90 @@
+2015-08-26 Mark Lam <mark....@apple.com>
+
+ watchdog m_didFire state erroneously retained.
+ https://bugs.webkit.org/show_bug.cgi?id=131082
+
+ Reviewed by Geoffrey Garen.
+
+ The watchdog can fire for 2 reasons:
+ 1. an external controlling entity (i.e. another thread) has scheduled termination
+ of the script thread via watchdog::terminateSoon().
+ 2. the allowed CPU time has expired.
+
+ For case 1, we're doing away with the m_didFire flag. Watchdog::terminateSoon()
+ will set the timer deadlines and m_timeLimit to 0, and m_timerDidFire to true.
+ This will get the script thread to check Watchdog::didFire() and terminate
+ execution.
+
+ Note: the watchdog only guarantees that script execution will terminate as soon
+ as possible due to a time limit of 0. Once we've exited the VM, the client of the
+ VM is responsible from keeping a flag to prevent new script execution.
+
+ In a race condition, if terminateSoon() is called just after execution has gotten
+ past the client's reentry check and the client is in the process of re-entering,
+ the worst that can happen is that we will schedule the watchdog timer to fire
+ after a period of 0. This will terminate script execution quickly, and thereafter
+ the client's check should be able to prevent further entry into the VM.
+
+ The correctness (i.e. has no race condition) of this type of termination relies
+ on the termination state being sticky. Once the script thread is terminated this
+ way, the VM will continue to terminate scripts quickly until the client sets the
+ time limit to a non-zero value (or clears it which sets the time limit to
+ noTimeLimit).
+
+ For case 2, the watchdog does not alter m_timeLimit. If the CPU deadline has
+ been reached, the script thread will terminate execution and exit the VM.
+
+ If the client of the VM starts new script execution, the watchdog will allow
+ execution for the specified m_timeLimit. In this case, since m_timeLimit is not
+ 0, the script gets a fresh allowance of CPU time to execute. Hence, terminations
+ due to watchdog time outs are no longer sticky.
+
+ * API/JSContextRef.cpp:
+ (JSContextGroupSetExecutionTimeLimit):
+ (JSContextGroupClearExecutionTimeLimit):
+ * API/tests/ExecutionTimeLimitTest.cpp:
+ - Add test scenarios to verify that the watchdog is automatically reset by the VM
+ upon throwing the TerminatedExecutionException.
+
+ (testResetAfterTimeout):
+ (testExecutionTimeLimit):
+ * _javascript_Core.vcxproj/_javascript_Core.vcxproj:
+ * _javascript_Core.vcxproj/_javascript_Core.vcxproj.filters:
+ * _javascript_Core.xcodeproj/project.pbxproj:
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * interpreter/Interpreter.cpp:
+ (JSC::Interpreter::execute):
+ (JSC::Interpreter::executeCall):
+ (JSC::Interpreter::executeConstruct):
+ * jit/JITOpcodes.cpp:
+ (JSC::JIT::emit_op_loop_hint):
+ (JSC::JIT::emitSlow_op_loop_hint):
+ * jit/JITOperations.cpp:
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ (JSC::VM::ensureWatchdog):
+ * runtime/VM.h:
+ * runtime/VMInlines.h: Added.
+ (JSC::VM::shouldTriggerTermination):
+ * runtime/Watchdog.cpp:
+ (JSC::Watchdog::Watchdog):
+ (JSC::Watchdog::setTimeLimit):
+ (JSC::Watchdog::terminateSoon):
+ (JSC::Watchdog::didFireSlow):
+ (JSC::Watchdog::hasTimeLimit):
+ (JSC::Watchdog::enteredVM):
+ (JSC::Watchdog::exitedVM):
+ (JSC::Watchdog::startTimer):
+ (JSC::Watchdog::stopTimer):
+ (JSC::Watchdog::hasStartedTimer): Deleted.
+ (JSC::Watchdog::fire): Deleted.
+ * runtime/Watchdog.h:
+ (JSC::Watchdog::didFire):
+ (JSC::Watchdog::timerDidFireAddress):
+
2015-08-26 Joseph Pecoraro <pecor...@apple.com>
Web Inspector: Implement tracking of active stylesheets in the frontend
Modified: trunk/Source/_javascript_Core/_javascript_Core.vcxproj/_javascript_Core.vcxproj (189008 => 189009)
--- trunk/Source/_javascript_Core/_javascript_Core.vcxproj/_javascript_Core.vcxproj 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/_javascript_Core.vcxproj/_javascript_Core.vcxproj 2015-08-27 02:49:52 UTC (rev 189009)
@@ -1770,6 +1770,7 @@
<ClInclude Include="..\runtime\Uint32Array.h" />
<ClInclude Include="..\runtime\Uint8Array.h" />
<ClInclude Include="..\runtime\VM.h" />
+ <ClInclude Include="..\runtime\VMInlines.h" />
<ClInclude Include="..\runtime\VMEntryScope.h" />
<ClInclude Include="..\runtime\VarOffset.h" />
<ClInclude Include="..\runtime\Watchdog.h" />
Modified: trunk/Source/_javascript_Core/_javascript_Core.vcxproj/_javascript_Core.vcxproj.filters (189008 => 189009)
--- trunk/Source/_javascript_Core/_javascript_Core.vcxproj/_javascript_Core.vcxproj.filters 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/_javascript_Core.vcxproj/_javascript_Core.vcxproj.filters 2015-08-27 02:49:52 UTC (rev 189009)
@@ -3267,6 +3267,9 @@
<ClInclude Include="..\runtime\VM.h">
<Filter>runtime</Filter>
</ClInclude>
+ <ClInclude Include="..\runtime\VMInlines.h">
+ <Filter>runtime</Filter>
+ </ClInclude>
<ClInclude Include="..\runtime\VMEntryScope.h">
<Filter>runtime</Filter>
</ClInclude>
Modified: trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj (189008 => 189009)
--- trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2015-08-27 02:49:52 UTC (rev 189009)
@@ -3637,6 +3637,7 @@
FE5932A6183C5A2600A1ECCC /* VMEntryScope.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = VMEntryScope.h; sourceTree = "<group>"; };
FE7BA60D1A1A7CEC00F1F7B4 /* HeapVerifier.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HeapVerifier.cpp; sourceTree = "<group>"; };
FE7BA60E1A1A7CEC00F1F7B4 /* HeapVerifier.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HeapVerifier.h; sourceTree = "<group>"; };
+ FE90BB3A1B7CF64E006B3F03 /* VMInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = VMInlines.h; sourceTree = "<group>"; };
FEA0861E182B7A0400F6D851 /* Breakpoint.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Breakpoint.h; sourceTree = "<group>"; };
FEA0861F182B7A0400F6D851 /* DebuggerPrimitives.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DebuggerPrimitives.h; sourceTree = "<group>"; };
FEB51F6A1A97B688001F921C /* Regress141809.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = Regress141809.h; path = API/tests/Regress141809.h; sourceTree = "<group>"; };
@@ -4998,6 +4999,7 @@
0FE050241AA9095600D33B33 /* VarOffset.h */,
E18E3A570DF9278C00D90B34 /* VM.cpp */,
E18E3A560DF9278C00D90B34 /* VM.h */,
+ FE90BB3A1B7CF64E006B3F03 /* VMInlines.h */,
FE5932A5183C5A2600A1ECCC /* VMEntryScope.cpp */,
FE5932A6183C5A2600A1ECCC /* VMEntryScope.h */,
FED94F2B171E3E2300BE77A4 /* Watchdog.cpp */,
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -4107,7 +4107,7 @@
addToGraph(LoopHint);
- if (m_vm->watchdog && m_vm->watchdog->hasTimeLimit())
+ if (m_vm->watchdog)
addToGraph(CheckWatchdogTimer);
NEXT_OPCODE(op_loop_hint);
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -71,8 +71,8 @@
#include "StrongInlines.h"
#include "Symbol.h"
#include "VMEntryScope.h"
+#include "VMInlines.h"
#include "VirtualRegister.h"
-#include "Watchdog.h"
#include <limits.h>
#include <stdio.h>
@@ -865,7 +865,7 @@
ProgramCodeBlock* codeBlock = program->codeBlock();
- if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
+ if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
return throwTerminatedExecutionException(callFrame);
ASSERT(codeBlock->numParameters() == 1); // 1 parameter for 'this'.
@@ -928,7 +928,7 @@
} else
newCodeBlock = 0;
- if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
+ if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
return throwTerminatedExecutionException(callFrame);
ProtoCallFrame protoCallFrame;
@@ -998,7 +998,7 @@
} else
newCodeBlock = 0;
- if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
+ if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
return throwTerminatedExecutionException(callFrame);
ProtoCallFrame protoCallFrame;
@@ -1071,7 +1071,7 @@
if (LegacyProfiler* profiler = vm.enabledProfiler())
profiler->willExecute(closure.oldCallFrame, closure.function);
- if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(closure.oldCallFrame)))
+ if (UNLIKELY(vm.shouldTriggerTermination(closure.oldCallFrame)))
return throwTerminatedExecutionException(closure.oldCallFrame);
// Execute the code:
@@ -1152,7 +1152,7 @@
}
}
- if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
+ if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
return throwTerminatedExecutionException(callFrame);
ASSERT(codeBlock->numParameters() == 1); // 1 parameter for 'this'.
Modified: trunk/Source/_javascript_Core/jit/JITOpcodes.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/jit/JITOpcodes.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -915,7 +915,7 @@
}
// Emit the watchdog timer check:
- if (m_vm->watchdog && m_vm->watchdog->hasTimeLimit())
+ if (m_vm->watchdog)
addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_vm->watchdog->timerDidFireAddress())));
}
@@ -941,7 +941,7 @@
#endif
// Emit the slow path of the watchdog timer check:
- if (m_vm->watchdog && m_vm->watchdog->hasTimeLimit()) {
+ if (m_vm->watchdog) {
linkSlowCase(iter);
callOperation(operationHandleWatchdogTimer);
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -58,7 +58,7 @@
#include "ScopedArguments.h"
#include "TestRunnerUtils.h"
#include "TypeProfilerLog.h"
-#include "Watchdog.h"
+#include "VMInlines.h"
#include <wtf/InlineASM.h>
namespace JSC {
@@ -1094,7 +1094,7 @@
VM& vm = exec->vm();
NativeCallFrameTracer tracer(&vm, exec);
- if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(exec)))
+ if (UNLIKELY(vm.shouldTriggerTermination(exec)))
vm.throwException(exec, createTerminatedExecutionException(&vm));
return nullptr;
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -54,7 +54,7 @@
#include "ObjectConstructor.h"
#include "ProtoCallFrame.h"
#include "StructureRareDataInlines.h"
-#include "Watchdog.h"
+#include "VMInlines.h"
#include <wtf/StringPrintStream.h>
namespace JSC { namespace LLInt {
@@ -1303,7 +1303,7 @@
{
LLINT_BEGIN_NO_SET_PC();
ASSERT(vm.watchdog);
- if (UNLIKELY(vm.watchdog->didFire(exec)))
+ if (UNLIKELY(vm.shouldTriggerTermination(exec)))
LLINT_THROW(createTerminatedExecutionException(&vm));
LLINT_RETURN_TWO(0, exec);
}
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -292,7 +292,7 @@
if (Options::watchdog()) {
std::chrono::milliseconds timeoutMillis(Options::watchdog());
Watchdog& watchdog = ensureWatchdog();
- watchdog.setTimeLimit(*this, timeoutMillis);
+ watchdog.setTimeLimit(timeoutMillis);
}
}
@@ -388,6 +388,11 @@
// the LLINT assumes that the internal shape of a std::unique_ptr is the
// same as a plain C++ pointer, and loads the address of Watchdog from it.
RELEASE_ASSERT(*reinterpret_cast<Watchdog**>(&watchdog) == watchdog.get());
+
+ // And if we've previously compiled any functions, we need to revert
+ // them because they don't have the needed polling checks for the watchdog
+ // yet.
+ deleteAllCode();
}
return *watchdog;
}
Modified: trunk/Source/_javascript_Core/runtime/VM.h (189008 => 189009)
--- trunk/Source/_javascript_Core/runtime/VM.h 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2015-08-27 02:49:52 UTC (rev 189009)
@@ -236,7 +236,7 @@
static Ref<VM> createContextGroup(HeapType = SmallHeap);
JS_EXPORT_PRIVATE ~VM();
- Watchdog& ensureWatchdog();
+ JS_EXPORT_PRIVATE Watchdog& ensureWatchdog();
private:
RefPtr<JSLock> m_apiLock;
@@ -565,6 +565,8 @@
JS_EXPORT_PRIVATE void queueMicrotask(JSGlobalObject*, PassRefPtr<Microtask>);
JS_EXPORT_PRIVATE void drainMicrotasks();
+ inline bool shouldTriggerTermination(ExecState*);
+
private:
friend class LLIntOffsetsExtractor;
friend class ClearExceptionScope;
Added: trunk/Source/_javascript_Core/runtime/VMInlines.h (0 => 189009)
--- trunk/Source/_javascript_Core/runtime/VMInlines.h (rev 0)
+++ trunk/Source/_javascript_Core/runtime/VMInlines.h 2015-08-27 02:49:52 UTC (rev 189009)
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef VMInlines_h
+#define VMInlines_h
+
+#include "VM.h"
+#include "Watchdog.h"
+
+namespace JSC {
+
+bool VM::shouldTriggerTermination(ExecState* exec)
+{
+ if (!watchdog)
+ return false;
+ return watchdog->didFire(exec);
+}
+
+} // namespace JSC
+
+#endif // LLIntData_h
+
Modified: trunk/Source/_javascript_Core/runtime/Watchdog.cpp (189008 => 189009)
--- trunk/Source/_javascript_Core/runtime/Watchdog.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/runtime/Watchdog.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -42,7 +42,6 @@
Watchdog::Watchdog()
: m_timerDidFire(false)
- , m_didFire(false)
, m_timeLimit(noTimeLimit)
, m_cpuDeadline(noTimeLimit)
, m_wallClockDeadline(noTimeLimit)
@@ -52,127 +51,72 @@
, m_timerQueue(WorkQueue::create("jsc.watchdog.queue", WorkQueue::Type::Serial, WorkQueue::QOS::Utility))
{
m_timerHandler = [this] {
+ LockHolder locker(m_lock);
this->m_timerDidFire = true;
this->deref();
};
}
-inline bool Watchdog::hasStartedTimer()
-{
- return m_cpuDeadline != noTimeLimit;
-}
-
-void Watchdog::setTimeLimit(VM& vm, std::chrono::microseconds limit,
+void Watchdog::setTimeLimit(std::chrono::microseconds limit,
ShouldTerminateCallback callback, void* data1, void* data2)
{
- bool hadTimeLimit = hasTimeLimit();
+ LockHolder locker(m_lock);
- m_didFire = false; // Reset the watchdog.
-
m_timeLimit = limit;
m_callback = callback;
m_callbackData1 = data1;
m_callbackData2 = data2;
- // If this is the first time that time limit is being enabled, then any
- // previously JIT compiled code will not have the needed polling checks.
- // Hence, we need to flush all the pre-existing compiled code.
- //
- // However, if the time limit is already enabled, and we're just changing the
- // time limit value, then any existing JITted code will have the appropriate
- // polling checks. Hence, there is no need to re-do this flushing.
- if (!hadTimeLimit) {
- // And if we've previously compiled any functions, we need to revert
- // them because they don't have the needed polling checks yet.
- vm.deleteAllCode();
- }
-
if (m_hasEnteredVM && hasTimeLimit())
- startTimer(m_timeLimit);
+ startTimer(locker, m_timeLimit);
}
-bool Watchdog::didFireSlow(ExecState* exec)
+JS_EXPORT_PRIVATE void Watchdog::terminateSoon()
{
- ASSERT(m_timerDidFire);
- m_timerDidFire = false;
+ LockHolder locker(m_lock);
- // A legitimate timer is the timer which woke up watchdog and can caused didFireSlow() to be
- // called after m_wallClockDeadline has expired. All other timers are considered to be stale,
- // and their wake ups are considered to be spurious and should be ignored.
- //
- // didFireSlow() will race against m_timerHandler to clear and set m_timerDidFire respectively.
- // We need to deal with a variety of scenarios where we can have stale timers and legitimate
- // timers firing in close proximity to each other.
- //
- // Consider the following scenarios:
- //
- // 1. A stale timer fires long before m_wallClockDeadline expires.
- //
- // In this case, the m_wallClockDeadline check below will fail and the stale timer will be
- // ignored as spurious. We just need to make sure that we clear m_timerDidFire before we
- // check m_wallClockDeadline and return below.
- //
- // 2. A stale timer fires just before m_wallClockDeadline expires.
- // Before the watchdog can gets to the clearing m_timerDidFire above, the legit timer also fires.
- //
- // In this case, m_timerDidFire was set twice by the 2 timers, but we only clear need to clear
- // it once. The m_wallClockDeadline below will pass and this invocation of didFireSlow() will
- // be treated as the response to the legit timer. The spurious timer is effectively ignored.
- //
- // 3. A state timer fires just before m_wallClockDeadline expires.
- // Right after the watchdog clears m_timerDidFire above, the legit timer also fires.
- //
- // The fact that the legit timer fails means that the m_wallClockDeadline check will pass.
- // As long as we clear m_timerDidFire before we do the check, we are safe. This is the same
- // scenario as 2 above.
- //
- // 4. A stale timer fires just before m_wallClockDeadline expires.
- // Right after we do the m_wallClockDeadline check below, the legit timer fires.
- //
- // The fact that the legit timer fires after the m_wallClockDeadline check means that
- // the m_wallClockDeadline check will have failed. This is the same scenario as 1 above.
- //
- // 5. A legit timer fires.
- // A stale timer also fires before we clear m_timerDidFire above.
- //
- // This is the same scenario as 2 above.
- //
- // 6. A legit timer fires.
- // A stale timer fires right after we clear m_timerDidFire above.
- //
- // In this case, the m_wallClockDeadline check will pass, and we fire the watchdog
- // though m_timerDidFire remains set. This just means that didFireSlow() will be called again due
- // to m_timerDidFire being set. The subsequent invocation of didFireSlow() will end up handling
- // the stale timer and ignoring it. This is the same scenario as 1 above.
- //
- // 7. A legit timer fires.
- // A stale timer fires right after we do the m_wallClockDeadline check.
- //
- // This is the same as 6, which means it's the same as 1 above.
- //
- // In all these cases, we just need to ensure that we clear m_timerDidFire before we do the
- // m_wallClockDeadline check below. Hence, a storeLoad fence is needed to ensure that these 2
- // operations do not get re-ordered.
+ m_timeLimit = std::chrono::microseconds(0);
+ m_cpuDeadline = std::chrono::microseconds(0);
+ m_wallClockDeadline = std::chrono::microseconds(0);
+ m_timerDidFire = true;
+}
- WTF::storeLoadFence();
+bool Watchdog::didFireSlow(ExecState* exec)
+{
+ {
+ LockHolder locker(m_lock);
- if (currentWallClockTime() < m_wallClockDeadline)
- return false; // Just a stale timer firing. Nothing to do.
+ ASSERT(m_timerDidFire);
+ m_timerDidFire = false;
- m_wallClockDeadline = noTimeLimit;
+ if (currentWallClockTime() < m_wallClockDeadline)
+ return false; // Just a stale timer firing. Nothing to do.
- if (currentCPUTime() >= m_cpuDeadline) {
- // Case 1: the allowed CPU time has elapsed.
+ // Set m_wallClockDeadline to noTimeLimit here so that we can reject all future
+ // spurious wakes.
+ m_wallClockDeadline = noTimeLimit;
- // If m_callback is not set, then we terminate by default.
- // Else, we let m_callback decide if we should terminate or not.
- bool needsTermination = !m_callback
- || m_callback(exec, m_callbackData1, m_callbackData2);
- if (needsTermination) {
- m_didFire = true;
- return true;
+ auto cpuTime = currentCPUTime();
+ if (cpuTime < m_cpuDeadline) {
+ auto remainingCPUTime = m_cpuDeadline - cpuTime;
+ startTimer(locker, remainingCPUTime);
+ return false;
}
+ }
+ // Note: we should not be holding the lock while calling the callbacks. The callbacks may
+ // call setTimeLimit() which will try to lock as well.
+
+ // If m_callback is not set, then we terminate by default.
+ // Else, we let m_callback decide if we should terminate or not.
+ bool needsTermination = !m_callback
+ || m_callback(exec, m_callbackData1, m_callbackData2);
+ if (needsTermination)
+ return true;
+
+ {
+ LockHolder locker(m_lock);
+
// If we get here, then the callback above did not want to terminate execution. As a
// result, the callback may have done one of the following:
// 1. cleared the time limit (i.e. watchdog is disabled),
@@ -184,15 +128,10 @@
// In the case of 3, we need to re-start the timer here.
ASSERT(m_hasEnteredVM);
- if (hasTimeLimit() && !hasStartedTimer())
- startTimer(m_timeLimit);
-
- } else {
- // Case 2: the allowed CPU time has NOT elapsed.
- auto remainingCPUTime = m_cpuDeadline - currentCPUTime();
- startTimer(remainingCPUTime);
+ bool callbackAlreadyStartedTimer = (m_cpuDeadline != noTimeLimit);
+ if (hasTimeLimit() && !callbackAlreadyStartedTimer)
+ startTimer(locker, m_timeLimit);
}
-
return false;
}
@@ -201,54 +140,45 @@
return (m_timeLimit != noTimeLimit);
}
-void Watchdog::fire()
-{
- m_didFire = true;
-}
-
void Watchdog::enteredVM()
{
m_hasEnteredVM = true;
- if (hasTimeLimit())
- startTimer(m_timeLimit);
+ if (hasTimeLimit()) {
+ LockHolder locker(m_lock);
+ startTimer(locker, m_timeLimit);
+ }
}
void Watchdog::exitedVM()
{
ASSERT(m_hasEnteredVM);
- stopTimer();
+ LockHolder locker(m_lock);
+ stopTimer(locker);
m_hasEnteredVM = false;
}
-void Watchdog::startTimer(std::chrono::microseconds timeLimit)
+void Watchdog::startTimer(LockHolder&, std::chrono::microseconds timeLimit)
{
ASSERT(m_hasEnteredVM);
ASSERT(hasTimeLimit());
+ ASSERT(timeLimit <= m_timeLimit);
m_cpuDeadline = currentCPUTime() + timeLimit;
auto wallClockTime = currentWallClockTime();
auto wallClockDeadline = wallClockTime + timeLimit;
if ((wallClockTime < m_wallClockDeadline)
- && (m_wallClockDeadline <= wallClockDeadline)) {
+ && (m_wallClockDeadline <= wallClockDeadline))
return; // Wait for the current active timer to expire before starting a new one.
- }
// Else, the current active timer won't fire soon enough. So, start a new timer.
this->ref(); // m_timerHandler will deref to match later.
m_wallClockDeadline = wallClockDeadline;
- m_timerDidFire = false;
- // We clear m_timerDidFire because we're starting a new timer. However, we need to make sure
- // that the clearing occurs before the timer thread is started. Thereafter, only didFireSlow()
- // should clear m_timerDidFire (unless we start yet another timer). Hence, we need a storeStore
- // fence here to ensure these operations do not get re-ordered.
- WTF::storeStoreFence();
-
m_timerQueue->dispatchAfter(std::chrono::nanoseconds(timeLimit), m_timerHandler);
}
-void Watchdog::stopTimer()
+void Watchdog::stopTimer(LockHolder&)
{
m_cpuDeadline = noTimeLimit;
}
Modified: trunk/Source/_javascript_Core/runtime/Watchdog.h (189008 => 189009)
--- trunk/Source/_javascript_Core/runtime/Watchdog.h 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/_javascript_Core/runtime/Watchdog.h 2015-08-27 02:49:52 UTC (rev 189009)
@@ -26,7 +26,7 @@
#ifndef Watchdog_h
#define Watchdog_h
-#include <mutex>
+#include <wtf/Lock.h>
#include <wtf/Ref.h>
#include <wtf/ThreadSafeRefCounted.h>
#include <wtf/WorkQueue.h>
@@ -44,14 +44,11 @@
Watchdog();
typedef bool (*ShouldTerminateCallback)(ExecState*, void* data1, void* data2);
- void setTimeLimit(VM&, std::chrono::microseconds limit, ShouldTerminateCallback = 0, void* data1 = 0, void* data2 = 0);
+ void setTimeLimit(std::chrono::microseconds limit, ShouldTerminateCallback = 0, void* data1 = 0, void* data2 = 0);
+ JS_EXPORT_PRIVATE void terminateSoon();
- // This version of didFire() will check the elapsed CPU time and call the
- // callback (if needed) to determine if the watchdog should fire.
bool didFire(ExecState* exec)
{
- if (m_didFire)
- return true;
if (!m_timerDidFire)
return false;
return didFireSlow(exec);
@@ -61,22 +58,14 @@
void enteredVM();
void exitedVM();
- // This version of didFire() is a more efficient version for when we want
- // to know if the watchdog has fired in the past, and not whether it should
- // fire right now.
- bool didFire() { return m_didFire; }
- JS_EXPORT_PRIVATE void fire();
-
void* timerDidFireAddress() { return &m_timerDidFire; }
static const std::chrono::microseconds noTimeLimit;
private:
- void startTimer(std::chrono::microseconds timeLimit);
- void stopTimer();
+ void startTimer(LockHolder&, std::chrono::microseconds timeLimit);
+ void stopTimer(LockHolder&);
- inline bool hasStartedTimer();
-
bool didFireSlow(ExecState*);
// m_timerDidFire indicates whether the timer fired. The Watchdog
@@ -85,13 +74,16 @@
// NOTE: m_timerDidFire is only set by the platform specific timer
// (probably from another thread) but is only cleared in the script thread.
bool m_timerDidFire;
- bool m_didFire;
std::chrono::microseconds m_timeLimit;
std::chrono::microseconds m_cpuDeadline;
std::chrono::microseconds m_wallClockDeadline;
+ // Writes to m_timerDidFire and m_timeLimit, and Reads+Writes to m_cpuDeadline and m_wallClockDeadline
+ // must be guarded by this lock.
+ Lock m_lock;
+
bool m_hasEnteredVM { false };
ShouldTerminateCallback m_callback;
Modified: trunk/Source/WebCore/ChangeLog (189008 => 189009)
--- trunk/Source/WebCore/ChangeLog 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/WebCore/ChangeLog 2015-08-27 02:49:52 UTC (rev 189009)
@@ -1,3 +1,26 @@
+2015-08-26 Mark Lam <mark....@apple.com>
+
+ watchdog m_didFire state erroneously retained.
+ https://bugs.webkit.org/show_bug.cgi?id=131082
+
+ Reviewed by Geoffrey Garen.
+
+ No new tests. The new code is covered by the JSC API tests and an existing test:
+ fast/workers/worker-terminate-forever.html
+
+ * bindings/js/JSEventListener.cpp:
+ (WebCore::JSEventListener::handleEvent):
+ * bindings/js/WorkerScriptController.cpp:
+ (WebCore::WorkerScriptController::WorkerScriptController):
+ - Always create a watchdog for the Web Worker's VM. We need this in order to support
+ Worker.terminate().
+ (WebCore::WorkerScriptController::evaluate):
+ (WebCore::WorkerScriptController::scheduleExecutionTermination):
+ (WebCore::WorkerScriptController::isTerminatingExecution):
+ (WebCore::WorkerScriptController::forbidExecution):
+ (WebCore::WorkerScriptController::isExecutionTerminating): Deleted.
+ * bindings/js/WorkerScriptController.h:
+
2015-08-26 Myles C. Maxfield <mmaxfi...@apple.com>
[Cocoa] PerformanceTest Layout/RegionsShapes.html is failing
Modified: trunk/Source/WebCore/bindings/js/JSEventListener.cpp (189008 => 189009)
--- trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/WebCore/bindings/js/JSEventListener.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -135,9 +135,10 @@
globalObject->setCurrentEvent(savedEvent);
if (is<WorkerGlobalScope>(*scriptExecutionContext)) {
+ auto scriptController = downcast<WorkerGlobalScope>(*scriptExecutionContext).script();
bool terminatorCausedException = (exec->hadException() && isTerminatedExecutionException(exec->exception()));
- if (terminatorCausedException || (vm.watchdog && vm.watchdog->didFire()))
- downcast<WorkerGlobalScope>(*scriptExecutionContext).script()->forbidExecution();
+ if (terminatorCausedException || scriptController->isTerminatingExecution())
+ scriptController->forbidExecution();
}
if (exception) {
Modified: trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp (189008 => 189009)
--- trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp 2015-08-27 02:49:52 UTC (rev 189009)
@@ -56,6 +56,7 @@
, m_workerGlobalScopeWrapper(*m_vm)
, m_executionForbidden(false)
{
+ m_vm->ensureWatchdog();
initNormalWorldClientData(m_vm.get());
}
@@ -121,8 +122,7 @@
JSC::evaluate(exec, sourceCode.jsSourceCode(), m_workerGlobalScopeWrapper->globalThis(), returnedException);
VM& vm = exec->vm();
- if ((returnedException && isTerminatedExecutionException(returnedException))
- || (vm.watchdog && vm.watchdog->didFire())) {
+ if ((returnedException && isTerminatedExecutionException(returnedException)) || isTerminatingExecution()) {
forbidExecution();
return;
}
@@ -149,20 +149,20 @@
void WorkerScriptController::scheduleExecutionTermination()
{
// The mutex provides a memory barrier to ensure that once
- // termination is scheduled, isExecutionTerminating will
+ // termination is scheduled, isTerminatingExecution() will
// accurately reflect that state when called from another thread.
LockHolder locker(m_scheduledTerminationMutex);
- if (m_vm->watchdog)
- m_vm->watchdog->fire();
+ m_isTerminatingExecution = true;
+
+ ASSERT(m_vm->watchdog);
+ m_vm->watchdog->terminateSoon();
}
-bool WorkerScriptController::isExecutionTerminating() const
+bool WorkerScriptController::isTerminatingExecution() const
{
// See comments in scheduleExecutionTermination regarding mutex usage.
LockHolder locker(m_scheduledTerminationMutex);
- if (m_vm->watchdog)
- return m_vm->watchdog->didFire();
- return false;
+ return m_isTerminatingExecution;
}
void WorkerScriptController::forbidExecution()
Modified: trunk/Source/WebCore/bindings/js/WorkerScriptController.h (189008 => 189009)
--- trunk/Source/WebCore/bindings/js/WorkerScriptController.h 2015-08-27 01:28:06 UTC (rev 189008)
+++ trunk/Source/WebCore/bindings/js/WorkerScriptController.h 2015-08-27 02:49:52 UTC (rev 189009)
@@ -71,7 +71,7 @@
// forbidExecution()/isExecutionForbidden() to guard against reentry into JS.
// Can be called from any thread.
void scheduleExecutionTermination();
- bool isExecutionTerminating() const;
+ bool isTerminatingExecution() const;
// Called on Worker thread when JS exits with termination exception caused by forbidExecution() request,
// or by Worker thread termination code to prevent future entry into JS.
@@ -97,6 +97,7 @@
WorkerGlobalScope* m_workerGlobalScope;
JSC::Strong<JSWorkerGlobalScope> m_workerGlobalScopeWrapper;
bool m_executionForbidden;
+ bool m_isTerminatingExecution { false };
mutable Lock m_scheduledTerminationMutex;
};
