Worker: Initiate worker thread shutdown always on the main thread

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 54 matching lines @@
         DCHECK(!m_workerThread->workerGlobalScope() || !m_workerThread->workerGlobalScope()->isClosing());
     }
 
     void didProcessTask() override
     {
         Microtask::performCheckpoint(m_workerThread->isolate());
         if (WorkerGlobalScope* globalScope = m_workerThread->workerGlobalScope()) {
             if (WorkerOrWorkletScriptController* scriptController = globalScope->scriptController())
                 scriptController->getRejectedPromises()->processQueue();
             if (globalScope->isClosing()) {
+                // |m_workerThread| will eventually be requested to terminate.
                 m_workerThread->workerReportingProxy().workerGlobalScopeClosed();
-                m_workerThread->terminateFromWorkerThread();
+
+                // Dispose WorkerGlobalScope to avoid processing the next task.
+                m_workerThread->prepareForShutdown();
             }
         }
     }
 
 private:
     // Thread owns the microtask runner; reference remains
     // valid for the lifetime of this object.
     WorkerThread* m_workerThread;
 };
 
@@ skipping 35 matching lines @@
     if (isInstrumented)
         isInstrumented = !task->taskNameForInstrumentation().isEmpty();
     if (isInstrumented) {
         DCHECK(isCurrentThread());
         InspectorInstrumentation::asyncTaskScheduled(workerGlobalScope(), "Worker task", task.get());
     }
     return threadSafeBind(&WorkerThread::performTask, AllowCrossThreadAccess(this), passed(std::move(task)), isInstrumented);
 }
 
 WorkerThread::WorkerThread(PassRefPtr<WorkerLoaderProxy> workerLoaderProxy, WorkerReportingProxy& workerReportingProxy)
-    : m_started(false)
-    , m_terminated(false)
-    , m_shutdown(false)
-    , m_pausedInDebugger(false)
-    , m_runningDebuggerTask(false)
-    , m_shouldTerminateV8Execution(false)
-    , m_inspectorTaskRunner(adoptPtr(new InspectorTaskRunner()))
+    : m_inspectorTaskRunner(adoptPtr(new InspectorTaskRunner()))
     , m_workerLoaderProxy(workerLoaderProxy)
     , m_workerReportingProxy(workerReportingProxy)
-    , m_webScheduler(nullptr)
     , 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());
     workerThreads().add(this);
 }
@@ skipping 81 matching lines @@
     bool success = m_workerGlobalScope->scriptController()->evaluate(ScriptSourceCode(sourceCode, scriptURL), nullptr, handler, v8CacheOptions);
     m_workerGlobalScope->didEvaluateWorkerScript();
     m_workerReportingProxy.didEvaluateWorkerScript(success);
 
     postInitialize();
 }
 
 void WorkerThread::shutdown()
 {
     DCHECK(isCurrentThread());
-    {
-        MutexLocker lock(m_threadStateMutex);
-        if (m_shutdown)
-            return;
-        m_shutdown = true;
-    }
-
-    // This should be called before we start the shutdown procedure.
-    workerReportingProxy().willDestroyWorkerGlobalScope();
-
-    workerGlobalScope()->dispose();
-
-    workerBackingThread().backingThread().removeTaskObserver(m_microtaskRunner.get());
+    prepareForShutdown();
     postTask(BLINK_FROM_HERE, createSameThreadTask(&WorkerThread::performShutdownTask, this));
 }
 
 void WorkerThread::performShutdownTask()
 {
     DCHECK(isCurrentThread());

[yhirano, 2016/05/23 10:05:19]

It might be good to assert m_terminated && m_readyToShutdown while you need a
#if DCHECK_IS_ON block for a mutex.

[nhiroki, 2016/05/24 06:50:43]

Done.

 
     // The below assignment will destroy the context, which will in turn notify
     // messaging proxy. We cannot let any objects survive past thread exit,
     // because no other thread will run GC or otherwise destroy them. If Oilpan
     // is enabled, we detach of the context/global scope, with the final heap
     // cleanup below sweeping it out.
     m_workerGlobalScope->notifyContextDestroyed();
     m_workerGlobalScope = nullptr;
 
     workerBackingThread().detach();
@@ skipping 31 matching lines @@
     // Keep this lock to prevent WorkerThread instances from being destroyed.
     MutexLocker lock(threadSetMutex());
     HashSet<WorkerThread*> threads = workerThreads();
     for (WorkerThread* thread : threads)
         thread->terminateInternal();
 
     for (WorkerThread* thread : threads)
         thread->m_shutdownEvent->wait();
 }
 
