Oilpan: Prepare moving InspectorController and InspectorAgents to oilpan.

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 12 matching lines @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  */
 
 #include "config.h"
 
 #include "core/workers/WorkerThread.h"
 
 #include "bindings/v8/ScriptSourceCode.h"
 #include "core/inspector/InspectorInstrumentation.h"
+#include "core/inspector/WorkerDebuggerAgent.h"
+#include "core/inspector/WorkerInspectorController.h"
 #include "core/workers/DedicatedWorkerGlobalScope.h"
 #include "core/workers/WorkerClients.h"
 #include "core/workers/WorkerReportingProxy.h"
 #include "core/workers/WorkerThreadStartupData.h"
 #include "platform/PlatformThreadData.h"
 #include "platform/heap/ThreadState.h"
 #include "platform/weborigin/KURL.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebWaitableEvent.h"
 #include "public/platform/WebWorkerRunLoop.h"
@@ skipping 52 matching lines @@
     m_threadID = createThread(WorkerThread::workerThreadStart, this, "WebCore: Worker");
 
     return m_threadID;
 }
 
 void WorkerThread::workerThreadStart(void* thread)
 {
     static_cast<WorkerThread*>(thread)->workerThread();
 }
 
+void WorkerThread::interruptAndDispatchInspectorCommands()
+{
+    MutexLocker locker(m_threadShutdownMutex);
+    if (!m_workerGlobalScope)
+        return;
+    if (WorkerInspectorController* inspectorController = m_workerGlobalScope->workerInspectorController())

[yurys, 2014/06/26 11:04:30]

WorkerGlobalScope::clearInspector may be executing on the worker thread in
parallel and you may end up with non-null inspectorController instance here
which will be destroyed immediately after that on the worker thread resulting in
use after free.

Having said that, I don't think this may happen with dedicated workers as
clearInspector is called in the following two places:
1) From WorkerGlobalScope::dispose, which is called with the mutex locked
2) From WorkerThreadShutdownFinishTask::performTask where you don't lock the
mutex but the task is always a result of WorkerThread::stop, which in turn is
only invoked from WorkerMessagingProxy when m_askedToTerminate is true. If
m_askedToTerminate is true we will bail out from
WorkerMessagingProxy::sendMessageToInspector before calling
interruptAndDispatchInspectorCommands. The latter is not true though when
interruptAndDispatchInspectorCommands is invoked from
WebEmbeddedWorkerImpl::dispatchDevToolsMessage without checking
m_askedToTerminate. 

All of that seems very fragile so I believe we need to guard
WorkerInspectorController destruction inside WorkerGlobalObject::clearInspector
with the same mutex as we do when disposing m_workerGlobalScope.


+horo who may have additional input regarding WebEmbeddedWorkerImpl

[horo, 2014/06/27 06:36:05]

I agree with yurys.

But it may be good to add "if (m_askedToTerminate) return;" in
WebEmbeddedWorkerImpl::dispatchDevToolsMessage and
WebSharedWorkerImpl::dispatchDevToolsMessage.

+        inspectorController->interruptAndDispatchInspectorCommands();
+}
+
 void WorkerThread::workerThread()
 {
     KURL scriptURL = m_startupData->m_scriptURL;
     String sourceCode = m_startupData->m_sourceCode;
     WorkerThreadStartMode startMode = m_startupData->m_startMode;
-
     {
         MutexLocker lock(m_threadCreationMutex);
         ThreadState::attach();
         m_workerGlobalScope = createWorkerGlobalScope(m_startupData.release());
         m_runLoop.setWorkerGlobalScope(workerGlobalScope());
 
         if (m_runLoop.terminated()) {
             // The worker was terminated before the thread had a chance to run. Since the context didn't exist yet,
             // forbidExecution() couldn't be called from stop().
             m_workerGlobalScope->script()->forbidExecution();
@@ skipping 12 matching lines @@
     InspectorInstrumentation::willEvaluateWorkerScript(workerGlobalScope(), startMode);
     script->evaluate(ScriptSourceCode(sourceCode, scriptURL));
 
     runEventLoop();
 
     // This should be called before we start the shutdown procedure.
     workerReportingProxy().willDestroyWorkerGlobalScope();
 
     ThreadIdentifier threadID = m_threadID;
 
-    // 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.
+    {
+        MutexLocker locker(m_threadShutdownMutex);
+        // 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());
+        ASSERT(m_workerGlobalScope->hasOneRef());
 #endif
-    m_workerGlobalScope->dispose();
-    m_workerGlobalScope = nullptr;
+        m_workerGlobalScope->dispose();
+        m_workerGlobalScope = nullptr;
+    }
 
     // Detach the ThreadState, cleaning out the thread's heap by
     // performing a final GC. The cleanup operation will at the end
     // assert that the heap is empty. If the heap does not become
     // empty, there are still pointers into the heap and those
     // pointers will be dangling after thread termination because we
     // are destroying the heap. It is important to detach while the
     // thread is still valid. In particular, finalizers for objects in
     // the heap for this thread will need to access thread local data.
     ThreadState::detach();
@@ skipping 79 matching lines @@
     }
     m_runLoop.terminate();
 }
 
 bool WorkerThread::isCurrentThread() const
 {
     return m_threadID == currentThread();
 }
 
 } // namespace WebCore