Remove the concept of a cleanup task

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 40 matching lines @@
 #include "wtf/Noncopyable.h"
 #include "wtf/WeakPtr.h"
 #include "wtf/text/WTFString.h"
 
 namespace blink {
 
 namespace {
 const int64_t kShortIdleHandlerDelayMs = 1000;
 const int64_t kLongIdleHandlerDelayMs = 10*1000;
 
-class MicrotaskRunner : public WebThread::TaskObserver {
+} // namespace
+
+class WorkerMicrotaskRunner : public WebThread::TaskObserver {
 public:
-    explicit MicrotaskRunner(WorkerThread* workerThread)
+    explicit WorkerMicrotaskRunner(WorkerThread* workerThread)
         : m_workerThread(workerThread)
     {
     }
 
-    virtual void willProcessTask() override { }
+    virtual void willProcessTask() override
+    {
+        // No tasks should get executed after we have closed.
+        WorkerGlobalScope* globalScope = m_workerThread->workerGlobalScope();
+        ASSERT_UNUSED(globalScope, !globalScope || !globalScope->isClosing());
+    }
+
     virtual void didProcessTask() override
     {
         Microtask::performCheckpoint();
         if (WorkerGlobalScope* globalScope = m_workerThread->workerGlobalScope()) {
             if (WorkerScriptController* scriptController = globalScope->script())
                 scriptController->rejectedPromises()->processQueue();
+            if (globalScope->isClosing()) {
+                m_workerThread->workerReportingProxy().workerGlobalScopeClosed();
+                m_workerThread->shutdown();
+            }
         }
     }
 
 private:
     // Thread owns the microtask runner; reference remains
     // valid for the lifetime of this object.
     WorkerThread* m_workerThread;
 };
 
-} // namespace
-
 static Mutex& threadSetMutex()
 {
     AtomicallyInitializedStaticReference(Mutex, mutex, new Mutex);
     return mutex;
 }
 
 static HashSet<WorkerThread*>& workerThreads()
 {
     DEFINE_STATIC_LOCAL(HashSet<WorkerThread*>, threads, ());
     return threads;
@@ skipping 105 matching lines @@
     static PassOwnPtr<WorkerThreadTask> create(WorkerThread& workerThread, PassOwnPtr<ExecutionContextTask> task, bool isInstrumented)
     {
         return adoptPtr(new WorkerThreadTask(workerThread, task, isInstrumented));
     }
 
     virtual ~WorkerThreadTask() { }
 
     virtual void run() override
     {
         WorkerGlobalScope* workerGlobalScope = m_workerThread.workerGlobalScope();
-        // Tasks could be put on the message loop after the cleanup task,
-        // ensure none of those are ran.
-        if (!workerGlobalScope)
+        // If the thread is terminated before it had a chance initialize (see
+        // WorkerThread::Initialize()), we mustn't run any of the posted tasks.
+        if (!workerGlobalScope) {
+            ASSERT(m_workerThread.terminated());
             return;
+        }
 
         if (m_isInstrumented)
             InspectorInstrumentation::willPerformExecutionContextTask(workerGlobalScope, m_task.get());
-        if ((!workerGlobalScope->isClosing() && !m_workerThread.terminated()) || m_task->isCleanupTask())
-            m_task->performTask(workerGlobalScope);
+        m_task->performTask(workerGlobalScope);
         if (m_isInstrumented)
             InspectorInstrumentation::didPerformExecutionContextTask(workerGlobalScope);
     }
 
 private:
     WorkerThreadTask(WorkerThread& workerThread, PassOwnPtr<ExecutionContextTask> task, bool isInstrumented)
         : m_workerThread(workerThread)
         , m_task(task)
         , m_isInstrumented(isInstrumented)
     {
@@ skipping 72 matching lines @@
 
 void WorkerThread::initialize()
 {
     KURL scriptURL = m_startupData->m_scriptURL;
     String sourceCode = m_startupData->m_sourceCode;
     WorkerThreadStartMode startMode = m_startupData->m_startMode;
     OwnPtr<Vector<char>> cachedMetaData = m_startupData->m_cachedMetaData.release();
     V8CacheOptions v8CacheOptions = m_startupData->m_v8CacheOptions;
 
     {
-        MutexLocker lock(m_threadCreationMutex);
+        MutexLocker lock(m_threadStateMutex);
 
         // The worker was terminated before the thread had a chance to run.
         if (m_terminated) {
             // Notify the proxy that the WorkerGlobalScope has been disposed of.
             // This can free this thread object, hence it must not be touched afterwards.
             m_workerReportingProxy.workerThreadTerminated();
             return;
         }
 
-        m_microtaskRunner = adoptPtr(new MicrotaskRunner(this));
+        m_microtaskRunner = adoptPtr(new WorkerMicrotaskRunner(this));
         backingThread().addTaskObserver(m_microtaskRunner.get());
-        backingThread().attachGC();
+        backingThread().initialize();
 
         m_isolate = initializeIsolate();
         m_workerGlobalScope = createWorkerGlobalScope(m_startupData.release());
         m_workerGlobalScope->scriptLoaded(sourceCode.length(), cachedMetaData.get() ? cachedMetaData->size() : 0);
 
         PlatformThreadData::current().threadTimers().setSharedTimer(adoptPtr(new WorkerSharedTimer(this)));
     }
 
     // The corresponding call to stopRunLoop() is in ~WorkerScriptController().
     didStartRunLoop();
@@ skipping 10 matching lines @@
     OwnPtr<CachedMetadataHandler> handler(workerGlobalScope()->createWorkerScriptCachedMetadataHandler(scriptURL, cachedMetaData.get()));
     bool success = script->evaluate(ScriptSourceCode(sourceCode, scriptURL), nullptr, handler.get(), v8CacheOptions);
     m_workerGlobalScope->didEvaluateWorkerScript();
     m_workerReportingProxy.didEvaluateWorkerScript(success);
 
     postInitialize();
 
     postDelayedTask(FROM_HERE, createSameThreadTask(&WorkerThread::idleHandler, this), kShortIdleHandlerDelayMs);
 }
 
-void WorkerThread::cleanup()
+void WorkerThread::shutdown()
 {
+    MutexLocker lock(m_threadStateMutex);
+    ASSERT(isCurrentThread());
+
+    PlatformThreadData::current().threadTimers().setSharedTimer(nullptr);
+    workerGlobalScope()->dispose();
+    willDestroyIsolate();
+
     // This should be called before we start the shutdown procedure.
     workerReportingProxy().willDestroyWorkerGlobalScope();
 
     // 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.
 #if !ENABLE(OILPAN)
     ASSERT(m_workerGlobalScope->hasOneRef());
 #endif
     m_workerGlobalScope->notifyContextDestroyed();
     m_workerGlobalScope = nullptr;
 
-    backingThread().detachGC();
+    backingThread().removeTaskObserver(m_microtaskRunner.get());
+    backingThread().shutdown();
     destroyIsolate();
 
-    backingThread().removeTaskObserver(m_microtaskRunner.get());
     m_microtaskRunner = nullptr;
 
     // Notify the proxy that the WorkerGlobalScope has been disposed of.
     // This can free this thread object, hence it must not be touched afterwards.
     workerReportingProxy().workerThreadTerminated();
 
     m_terminationEvent->signal();
 
     // Clean up PlatformThreadData before WTF::WTFThreadData goes away!
     PlatformThreadData::current().destroy();
 }
 
-class WorkerThreadShutdownFinishTask : public ExecutionContextTask {
-public:
-    static PassOwnPtr<WorkerThreadShutdownFinishTask> create()
-    {
-        return adoptPtr(new WorkerThreadShutdownFinishTask());
-    }
-
-    virtual void performTask(ExecutionContext *context)
-    {
-        WorkerGlobalScope* workerGlobalScope = toWorkerGlobalScope(context);
-        workerGlobalScope->dispose();
-
-        WorkerThread* workerThread = workerGlobalScope->thread();
-        workerThread->willDestroyIsolate();
-        workerThread->backingThread().postTask(FROM_HERE, new Task(WTF::bind(&WorkerThread::cleanup, workerThread)));
-    }
-
-    virtual bool isCleanupTask() const { return true; }
-};
-
-class WorkerThreadShutdownStartTask : public ExecutionContextTask {
-public:
-    static PassOwnPtr<WorkerThreadShutdownStartTask> create()
-    {
-        return adoptPtr(new WorkerThreadShutdownStartTask());
-    }
-
-    virtual void performTask(ExecutionContext *context)
-    {
-        WorkerGlobalScope* workerGlobalScope = toWorkerGlobalScope(context);
-        workerGlobalScope->stopActiveDOMObjects();
-        PlatformThreadData::current().threadTimers().setSharedTimer(nullptr);
-
-        // Event listeners would keep DOMWrapperWorld objects alive for too long. Also, they have references to JS objects,
-        // which become dangling once Heap is destroyed.
-        workerGlobalScope->removeAllEventListeners();
-
-        // Stick a shutdown command at the end of the queue, so that we deal
-        // with all the cleanup tasks the databases post first.
-        workerGlobalScope->postTask(FROM_HERE, WorkerThreadShutdownFinishTask::create());
-    }
-
-    virtual bool isCleanupTask() const { return true; }
-};
 
 void WorkerThread::stop()
 {
     // Prevent the deadlock between GC and an attempt to stop a thread.
     SafePointScope safePointScope(ThreadState::HeapPointersOnStack);
     stopInternal();
 }
 
 void WorkerThread::stopInShutdownSequence()
 {
     stopInternal();
 }
 
 void WorkerThread::terminateAndWait()
 {
     stop();
     m_terminationEvent->wait();
 }
 
 bool WorkerThread::terminated()
 {
-    MutexLocker lock(m_threadCreationMutex);
+    MutexLocker lock(m_threadStateMutex);
     return m_terminated;
 }
 
 void WorkerThread::stopInternal()
 {
-    // Protect against this method and initialize() racing each other.
-    MutexLocker lock(m_threadCreationMutex);
+    // Protect against this method, initialize() or termination via the global scope racing each other.
+    MutexLocker lock(m_threadStateMutex);
 
     // If stop 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_shutdownEvent)
         m_shutdownEvent->signal();
 
     if (!m_workerGlobalScope)

[kinuko, 2015/05/14 11:27:40]

Um... it's not really related to this patch, but if terminateAndWait() is called
immediately after start() (and before initialize runs) we don't post shutdown
task, therefore we won't signal m_terminationEvent and terminateAndWait() never
ends.  It seems problematic?  Should we signal m_terminationEvent here?

[Sami, 2015/05/14 11:32:17]

Yes, I believe that would lead to a deadlock. Let me put together another patch
to add the signaling here.

         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.
     terminateV8Execution();
 
     InspectorInstrumentation::didKillAllExecutionContextTasks(m_workerGlobalScope.get());
     m_debuggerMessageQueue.kill();
-    postTask(FROM_HERE, WorkerThreadShutdownStartTask::create());
+    backingThread().postTask(FROM_HERE, new Task(threadSafeBind(&WorkerThread::shutdown, AllowCrossThreadAccess(this))));
 }
 
 void WorkerThread::didStartRunLoop()
 {
     ASSERT(isCurrentThread());
     Platform::current()->didStartWorkerRunLoop();
 }
 
 void WorkerThread::didStopRunLoop()
 {
@@ skipping 119 matching lines @@
     InspectorInstrumentation::didLeaveNestedRunLoop(m_workerGlobalScope.get());
 }
 
 void WorkerThread::setWorkerInspectorController(WorkerInspectorController* workerInspectorController)
 {
     MutexLocker locker(m_workerInspectorControllerMutex);
     m_workerInspectorController = workerInspectorController;
 }
 
 } // namespace blink