Worker: Attempt to gracefully terminate WorkerThread as much as possible

third_party/WebKit/Source/core/workers/WorkerThread.cpp

 /*
  * Copyright (C) 2008 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
@@ skipping 24 matching lines @@
 #include "core/inspector/WorkerThreadDebugger.h"
 #include "core/workers/WorkerBackingThread.h"
 #include "core/workers/WorkerClients.h"
 #include "core/workers/WorkerReportingProxy.h"
 #include "core/workers/WorkerThreadStartupData.h"
 #include "platform/ThreadSafeFunctional.h"
 #include "platform/WaitableEvent.h"
 #include "platform/WebThreadSupportingGC.h"
 #include "platform/heap/SafePoint.h"
 #include "platform/heap/ThreadState.h"
+#include "platform/scheduler/CancellableTaskFactory.h"
 #include "platform/weborigin/KURL.h"
 #include "public/platform/WebThread.h"
 #include "wtf/Functional.h"
 #include "wtf/Noncopyable.h"
 #include "wtf/text/WTFString.h"
 #include <limits.h>
 
 namespace blink {
 
-class WorkerThread::WorkerMicrotaskRunner : public WebThread::TaskObserver {
+// TODO(nhiroki): Adjust the delay based on UMA.
+const long long kForceTerminationDelayInMs = 3000; // 3 secs

[kinuko, 2016/05/31 10:13:26]

nit: we used to have 2 secs timer to forcibly kill worker process, we might be
able to start with that

[nhiroki, 2016/06/01 04:16:36]

Done.

+
+// ForceTerminationTask is used for posting a delayed task to terminate an
+// isolate from the main thread. This task is expected to run when the shutdown
+// sequence does not start in a certain time period because of an inifite loop
+// in the JS execution context etc. When the shutdown sequence is started before
+// this task runs, the task is simply cancelled.
+class WorkerThread::ForceTerminationTask final {
+public:
+    static PassOwnPtr<ForceTerminationTask> create(WorkerThread* workerThread)
+    {
+        return adoptPtr(new ForceTerminationTask(workerThread));
+    }
+
+    void schedule()
+    {
+        DCHECK(isMainThread());
+        Platform::current()->mainThread()->getWebTaskRunner()->postDelayedTask(BLINK_FROM_HERE, m_cancellableTaskFactory->cancelAndCreate(), m_workerThread->m_forceTerminationDelayInMs);
+    }
+
+private:
+    explicit ForceTerminationTask(WorkerThread* workerThread)
+        : m_workerThread(workerThread)
+    {
+        DCHECK(isMainThread());
+        m_cancellableTaskFactory = CancellableTaskFactory::create(this, &ForceTerminationTask::run);
+    }
+
+    void run()
+    {
+        DCHECK(isMainThread());
+        MutexLocker lock(m_workerThread->m_threadStateMutex);
+        if (m_workerThread->m_readyToShutdown) {
+            // Shutdown sequence is now running. Just return.
+            return;
+        }
+
+        m_workerThread->forciblyTerminateIsolate();
+        DCHECK_EQ(WorkerThread::ExitCode::NotTerminated, m_workerThread->m_exitCode);
+        m_workerThread->m_exitCode = WorkerThread::ExitCode::AsyncForciblyTerminated;
+    }
+
+    WorkerThread* m_workerThread;
+    OwnPtr<CancellableTaskFactory> m_cancellableTaskFactory;
+};
+
+class WorkerThread::WorkerMicrotaskRunner final : public WebThread::TaskObserver {
 public:
     explicit WorkerMicrotaskRunner(WorkerThread* workerThread)
         : m_workerThread(workerThread)
     {
     }
 
     void willProcessTask() override
     {
         // No tasks should get executed after we have closed.
         DCHECK(!m_workerThread->workerGlobalScope() || !m_workerThread->workerGlobalScope()->isClosing());
@@ skipping 28 matching lines @@
 }
 
 static HashSet<WorkerThread*>& workerThreads()
 {
     DEFINE_STATIC_LOCAL(HashSet<WorkerThread*>, threads, ());
     return threads;
 }
 
 WorkerThread::~WorkerThread()
 {
+    DCHECK(isMainThread());
     MutexLocker lock(threadSetMutex());
     DCHECK(workerThreads().contains(this));
     workerThreads().remove(this);
+
+    // TODO(nhiroki): Record how this thread is terminated (i.e. m_exitCode)
+    // in UMA.
+    DCHECK_NE(ExitCode::NotTerminated, m_exitCode);
 }
 
 void WorkerThread::start(PassOwnPtr<WorkerThreadStartupData> startupData)
 {
     DCHECK(isMainThread());
 
     if (m_started)
         return;
 
     m_started = true;
     workerBackingThread().backingThread().postTask(BLINK_FROM_HERE, threadSafeBind(&WorkerThread::initializeOnWorkerThread, AllowCrossThreadAccess(this), passed(std::move(startupData))));
 }
 
 void WorkerThread::terminate()
 {
     DCHECK(isMainThread());
-
-    // Prevent the deadlock between GC and an attempt to terminate a thread.
-    SafePointScope safePointScope(BlinkGC::HeapPointersOnStack);
-
-    // Protect against this method, initializeOnWorkerThread() or termination
-    // via the global scope racing each other.
-    MutexLocker lock(m_threadStateMutex);
-
-    // If terminate has already been called, just return.
-    if (m_terminated)
-        return;
-    m_terminated = true;
-
-    // Signal the thread to notify that the thread's stopping.
-    if (m_terminationEvent)
-        m_terminationEvent->signal();
-
-    // If the worker thread was never initialized, don't start another
-    // shutdown, but still wait for the thread to signal when shutdown has
-    // completed on initializeOnWorkerThread().
-    if (!m_workerGlobalScope)
-        return;
-
-    // Determine if we should terminate the isolate so that the task can
-    // be handled by thread event loop. If script execution weren't forbidden,
-    // a while(1) loop in JS could keep the thread alive forever.
-    //
-    // (1) |m_readyToShutdown|: It this is set, the worker thread has already
-    // noticed that the thread is about to be terminated and the worker global
-    // scope is already disposed, so we don't have to explicitly terminate the
-    // isolate.
-    //
-    // (2) |workerScriptCount() == 1|: If other WorkerGlobalScopes are running
-    // on the worker thread, we should not terminate the isolate. This condition
-    // is not entirely correct because other scripts can be being initialized or
-    // terminated simuletaneously. Though this function itself is protected by a
-    // mutex, it is possible that |workerScriptCount()| here is not consistent
-    // with that in |initialize| and |shutdown|.
-    //
-    // (3) |m_runningDebuggerTask|: Terminating during debugger task may lead to
-    // crash due to heavy use of v8 api in debugger. Any debugger task is
-    // guaranteed to finish, so we can wait for the completion.
-    bool shouldTerminateIsolate = !m_readyToShutdown && (workerBackingThread().workerScriptCount() == 1) && !m_runningDebuggerTask;
-
-    if (shouldTerminateIsolate) {
-        // TODO(yhirano): TerminateExecution should be called more
-        // carefully (https://crbug.com/413518).
-        m_workerGlobalScope->scriptController()->willScheduleExecutionTermination();
-        isolate()->TerminateExecution();
-    }
-
-    m_inspectorTaskRunner->kill();
-    workerBackingThread().backingThread().postTask(BLINK_FROM_HERE, threadSafeBind(&WorkerThread::prepareForShutdownOnWorkerThread, AllowCrossThreadAccess(this)));
-    workerBackingThread().backingThread().postTask(BLINK_FROM_HERE, threadSafeBind(&WorkerThread::performShutdownOnWorkerThread, AllowCrossThreadAccess(this)));
+    terminateInternal(TerminationMode::Graceful);
 }
 
 void WorkerThread::terminateAndWait()
 {
     DCHECK(isMainThread());
-    terminate();
+
+    // The main thread will be blocked, so asynchronous graceful shutdown does
+    // not work.
+    terminateInternal(TerminationMode::Forcible);
+
+    // Terminating the worker thread that hasn't started yet does not run the
+    // shutdown sequence on the worker thread, so should not wait for the
+    // shutdown event.
+    if (m_exitCode == ExitCode::TerminatedBeforeStarting)
+        return;
     m_shutdownEvent->wait();
 }
 
 void WorkerThread::terminateAndWaitForAllWorkers()
 {
     DCHECK(isMainThread());
 
     // Keep this lock to prevent WorkerThread instances from being destroyed.
     MutexLocker lock(threadSetMutex());
     HashSet<WorkerThread*> threads = workerThreads();
+
+    // The main thread will be blocked, so asynchronous graceful shutdown does
+    // not work.
     for (WorkerThread* thread : threads)
-        thread->terminate();
+        thread->terminateInternal(TerminationMode::Forcible);
 
-    for (WorkerThread* thread : threads)
+    for (WorkerThread* thread : threads) {
+        // Terminating the worker thread that hasn't started yet does not run
+        // the shutdown sequence on the worker thread, so should not wait for
+        // the shutdown event.
+        if (thread->m_exitCode == ExitCode::TerminatedBeforeStarting)
+            continue;
         thread->m_shutdownEvent->wait();
+    }
 }
 
 v8::Isolate* WorkerThread::isolate()
 {
     return workerBackingThread().isolate();
 }
 
 bool WorkerThread::isCurrentThread()
 {
     return m_started && workerBackingThread().backingThread().isCurrentThread();
@@ skipping 61 matching lines @@
 }
 
 PlatformThreadId WorkerThread::platformThreadId()
 {
     if (!m_started)
         return 0;
     return workerBackingThread().backingThread().platformThread().threadId();
 }
 
 WorkerThread::WorkerThread(PassRefPtr<WorkerLoaderProxy> workerLoaderProxy, WorkerReportingProxy& workerReportingProxy)
-    : m_inspectorTaskRunner(adoptPtr(new InspectorTaskRunner()))
+    : m_forceTerminationDelayInMs(kForceTerminationDelayInMs)
+    , m_inspectorTaskRunner(adoptPtr(new InspectorTaskRunner()))
     , m_workerLoaderProxy(workerLoaderProxy)
     , m_workerReportingProxy(workerReportingProxy)
     , m_terminationEvent(adoptPtr(new WaitableEvent(
         WaitableEvent::ResetPolicy::Manual,
         WaitableEvent::InitialState::NonSignaled)))
     , m_shutdownEvent(adoptPtr(new WaitableEvent(
         WaitableEvent::ResetPolicy::Manual,
         WaitableEvent::InitialState::NonSignaled)))
 {
     MutexLocker lock(threadSetMutex());
@@ skipping 65 matching lines @@
     }
 
     CachedMetadataHandler* handler = workerGlobalScope()->createWorkerScriptCachedMetadataHandler(scriptURL, cachedMetaData.get());
     bool success = m_workerGlobalScope->scriptController()->evaluate(ScriptSourceCode(sourceCode, scriptURL), nullptr, handler, v8CacheOptions);
     m_workerGlobalScope->didEvaluateWorkerScript();
     m_workerReportingProxy.didEvaluateWorkerScript(success);
 
     postInitialize();
 }
 
+void WorkerThread::terminateInternal(TerminationMode mode)
+{
+    // Prevent the deadlock between GC and an attempt to terminate a thread.
+    SafePointScope safePointScope(BlinkGC::HeapPointersOnStack);
+
+    // Protect against this method, initializeOnWorkerThread() or termination
+    // via the global scope racing each other.
+    MutexLocker lock(m_threadStateMutex);
+
+    // If terminate has already been called.
+    if (m_terminated) {
+        // The synchronous forcible termination request should overtake the
+        // scheduled termination task because the request will block the main
+        // thread and the scheduled termination task never runs.
+        if (mode == TerminationMode::Forcible && m_exitCode == ExitCode::NotTerminated) {
+            DCHECK(m_scheduledForceTerminationTask);
+            m_scheduledForceTerminationTask.reset();
+            forciblyTerminateIsolate();
+            DCHECK_EQ(ExitCode::NotTerminated, m_exitCode);
+            m_exitCode = ExitCode::SyncForciblyTerminated;
+        }
+        return;
+    }
+    m_terminated = true;
+
+    // Signal the thread to notify that the thread's stopping.
+    if (m_terminationEvent)
+        m_terminationEvent->signal();
+
+    // If the worker thread was never initialized, don't start another
+    // shutdown, but still wait for the thread to signal when shutdown has
+    // completed on initializeOnWorkerThread().

[kinuko, 2016/05/31 10:13:26]

Hmm, if we're not calling start() initializeOnWorkerThread isn't scheduled so it
looks it's fine not to wait, but if start() is called but
initializeOnWorkerThread has not yet run it'll eventually run and try to signal
m_shutdownEvent->signal?

[kinuko, 2016/05/31 10:13:26]

The comment looks stale now? (We're no longer waiting in this case)

[nhiroki, 2016/06/01 04:16:36]

Good point. I'm now thinking that the main thread should wait for the shutdown
signal (WorkerReportingProxy::workerThreadTerminated call) even if WorkerThread
hasn't actually started yet so that each worker class can have a chance to
delete a WorkerThread instance via workerThreadTerminated() call (e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...)

The patchset6+ ensures that workerThreadTerminate() is called on any termination
paths. WDYT?

+    if (!m_workerGlobalScope) {
+        DCHECK_EQ(ExitCode::NotTerminated, m_exitCode);
+        m_exitCode = ExitCode::TerminatedBeforeStarting;
+        return;
+    }
+
+    // Determine if we should synchronously terminate or schedule to terminate
+    // the isolate so that the task can be handled by thread event loop. If
+    // script execution weren't forbidden, a while(1) loop in JS could keep
+    // the thread alive forever.
+    //
+    // (1) |m_readyToShutdown|: It this is set, the worker thread has already
+    // noticed that the thread is about to be terminated and the worker global
+    // scope is already disposed, so we don't have to explicitly terminate the
+    // isolate.
+    //
+    // (2) |workerScriptCount() == 1|: If other WorkerGlobalScopes are running
+    // on the worker thread, we should not terminate the isolate. This condition
+    // is not entirely correct because other scripts can be being initialized or
+    // terminated simuletaneously. Though this function itself is protected by a
+    // mutex, it is possible that |workerScriptCount()| here is not consistent
+    // with that in |initialize| and |shutdown|.
+    //
+    // (3) |m_runningDebuggerTask|: Terminating during debugger task may lead to
+    // crash due to heavy use of v8 api in debugger. Any debugger task is
+    // guaranteed to finish, so we can wait for the completion.
+    bool shouldScheduleToTerminateIsolate = !m_readyToShutdown && (workerBackingThread().workerScriptCount() == 1) && !m_runningDebuggerTask;
+
+    if (shouldScheduleToTerminateIsolate) {
+        if (mode == TerminationMode::Forcible) {
+            forciblyTerminateIsolate();
+            DCHECK_EQ(ExitCode::NotTerminated, m_exitCode);
+            m_exitCode = ExitCode::SyncForciblyTerminated;
+        } else {
+            DCHECK_EQ(TerminationMode::Graceful, mode);

[kinuko, 2016/05/31 10:13:26]

nit: DCHECK(!m_scheduledForceTreminationTask) ?

[nhiroki, 2016/06/01 04:16:36]

Done.

+            m_scheduledForceTerminationTask = ForceTerminationTask::create(this);
+            m_scheduledForceTerminationTask->schedule();
+        }
+    }
+
+    m_inspectorTaskRunner->kill();
+    workerBackingThread().backingThread().postTask(BLINK_FROM_HERE, threadSafeBind(&WorkerThread::prepareForShutdownOnWorkerThread, AllowCrossThreadAccess(this)));
+    workerBackingThread().backingThread().postTask(BLINK_FROM_HERE, threadSafeBind(&WorkerThread::performShutdownOnWorkerThread, AllowCrossThreadAccess(this)));
+}
+
+void WorkerThread::forciblyTerminateIsolate()
+{
+    DCHECK(m_workerGlobalScope);
+    m_workerGlobalScope->scriptController()->willScheduleExecutionTermination();
+    isolate()->TerminateExecution();
+}
+
 void WorkerThread::prepareForShutdownOnWorkerThread()
 {
     DCHECK(isCurrentThread());
     {
         MutexLocker lock(m_threadStateMutex);
         if (m_readyToShutdown)
             return;
         m_readyToShutdown = true;
+        if (m_exitCode == ExitCode::NotTerminated)
+            m_exitCode = ExitCode::GracefullyTerminated;
     }
 
     workerReportingProxy().willDestroyWorkerGlobalScope();
     InspectorInstrumentation::allAsyncTasksCanceled(workerGlobalScope());
     workerGlobalScope()->dispose();
     workerBackingThread().backingThread().removeTaskObserver(m_microtaskRunner.get());
 }
 
 void WorkerThread::performShutdownOnWorkerThread()
 {
@@ skipping 71 matching lines @@
 }
 
 void WorkerThread::runDebuggerTaskDontWaitOnWorkerThread()
 {
     DCHECK(isCurrentThread());
     std::unique_ptr<CrossThreadClosure> task = m_inspectorTaskRunner->takeNextTask(InspectorTaskRunner::DontWaitForTask);
     if (task)
         (*task)();
 }
 
+WorkerThread::ExitCode WorkerThread::exitCode()
+{
+    MutexLocker lock(m_threadStateMutex);
+    return m_exitCode;
+}
+
 } // namespace blink