Changeset 189009 in webkit


Ignore:
Timestamp:
Aug 26, 2015, 7:49:52 PM (10 years ago)
Author:
mark.lam@apple.com
Message:

watchdog m_didFire state erroneously retained.
​https://bugs.webkit.org/show_bug.cgi?id=131082

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

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):

(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):

Source/WebCore:

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:

LayoutTests:

  • fast/workers/worker-terminate-forever-expected.txt:
  • fast/workers/worker-terminate-forever.html:
  • Updated to check if the worker actually did terminate.
Location:
trunk
Files:
1 added
22 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r189004 r189009  
     12015-08-26  Mark Lam  <mark.lam@apple.com>
     2
     3        watchdog m_didFire state erroneously retained.
     4        https://bugs.webkit.org/show_bug.cgi?id=131082
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        * fast/workers/worker-terminate-forever-expected.txt:
     9        * fast/workers/worker-terminate-forever.html:
     10        - Updated to check if the worker actually did terminate.
     11
    1122015-08-26  Andy Estes  <aestes@apple.com>
    213

  • trunk/LayoutTests/fast/workers/worker-terminate-forever-expected.txt

    r57192 r189009  
     1CONSOLE MESSAGE: line 15: Worker was started
     2CONSOLE MESSAGE: line 34: Worker was terminated
    13Test Worker.terminate() for a worker that tries to run forever.

  • trunk/LayoutTests/fast/workers/worker-terminate-forever.html

    r124680 r189009  
    22<p>Test Worker.terminate() for a worker that tries to run forever.</p>
    33<script>
    4 if (window.testRunner)
     4if (window.testRunner) {
    55    testRunner.dumpAsText();
     6    testRunner.waitUntilDone();
     7}
    68
    79var worker = new Worker('resources/worker-run-forever.js');
    8 worker.terminate();
     10
     11function waitForWorkerToStart() {
     12    var startTime = Date.now();
     13    function checkIfWorkerStarted() {
     14        if (internals.workerThreadCount == 1) {
     15            console.log("Worker was started");
     16            worker.terminate();
     17            setTimeout(waitForWorkerToStop, 0);
     18
     19        } else if (Date.now() - startTime < 5000) {
     20            setTimeout(checkIfWorkerStarted, 0);
     21
     22        } else {
     23            console.log("Worker did not show up");
     24            testRunner.notifyDone();
     25        }           
     26    }
     27    setTimeout(checkIfWorkerStarted, 0);
     28}
     29
     30function waitForWorkerToStop() {
     31    var startTime = Date.now();
     32    function checkIfWorkerStopped() {
     33        if (internals.workerThreadCount == 0) {
     34            console.log("Worker was terminated");
     35            testRunner.notifyDone();
     36
     37        } else if (Date.now() - startTime < 5000) {
     38            setTimeout(checkIfWorkerStopped, 0);
     39
     40        } else {
     41            console.log("Did not see worker terminate");
     42            testRunner.notifyDone();
     43        }           
     44    }
     45    setTimeout(checkIfWorkerStopped, 0);
     46}
     47
     48window.setTimeout(waitForWorkerToStart, 0);
     49
    950</script>
    1051</body>

  • trunk/Source/JavaScriptCore/API/JSContextRef.cpp

    r188338 r189009  
    100100    if (callback) {
    101101        void* callbackPtr = reinterpret_cast<void*>(callback);
    102         watchdog.setTimeLimit(vm, std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)), internalScriptTimeoutCallback, callbackPtr, callbackData);
     102        watchdog.setTimeLimit(std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)), internalScriptTimeoutCallback, callbackPtr, callbackData);
    103103    } else
    104         watchdog.setTimeLimit(vm, std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)));
     104        watchdog.setTimeLimit(std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(limit)));
    105105}
    106106
    … …  
    110110    JSLockHolder locker(&vm);
    111111    if (vm.watchdog)
    112         vm.watchdog->setTimeLimit(vm, Watchdog::noTimeLimit);
     112        vm.watchdog->setTimeLimit(Watchdog::noTimeLimit);
    113113}
    114114

  • trunk/Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp

    r188689 r189009  
    8484};
    8585
     86static void testResetAfterTimeout(bool& failed)
     87{
     88    JSValueRef v = nullptr;
     89    JSValueRef exception = nullptr;
     90    const char* reentryScript = "100";
     91    JSStringRef script = JSStringCreateWithUTF8CString(reentryScript);
     92    v = JSEvaluateScript(context, script, nullptr, nullptr, 1, &exception);
     93    if (exception) {
     94        printf("FAIL: Watchdog timeout was not reset.\n");
     95        failed = true;
     96    } else if (!JSValueIsNumber(context, v) || JSValueToNumber(context, v, nullptr) != 100) {
     97        printf("FAIL: Script result is not as expected.\n");
     98        failed = true;
     99    }
     100}
     101
    86102int testExecutionTimeLimit()
    87103{
    … …  
    153169                failed = true;
    154170            }
     171
     172            testResetAfterTimeout(failed);
    155173        }
    156174
    … …  
    188206                failed = true;
    189207            }
     208
     209            testResetAfterTimeout(failed);
    190210        }
    191211       
    … …  
    223243                failed = true;
    224244            }
     245
     246            testResetAfterTimeout(failed);
    225247        }
    226248       

  • trunk/Source/JavaScriptCore/ChangeLog

    r189002 r189009  
     12015-08-26  Mark Lam  <mark.lam@apple.com>
     2
     3        watchdog m_didFire state erroneously retained.
     4        https://bugs.webkit.org/show_bug.cgi?id=131082
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        The watchdog can fire for 2 reasons:
     9        1. an external controlling entity (i.e. another thread) has scheduled termination
     10           of the script thread via watchdog::terminateSoon().
     11        2. the allowed CPU time has expired.
     12
     13        For case 1, we're doing away with the m_didFire flag.  Watchdog::terminateSoon()
     14        will set the timer deadlines and m_timeLimit to 0, and m_timerDidFire to true.
     15        This will get the script thread to check Watchdog::didFire() and terminate
     16        execution.
     17
     18        Note: the watchdog only guarantees that script execution will terminate as soon
     19        as possible due to a time limit of 0.  Once we've exited the VM, the client of the
     20        VM is responsible from keeping a flag to prevent new script execution.
     21
     22        In a race condition, if terminateSoon() is called just after execution has gotten
     23        past the client's reentry check and the client is in the process of re-entering,
     24        the worst that can happen is that we will schedule the watchdog timer to fire
     25        after a period of 0.  This will terminate script execution quickly, and thereafter
     26        the client's check should be able to prevent further entry into the VM.
     27
     28        The correctness (i.e. has no race condition) of this type of termination relies
     29        on the termination state being sticky.  Once the script thread is terminated this
     30        way, the VM will continue to terminate scripts quickly until the client sets the
     31        time limit to a non-zero value (or clears it which sets the time limit to
     32        noTimeLimit).
     33
     34        For case 2, the watchdog does not alter m_timeLimit.  If the CPU deadline has
     35        been reached, the script thread will terminate execution and exit the VM.
     36
     37        If the client of the VM starts new script execution, the watchdog will allow
     38        execution for the specified m_timeLimit.  In this case, since m_timeLimit is not
     39        0, the script gets a fresh allowance of CPU time to execute.  Hence, terminations
     40        due to watchdog time outs are no longer sticky.
     41
     42        * API/JSContextRef.cpp:
     43        (JSContextGroupSetExecutionTimeLimit):
     44        (JSContextGroupClearExecutionTimeLimit):
     45        * API/tests/ExecutionTimeLimitTest.cpp:
     46        - Add test scenarios to verify that the watchdog is automatically reset by the VM
     47          upon throwing the TerminatedExecutionException.
     48
     49        (testResetAfterTimeout):
     50        (testExecutionTimeLimit):
     51        * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
     52        * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
     53        * JavaScriptCore.xcodeproj/project.pbxproj:
     54        * dfg/DFGByteCodeParser.cpp:
     55        (JSC::DFG::ByteCodeParser::parseBlock):
     56        * interpreter/Interpreter.cpp:
     57        (JSC::Interpreter::execute):
     58        (JSC::Interpreter::executeCall):
     59        (JSC::Interpreter::executeConstruct):
     60        * jit/JITOpcodes.cpp:
     61        (JSC::JIT::emit_op_loop_hint):
     62        (JSC::JIT::emitSlow_op_loop_hint):
     63        * jit/JITOperations.cpp:
     64        * llint/LLIntSlowPaths.cpp:
     65        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
     66        * runtime/VM.cpp:
     67        (JSC::VM::VM):
     68        (JSC::VM::ensureWatchdog):
     69        * runtime/VM.h:
     70        * runtime/VMInlines.h: Added.
     71        (JSC::VM::shouldTriggerTermination):
     72        * runtime/Watchdog.cpp:
     73        (JSC::Watchdog::Watchdog):
     74        (JSC::Watchdog::setTimeLimit):
     75        (JSC::Watchdog::terminateSoon):
     76        (JSC::Watchdog::didFireSlow):
     77        (JSC::Watchdog::hasTimeLimit):
     78        (JSC::Watchdog::enteredVM):
     79        (JSC::Watchdog::exitedVM):
     80        (JSC::Watchdog::startTimer):
     81        (JSC::Watchdog::stopTimer):
     82        (JSC::Watchdog::hasStartedTimer): Deleted.
     83        (JSC::Watchdog::fire): Deleted.
     84        * runtime/Watchdog.h:
     85        (JSC::Watchdog::didFire):
     86        (JSC::Watchdog::timerDidFireAddress):
     87
    1882015-08-26  Joseph Pecoraro  <pecoraro@apple.com>
    289

  • trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj

    r188979 r189009  
    17711771    <ClInclude Include="..\runtime\Uint8Array.h" />
    17721772    <ClInclude Include="..\runtime\VM.h" />
     1773    <ClInclude Include="..\runtime\VMInlines.h" />
    17731774    <ClInclude Include="..\runtime\VMEntryScope.h" />
    17741775    <ClInclude Include="..\runtime\VarOffset.h" />

  • trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters

    r188796 r189009  
    32683268      <Filter>runtime</Filter>
    32693269    </ClInclude>
     3270    <ClInclude Include="..\runtime\VMInlines.h">
     3271      <Filter>runtime</Filter>
     3272    </ClInclude>
    32703273    <ClInclude Include="..\runtime\VMEntryScope.h">
    32713274      <Filter>runtime</Filter>

  • trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r188979 r189009  
    36383638                FE7BA60D1A1A7CEC00F1F7B4 /* HeapVerifier.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HeapVerifier.cpp; sourceTree = "<group>"; };
    36393639                FE7BA60E1A1A7CEC00F1F7B4 /* HeapVerifier.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HeapVerifier.h; sourceTree = "<group>"; };
     3640                FE90BB3A1B7CF64E006B3F03 /* VMInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = VMInlines.h; sourceTree = "<group>"; };
    36403641                FEA0861E182B7A0400F6D851 /* Breakpoint.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Breakpoint.h; sourceTree = "<group>"; };
    36413642                FEA0861F182B7A0400F6D851 /* DebuggerPrimitives.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DebuggerPrimitives.h; sourceTree = "<group>"; };
    … …  
    49995000                                E18E3A570DF9278C00D90B34 /* VM.cpp */,
    50005001                                E18E3A560DF9278C00D90B34 /* VM.h */,
     5002                                FE90BB3A1B7CF64E006B3F03 /* VMInlines.h */,
    50015003                                FE5932A5183C5A2600A1ECCC /* VMEntryScope.cpp */,
    50025004                                FE5932A6183C5A2600A1ECCC /* VMEntryScope.h */,

  • trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

    r188979 r189009  
    41084108            addToGraph(LoopHint);
    41094109           
    4110             if (m_vm->watchdog && m_vm->watchdog->hasTimeLimit())
     4110            if (m_vm->watchdog)
    41114111                addToGraph(CheckWatchdogTimer);
    41124112           

  • trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

    r188583 r189009  
    7272#include "Symbol.h"
    7373#include "VMEntryScope.h"
     74#include "VMInlines.h"
    7475#include "VirtualRegister.h"
    75 #include "Watchdog.h"
    7676
    7777#include <limits.h>
    … …  
    866866    ProgramCodeBlock* codeBlock = program->codeBlock();
    867867
    868     if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
     868    if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
    869869        return throwTerminatedExecutionException(callFrame);
    870870
    … …  
    929929        newCodeBlock = 0;
    930930
    931     if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
     931    if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
    932932        return throwTerminatedExecutionException(callFrame);
    933933
    … …  
    999999        newCodeBlock = 0;
    10001000
    1001     if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
     1001    if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
    10021002        return throwTerminatedExecutionException(callFrame);
    10031003
    … …  
    10721072        profiler->willExecute(closure.oldCallFrame, closure.function);
    10731073
    1074     if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(closure.oldCallFrame)))
     1074    if (UNLIKELY(vm.shouldTriggerTermination(closure.oldCallFrame)))
    10751075        return throwTerminatedExecutionException(closure.oldCallFrame);
    10761076
    … …  
    11531153    }
    11541154
    1155     if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(callFrame)))
     1155    if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
    11561156        return throwTerminatedExecutionException(callFrame);
    11571157

  • trunk/Source/JavaScriptCore/jit/JITOpcodes.cpp

    r188545 r189009  
    916916
    917917    // Emit the watchdog timer check:
    918     if (m_vm->watchdog && m_vm->watchdog->hasTimeLimit())
     918    if (m_vm->watchdog)
    919919        addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_vm->watchdog->timerDidFireAddress())));
    920920}
    … …  
    942942
    943943    // Emit the slow path of the watchdog timer check:
    944     if (m_vm->watchdog && m_vm->watchdog->hasTimeLimit()) {
     944    if (m_vm->watchdog) {
    945945        linkSlowCase(iter);
    946946        callOperation(operationHandleWatchdogTimer);

  • trunk/Source/JavaScriptCore/jit/JITOperations.cpp

    r188932 r189009  
    5959#include "TestRunnerUtils.h"
    6060#include "TypeProfilerLog.h"
    61 #include "Watchdog.h"
     61#include "VMInlines.h"
    6262#include <wtf/InlineASM.h>
    6363
    … …  
    10951095    NativeCallFrameTracer tracer(&vm, exec);
    10961096
    1097     if (UNLIKELY(vm.watchdog && vm.watchdog->didFire(exec)))
     1097    if (UNLIKELY(vm.shouldTriggerTermination(exec)))
    10981098        vm.throwException(exec, createTerminatedExecutionException(&vm));
    10991099

  • trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

    r188545 r189009  
    5555#include "ProtoCallFrame.h"
    5656#include "StructureRareDataInlines.h"
    57 #include "Watchdog.h"
     57#include "VMInlines.h"
    5858#include <wtf/StringPrintStream.h>
    5959
    … …  
    13041304    LLINT_BEGIN_NO_SET_PC();
    13051305    ASSERT(vm.watchdog);
    1306     if (UNLIKELY(vm.watchdog->didFire(exec)))
     1306    if (UNLIKELY(vm.shouldTriggerTermination(exec)))
    13071307        LLINT_THROW(createTerminatedExecutionException(&vm));
    13081308    LLINT_RETURN_TWO(0, exec);

  • trunk/Source/JavaScriptCore/runtime/VM.cpp

    r188884 r189009  
    293293        std::chrono::milliseconds timeoutMillis(Options::watchdog());
    294294        Watchdog& watchdog = ensureWatchdog();
    295         watchdog.setTimeLimit(*this, timeoutMillis);
     295        watchdog.setTimeLimit(timeoutMillis);
    296296    }
    297297}
    … …  
    389389        // same as a plain C++ pointer, and loads the address of Watchdog from it.
    390390        RELEASE_ASSERT(*reinterpret_cast<Watchdog**>(&watchdog) == watchdog.get());
     391
     392        // And if we've previously compiled any functions, we need to revert
     393        // them because they don't have the needed polling checks for the watchdog
     394        // yet.
     395        deleteAllCode();
    391396    }
    392397    return *watchdog;

  • trunk/Source/JavaScriptCore/runtime/VM.h

    r188846 r189009  
    237237    JS_EXPORT_PRIVATE ~VM();
    238238
    239     Watchdog& ensureWatchdog();
     239    JS_EXPORT_PRIVATE Watchdog& ensureWatchdog();
    240240
    241241private:
    … …  
    566566    JS_EXPORT_PRIVATE void drainMicrotasks();
    567567
     568    inline bool shouldTriggerTermination(ExecState*);
     569
    568570private:
    569571    friend class LLIntOffsetsExtractor;

  • trunk/Source/JavaScriptCore/runtime/Watchdog.cpp

    r188394 r189009  
    4343Watchdog::Watchdog()
    4444    : m_timerDidFire(false)
    45     , m_didFire(false)
    4645    , m_timeLimit(noTimeLimit)
    4746    , m_cpuDeadline(noTimeLimit)
    … …  
    5352{
    5453    m_timerHandler = [this] {
     54        LockHolder locker(m_lock);
    5555        this->m_timerDidFire = true;
    5656        this->deref();
    … …  
    5858}
    5959
    60 inline bool Watchdog::hasStartedTimer()
    61 {
    62     return m_cpuDeadline != noTimeLimit;
    63 }
    64 
    65 void Watchdog::setTimeLimit(VM& vm, std::chrono::microseconds limit,
     60void Watchdog::setTimeLimit(std::chrono::microseconds limit,
    6661    ShouldTerminateCallback callback, void* data1, void* data2)
    6762{
    68     bool hadTimeLimit = hasTimeLimit();
    69 
    70     m_didFire = false; // Reset the watchdog.
     63    LockHolder locker(m_lock);
    7164
    7265    m_timeLimit = limit;
    … …  
    7568    m_callbackData2 = data2;
    7669
    77     // If this is the first time that time limit is being enabled, then any
    78     // previously JIT compiled code will not have the needed polling checks.
    79     // Hence, we need to flush all the pre-existing compiled code.
    80     //
    81     // However, if the time limit is already enabled, and we're just changing the
    82     // time limit value, then any existing JITted code will have the appropriate
    83     // polling checks. Hence, there is no need to re-do this flushing.
    84     if (!hadTimeLimit) {
    85         // And if we've previously compiled any functions, we need to revert
    86         // them because they don't have the needed polling checks yet.
    87         vm.deleteAllCode();
    88     }
     70    if (m_hasEnteredVM && hasTimeLimit())
     71        startTimer(locker, m_timeLimit);
     72}
    8973
    90     if (m_hasEnteredVM && hasTimeLimit())
    91         startTimer(m_timeLimit);
     74JS_EXPORT_PRIVATE void Watchdog::terminateSoon()
     75{
     76    LockHolder locker(m_lock);
     77
     78    m_timeLimit = std::chrono::microseconds(0);
     79    m_cpuDeadline = std::chrono::microseconds(0);
     80    m_wallClockDeadline = std::chrono::microseconds(0);
     81    m_timerDidFire = true;
    9282}
    9383
    9484bool Watchdog::didFireSlow(ExecState* exec)
    9585{
    96     ASSERT(m_timerDidFire);
    97     m_timerDidFire = false;
     86    {
     87        LockHolder locker(m_lock);
    9888
    99     // A legitimate timer is the timer which woke up watchdog and can caused didFireSlow() to be
    100     // called after m_wallClockDeadline has expired. All other timers are considered to be stale,
    101     // and their wake ups are considered to be spurious and should be ignored.
    102     //
    103     // didFireSlow() will race against m_timerHandler to clear and set m_timerDidFire respectively.
    104     // We need to deal with a variety of scenarios where we can have stale timers and legitimate
    105     // timers firing in close proximity to each other.
    106     //
    107     // Consider the following scenarios:
    108     //
    109     // 1. A stale timer fires long before m_wallClockDeadline expires.
    110     //
    111     //    In this case, the m_wallClockDeadline check below will fail and the stale timer will be
    112     //    ignored as spurious. We just need to make sure that we clear m_timerDidFire before we
    113     //    check m_wallClockDeadline and return below.
    114     //
    115     // 2. A stale timer fires just before m_wallClockDeadline expires.
    116     //    Before the watchdog can gets to the clearing m_timerDidFire above, the legit timer also fires.
    117     //
    118     //    In this case, m_timerDidFire was set twice by the 2 timers, but we only clear need to clear
    119     //    it once. The m_wallClockDeadline below will pass and this invocation of didFireSlow() will
    120     //    be treated as the response to the legit timer. The spurious timer is effectively ignored.
    121     //
    122     // 3. A state timer fires just before m_wallClockDeadline expires.
    123     //    Right after the watchdog clears m_timerDidFire above, the legit timer also fires.
    124     //
    125     //    The fact that the legit timer fails means that the m_wallClockDeadline check will pass.
    126     //    As long as we clear m_timerDidFire before we do the check, we are safe. This is the same
    127     //    scenario as 2 above.
    128     //
    129     // 4. A stale timer fires just before m_wallClockDeadline expires.
    130     //    Right after we do the m_wallClockDeadline check below, the legit timer fires.
    131     //
    132     //    The fact that the legit timer fires after the m_wallClockDeadline check means that
    133     //    the m_wallClockDeadline check will have failed. This is the same scenario as 1 above.
    134     //
    135     // 5. A legit timer fires.
    136     //    A stale timer also fires before we clear m_timerDidFire above.
    137     //
    138     //    This is the same scenario as 2 above.
    139     //
    140     // 6. A legit timer fires.
    141     //    A stale timer fires right after we clear m_timerDidFire above.
    142     //
    143     //    In this case, the m_wallClockDeadline check will pass, and we fire the watchdog
    144     //    though m_timerDidFire remains set. This just means that didFireSlow() will be called again due
    145     //    to m_timerDidFire being set. The subsequent invocation of didFireSlow() will end up handling
    146     //    the stale timer and ignoring it. This is the same scenario as 1 above.
    147     //
    148     // 7. A legit timer fires.
    149     //    A stale timer fires right after we do the m_wallClockDeadline check.
    150     //
    151     //    This is the same as 6, which means it's the same as 1 above.
    152     //
    153     // In all these cases, we just need to ensure that we clear m_timerDidFire before we do the
    154     // m_wallClockDeadline check below. Hence, a storeLoad fence is needed to ensure that these 2
    155     // operations do not get re-ordered.
     89        ASSERT(m_timerDidFire);
     90        m_timerDidFire = false;
    15691
    157     WTF::storeLoadFence();
     92        if (currentWallClockTime() < m_wallClockDeadline)
     93            return false; // Just a stale timer firing. Nothing to do.
    15894
    159     if (currentWallClockTime() < m_wallClockDeadline)
    160         return false; // Just a stale timer firing. Nothing to do.
     95        // Set m_wallClockDeadline to noTimeLimit here so that we can reject all future
     96        // spurious wakes.
     97        m_wallClockDeadline = noTimeLimit;
    16198
    162     m_wallClockDeadline = noTimeLimit;
     99        auto cpuTime = currentCPUTime();
     100        if (cpuTime < m_cpuDeadline) {
     101            auto remainingCPUTime = m_cpuDeadline - cpuTime;
     102            startTimer(locker, remainingCPUTime);
     103            return false;
     104        }
     105    }
    163106
    164     if (currentCPUTime() >= m_cpuDeadline) {
    165         // Case 1: the allowed CPU time has elapsed.
     107    // Note: we should not be holding the lock while calling the callbacks. The callbacks may
     108    // call setTimeLimit() which will try to lock as well.
    166109
    167         // If m_callback is not set, then we terminate by default.
    168         // Else, we let m_callback decide if we should terminate or not.
    169         bool needsTermination = !m_callback
    170             || m_callback(exec, m_callbackData1, m_callbackData2);
    171         if (needsTermination) {
    172             m_didFire = true;
    173             return true;
    174         }
     110    // If m_callback is not set, then we terminate by default.
     111    // Else, we let m_callback decide if we should terminate or not.
     112    bool needsTermination = !m_callback
     113        || m_callback(exec, m_callbackData1, m_callbackData2);
     114    if (needsTermination)
     115        return true;
     116
     117    {
     118        LockHolder locker(m_lock);
    175119
    176120        // If we get here, then the callback above did not want to terminate execution. As a
    … …  
    185129
    186130        ASSERT(m_hasEnteredVM);
    187         if (hasTimeLimit() && !hasStartedTimer())
    188             startTimer(m_timeLimit);
    189 
    190     } else {
    191         // Case 2: the allowed CPU time has NOT elapsed.
    192         auto remainingCPUTime = m_cpuDeadline - currentCPUTime();
    193         startTimer(remainingCPUTime);
     131        bool callbackAlreadyStartedTimer = (m_cpuDeadline != noTimeLimit);
     132        if (hasTimeLimit() && !callbackAlreadyStartedTimer)
     133            startTimer(locker, m_timeLimit);
    194134    }
    195 
    196135    return false;
    197136}
    … …  
    202141}
    203142
    204 void Watchdog::fire()
    205 {
    206     m_didFire = true;
    207 }
    208 
    209143void Watchdog::enteredVM()
    210144{
    211145    m_hasEnteredVM = true;
    212     if (hasTimeLimit())
    213         startTimer(m_timeLimit);
     146    if (hasTimeLimit()) {
     147        LockHolder locker(m_lock);
     148        startTimer(locker, m_timeLimit);
     149    }
    214150}
    215151
    … …  
    217153{
    218154    ASSERT(m_hasEnteredVM);
    219     stopTimer();
     155    LockHolder locker(m_lock);
     156    stopTimer(locker);
    220157    m_hasEnteredVM = false;
    221158}
    222159
    223 void Watchdog::startTimer(std::chrono::microseconds timeLimit)
     160void Watchdog::startTimer(LockHolder&, std::chrono::microseconds timeLimit)
    224161{
    225162    ASSERT(m_hasEnteredVM);
    226163    ASSERT(hasTimeLimit());
     164    ASSERT(timeLimit <= m_timeLimit);
    227165
    228166    m_cpuDeadline = currentCPUTime() + timeLimit;
    … …  
    231169
    232170    if ((wallClockTime < m_wallClockDeadline)
    233         && (m_wallClockDeadline <= wallClockDeadline)) {
     171        && (m_wallClockDeadline <= wallClockDeadline))
    234172        return; // Wait for the current active timer to expire before starting a new one.
    235     }
    236173
    237174    // Else, the current active timer won't fire soon enough. So, start a new timer.
    238175    this->ref(); // m_timerHandler will deref to match later.
    239176    m_wallClockDeadline = wallClockDeadline;
    240     m_timerDidFire = false;
    241 
    242     // We clear m_timerDidFire because we're starting a new timer. However, we need to make sure
    243     // that the clearing occurs before the timer thread is started. Thereafter, only didFireSlow()
    244     // should clear m_timerDidFire (unless we start yet another timer). Hence, we need a storeStore
    245     // fence here to ensure these operations do not get re-ordered.
    246     WTF::storeStoreFence();
    247177
    248178    m_timerQueue->dispatchAfter(std::chrono::nanoseconds(timeLimit), m_timerHandler);
    249179}
    250180
    251 void Watchdog::stopTimer()
     181void Watchdog::stopTimer(LockHolder&)
    252182{
    253183    m_cpuDeadline = noTimeLimit;

  • trunk/Source/JavaScriptCore/runtime/Watchdog.h

    r188329 r189009  
    2727#define Watchdog_h
    2828
    29 #include <mutex>
     29#include <wtf/Lock.h>
    3030#include <wtf/Ref.h>
    3131#include <wtf/ThreadSafeRefCounted.h>
    … …  
    4545
    4646    typedef bool (*ShouldTerminateCallback)(ExecState*, void* data1, void* data2);
    47     void setTimeLimit(VM&, std::chrono::microseconds limit, ShouldTerminateCallback = 0, void* data1 = 0, void* data2 = 0);
     47    void setTimeLimit(std::chrono::microseconds limit, ShouldTerminateCallback = 0, void* data1 = 0, void* data2 = 0);
     48    JS_EXPORT_PRIVATE void terminateSoon();
    4849
    49     // This version of didFire() will check the elapsed CPU time and call the
    50     // callback (if needed) to determine if the watchdog should fire.
    5150    bool didFire(ExecState* exec)
    5251    {
    53         if (m_didFire)
    54             return true;
    5552        if (!m_timerDidFire)
    5653            return false;
    … …  
    6259    void exitedVM();
    6360
    64     // This version of didFire() is a more efficient version for when we want
    65     // to know if the watchdog has fired in the past, and not whether it should
    66     // fire right now.
    67     bool didFire() { return m_didFire; }
    68     JS_EXPORT_PRIVATE void fire();
    69 
    7061    void* timerDidFireAddress() { return &m_timerDidFire; }
    7162
    … …  
    7364
    7465private:
    75     void startTimer(std::chrono::microseconds timeLimit);
    76     void stopTimer();
    77 
    78     inline bool hasStartedTimer();
     66    void startTimer(LockHolder&, std::chrono::microseconds timeLimit);
     67    void stopTimer(LockHolder&);
    7968
    8069    bool didFireSlow(ExecState*);
    … …  
    8675    // (probably from another thread) but is only cleared in the script thread.
    8776    bool m_timerDidFire;
    88     bool m_didFire;
    8977
    9078    std::chrono::microseconds m_timeLimit;
    … …  
    9280    std::chrono::microseconds m_cpuDeadline;
    9381    std::chrono::microseconds m_wallClockDeadline;
     82
     83    // Writes to m_timerDidFire and m_timeLimit, and Reads+Writes to m_cpuDeadline and m_wallClockDeadline
     84    // must be guarded by this lock.
     85    Lock m_lock;
    9486
    9587    bool m_hasEnteredVM { false };

  • trunk/Source/WebCore/ChangeLog

    r189005 r189009  
     12015-08-26  Mark Lam  <mark.lam@apple.com>
     2
     3        watchdog m_didFire state erroneously retained.
     4        https://bugs.webkit.org/show_bug.cgi?id=131082
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        No new tests.  The new code is covered by the JSC API tests and an existing test:
     9        fast/workers/worker-terminate-forever.html
     10
     11        * bindings/js/JSEventListener.cpp:
     12        (WebCore::JSEventListener::handleEvent):
     13        * bindings/js/WorkerScriptController.cpp:
     14        (WebCore::WorkerScriptController::WorkerScriptController):
     15        - Always create a watchdog for the Web Worker's VM.  We need this in order to support
     16          Worker.terminate().
     17        (WebCore::WorkerScriptController::evaluate):
     18        (WebCore::WorkerScriptController::scheduleExecutionTermination):
     19        (WebCore::WorkerScriptController::isTerminatingExecution):
     20        (WebCore::WorkerScriptController::forbidExecution):
     21        (WebCore::WorkerScriptController::isExecutionTerminating): Deleted.
     22        * bindings/js/WorkerScriptController.h:
     23
    1242015-08-26  Myles C. Maxfield  <mmaxfield@apple.com>
    225

  • trunk/Source/WebCore/bindings/js/JSEventListener.cpp

    r188329 r189009  
    136136
    137137        if (is<WorkerGlobalScope>(*scriptExecutionContext)) {
     138            auto scriptController = downcast<WorkerGlobalScope>(*scriptExecutionContext).script();
    138139            bool terminatorCausedException = (exec->hadException() && isTerminatedExecutionException(exec->exception()));
    139             if (terminatorCausedException || (vm.watchdog && vm.watchdog->didFire()))
    140                 downcast<WorkerGlobalScope>(*scriptExecutionContext).script()->forbidExecution();
     140            if (terminatorCausedException || scriptController->isTerminatingExecution())
     141                scriptController->forbidExecution();
    141142        }
    142143

  • trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp

    r188594 r189009  
    5757    , m_executionForbidden(false)
    5858{
     59    m_vm->ensureWatchdog();
    5960    initNormalWorldClientData(m_vm.get());
    6061}
    … …  
    122123
    123124    VM& vm = exec->vm();
    124     if ((returnedException && isTerminatedExecutionException(returnedException))
    125         || (vm.watchdog && vm.watchdog->didFire())) {
     125    if ((returnedException && isTerminatedExecutionException(returnedException)) || isTerminatingExecution()) {
    126126        forbidExecution();
    127127        return;
    … …  
    150150{
    151151    // The mutex provides a memory barrier to ensure that once
    152     // termination is scheduled, isExecutionTerminating will
     152    // termination is scheduled, isTerminatingExecution() will
    153153    // accurately reflect that state when called from another thread.
    154154    LockHolder locker(m_scheduledTerminationMutex);
    155     if (m_vm->watchdog)
    156         m_vm->watchdog->fire();
     155    m_isTerminatingExecution = true;
     156
     157    ASSERT(m_vm->watchdog);
     158    m_vm->watchdog->terminateSoon();
    157159}
    158160
    159 bool WorkerScriptController::isExecutionTerminating() const
     161bool WorkerScriptController::isTerminatingExecution() const
    160162{
    161163    // See comments in scheduleExecutionTermination regarding mutex usage.
    162164    LockHolder locker(m_scheduledTerminationMutex);
    163     if (m_vm->watchdog)
    164         return m_vm->watchdog->didFire();
    165     return false;
     165    return m_isTerminatingExecution;
    166166}
    167167

  • trunk/Source/WebCore/bindings/js/WorkerScriptController.h

    r188594 r189009  
    7272        // Can be called from any thread.
    7373        void scheduleExecutionTermination();
    74         bool isExecutionTerminating() const;
     74        bool isTerminatingExecution() const;
    7575
    7676        // Called on Worker thread when JS exits with termination exception caused by forbidExecution() request,
    … …  
    9898        JSC::Strong<JSWorkerGlobalScope> m_workerGlobalScopeWrapper;
    9999        bool m_executionForbidden;
     100        bool m_isTerminatingExecution { false };
    100101        mutable Lock m_scheduledTerminationMutex;
    101102    };
Note: See TracChangeset for help on using the changeset viewer.