-void WorkerThread::terminateFromWorkerThread()
+void WorkerThread::prepareForShutdown()
 {
     DCHECK(isCurrentThread());
-    shutdown();
+    {
+        MutexLocker lock(m_threadStateMutex);
+        if (m_readyToShutdown)
+            return;
+        m_readyToShutdown = true;
+    }
+
+    workerReportingProxy().willDestroyWorkerGlobalScope();
+    workerGlobalScope()->dispose();
+    workerBackingThread().backingThread().removeTaskObserver(m_microtaskRunner.get());
 }
 
 WorkerGlobalScope* WorkerThread::workerGlobalScope()
 {
     DCHECK(isCurrentThread());
     return m_workerGlobalScope.get();
 }
 
 bool WorkerThread::terminated()
 {
@@ skipping 11 matching lines @@
 
     // If terminateInternal 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 thread has already initiated shutdown, just return.
-    if (m_shutdown)
-        return;
-
     // 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 initialize().
     if (!m_workerGlobalScope)
         return;
 
-    // Ensure that tasks are being handled by thread event loop. If script
-    // execution weren't forbidden, a while(1) loop in JS could keep the thread
-    // alive forever.
-    m_workerGlobalScope->scriptController()->willScheduleExecutionTermination();
+    // If |m_readyToShutdown| is set, scriptController() is already disposed.
+    if (!m_readyToShutdown) {
+        DCHECK(m_workerGlobalScope->scriptController());
+
+        // Ensure that tasks are being handled by thread event loop. If script
+        // execution weren't forbidden, a while(1) loop in JS could keep the thread
+        // alive forever.
+        m_workerGlobalScope->scriptController()->willScheduleExecutionTermination();
+    }
 
     if (workerBackingThread().workerScriptCount() == 1) {
         // 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|.
         // TODO(yhirano): TerminateExecution should be called more carefully.
         // https://crbug.com/413518
         if (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 postpone termination after task has finished.
             // Note: m_runningDebuggerTask and m_shouldTerminateV8Execution
             // access must be guarded by the lock.
             m_shouldTerminateV8Execution = true;
         } else {
             isolate()->TerminateExecution();
         }
     }
 
     InspectorInstrumentation::allAsyncTasksCanceled(m_workerGlobalScope.get());
     m_inspectorTaskRunner->kill();
     workerBackingThread().backingThread().postTask(BLINK_FROM_HERE, threadSafeBind(&WorkerThread::shutdown, AllowCrossThreadAccess(this)));

[yhirano, 2016/05/23 10:05:19]

Can you post two tasks (prepareForShutdown and performShutdownTask) here and
remove shutdown method? It's a bit confusing to me that shutdown can be called
even when prepareForShutdown is called.

[nhiroki, 2016/05/24 06:50:43]

Done.

 }
 
 v8::Isolate* WorkerThread::isolate()
 {
     return workerBackingThread().isolate();
 }
 
 bool WorkerThread::isCurrentThread()
 {
     return m_started && workerBackingThread().backingThread().isCurrentThread();
 }
 
 void WorkerThread::postTask(const WebTraceLocation& location, std::unique_ptr<ExecutionContextTask> task)
 {
     workerBackingThread().backingThread().postTask(location, createWorkerThreadTask(std::move(task), true));
 }
 
 void WorkerThread::runDebuggerTaskDontWait()
 {
     DCHECK(isCurrentThread());
     std::unique_ptr<CrossThreadClosure> task = m_inspectorTaskRunner->takeNextTask(InspectorTaskRunner::DontWaitForTask);
     if (task)
         (*task)();
 }
 
 void WorkerThread::appendDebuggerTask(std::unique_ptr<CrossThreadClosure> task)
 {
     {
         MutexLocker lock(m_threadStateMutex);
-        if (m_shutdown)
+        if (m_readyToShutdown)
             return;
     }
     m_inspectorTaskRunner->appendTask(threadSafeBind(&WorkerThread::runDebuggerTask, AllowCrossThreadAccess(this), passed(std::move(task))));
     {
         MutexLocker lock(m_threadStateMutex);
         if (isolate())
             m_inspectorTaskRunner->interruptAndRunAllTasksDontWait(isolate());
     }
     workerBackingThread().backingThread().postTask(BLINK_FROM_HERE, threadSafeBind(&WorkerThread::runDebuggerTaskDontWait, AllowCrossThreadAccess(this)));
 }
@@ skipping 35 matching lines @@
     } while (task && m_pausedInDebugger);
     ThreadDebugger::idleFinished(isolate());
 }
 
 void WorkerThread::stopRunningDebuggerTasksOnPause()
 {
     m_pausedInDebugger = false;
 }
 
 } // namespace blink