2009-04-14 Dmitry Titov <dimich@chromium.org>
authordimich@chromium.org <dimich@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Apr 2009 18:15:31 +0000 (18:15 +0000)
committerdimich@chromium.org <dimich@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Apr 2009 18:15:31 +0000 (18:15 +0000)
        Reviewed by Dimitri Glazkov.

        https://bugs.webkit.org/show_bug.cgi?id=25163
        Upstream fix for releasing v8 objects on worker thread termination in Chromium.

        * bindings/v8/V8DOMMap.cpp:
        (WebCore::domDataList): Now uses Vector instead of HashMap.
        (WebCore::domDataListMutex):
        (WebCore::ThreadSpecificDOMData::ThreadSpecificDOMData):  remove usage of currentThread();
        (WebCore::ThreadSpecificDOMData::~ThreadSpecificDOMData): ditto.
        (WebCore::NonMainThreadSpecificDOMData::~NonMainThreadSpecificDOMData): moved call to removeAllDOMObjectsInCurrentThread() to ~WorkerScriptController.
        (WebCore::handleWeakObjectInOwningThread):
        (WebCore::derefDelayedObjects):
        (WebCore::removeAllDOMObjectsInCurrentThread): not static anymore.
        * bindings/v8/V8DOMMap.h:
        * bindings/v8/WorkerContextExecutionProxy.cpp:
        (WebCore::WorkerContextExecutionProxy::dispose): removed code that avoided dual-dereference of WorkerContext.
        (WebCore::WorkerContextExecutionProxy::initContextIfNeeded): this ref() is countered in removeAllDOMObjectsInCurrentThread(), when the WorkerContext is removed from the v8 map.
        * bindings/v8/WorkerScriptController.cpp:
        (WebCore::WorkerScriptController::~WorkerScriptController):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@42504 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bindings/v8/V8DOMMap.cpp
WebCore/bindings/v8/V8DOMMap.h
WebCore/bindings/v8/WorkerContextExecutionProxy.cpp
WebCore/bindings/v8/WorkerScriptController.cpp

index fcfb71d..a954329 100644 (file)
@@ -1,3 +1,26 @@
+2009-04-14  Dmitry Titov  <dimich@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        https://bugs.webkit.org/show_bug.cgi?id=25163
+        Upstream fix for releasing v8 objects on worker thread termination in Chromium.
+
+        * bindings/v8/V8DOMMap.cpp:
+        (WebCore::domDataList): Now uses Vector instead of HashMap.
+        (WebCore::domDataListMutex):
+        (WebCore::ThreadSpecificDOMData::ThreadSpecificDOMData):  remove usage of currentThread();
+        (WebCore::ThreadSpecificDOMData::~ThreadSpecificDOMData): ditto.
+        (WebCore::NonMainThreadSpecificDOMData::~NonMainThreadSpecificDOMData): moved call to removeAllDOMObjectsInCurrentThread() to ~WorkerScriptController.
+        (WebCore::handleWeakObjectInOwningThread):
+        (WebCore::derefDelayedObjects):
+        (WebCore::removeAllDOMObjectsInCurrentThread): not static anymore.
+        * bindings/v8/V8DOMMap.h:
+        * bindings/v8/WorkerContextExecutionProxy.cpp:
+        (WebCore::WorkerContextExecutionProxy::dispose): removed code that avoided dual-dereference of WorkerContext.
+        (WebCore::WorkerContextExecutionProxy::initContextIfNeeded): this ref() is countered in removeAllDOMObjectsInCurrentThread(), when the WorkerContext is removed from the v8 map.
+        * bindings/v8/WorkerScriptController.cpp:
+        (WebCore::WorkerScriptController::~WorkerScriptController):
+
 2009-04-14  Adam Roben  <aroben@apple.com>
 
         Fix Bug 25183: Split up WebCore.vcproj's settings into vsprops files
index 759524b..60ce32b 100644 (file)
@@ -40,6 +40,7 @@
 #include <wtf/StdLibExtras.h>
 #include <wtf/Threading.h>
 #include <wtf/ThreadSpecific.h>
+#include <wtf/Vector.h>
 
 namespace WebCore {
 
@@ -103,23 +104,21 @@ static void weakSVGObjectWithContextCallback(v8::Persistent<v8::Value> v8Object,
 // The helper function will be scheduled by the GC thread to get called from the owning thread.
 static void derefDelayedObjectsInCurrentThread(void*);
 
-// This should be called to remove all DOM objects associated with the current thread when it is tearing down.
-static void removeAllDOMObjectsInCurrentThread();
-
-// A map from a thread ID to thread's specific data.
+// A list of all ThreadSpecific DOM Data objects. Traversed during GC to find a thread-specific map that
+// contains the object - so we can schedule the object to be deleted on the thread which created it.
 class ThreadSpecificDOMData;
-typedef WTF::HashMap<WTF::ThreadIdentifier, ThreadSpecificDOMData*> DOMThreadMap;
-static DOMThreadMap& domThreadMap()
+typedef WTF::Vector<ThreadSpecificDOMData*> DOMDataList;
+static DOMDataList& domDataList()
 {
-    DEFINE_STATIC_LOCAL(DOMThreadMap, staticDOMThreadMap, ());
-    return staticDOMThreadMap;
+    DEFINE_STATIC_LOCAL(DOMDataList, staticDOMDataList, ());
+    return staticDOMDataList;
 }
 
-// Mutex to protect against concurrent access of domThreadMap.
-static WTF::Mutex& domThreadMapMutex()
+// Mutex to protect against concurrent access of DOMDataList.
+static WTF::Mutex& domDataListMutex()
 {
-    DEFINE_STATIC_LOCAL(WTF::Mutex, staticDOMThreadMapMutex, ());
-    return staticDOMThreadMapMutex;
+    DEFINE_STATIC_LOCAL(WTF::Mutex, staticDOMDataListMutex, ());
+    return staticDOMDataListMutex;
 }
 
 class ThreadSpecificDOMData : Noncopyable {
@@ -161,14 +160,14 @@ public:
         , m_delayedProcessingScheduled(false)
         , m_isMainThread(WTF::isMainThread())
     {
-        WTF::MutexLocker locker(domThreadMapMutex());
-        domThreadMap().set(WTF::currentThread(), this);
+        WTF::MutexLocker locker(domDataListMutex());
+        domDataList().append(this);
     }
 
     virtual ~ThreadSpecificDOMData()
     {
-        WTF::MutexLocker locker(domThreadMapMutex());
-        domThreadMap().remove(WTF::currentThread());
+        WTF::MutexLocker locker(domDataListMutex());
+        domDataList().remove(domDataList().find(this));
     }
 
     void* getDOMWrapperMap(DOMWrapperMapType type)
@@ -241,8 +240,6 @@ public:
     // We assume that all child threads running V8 instances are created by WTF.
     virtual ~NonMainThreadSpecificDOMData()
     {
-        removeAllDOMObjectsInCurrentThread();
-
         delete m_domNodeMap;
         delete m_domObjectMap;
         delete m_activeDomObjectMap;
@@ -385,16 +382,10 @@ static void weakSVGObjectWithContextCallback(v8::Persistent<v8::Value> v8Object,
 template<typename T>
 static void handleWeakObjectInOwningThread(ThreadSpecificDOMData::DOMWrapperMapType mapType, V8ClassIndex::V8WrapperType objectType, T* object)
 {
-    WTF::MutexLocker locker(domThreadMapMutex());
-    for (typename DOMThreadMap::iterator iter(domThreadMap().begin()); iter != domThreadMap().end(); ++iter) {
-        WTF::ThreadIdentifier threadID = iter->first;
-        ThreadSpecificDOMData* threadData = iter->second;
-
-        // Skip the current thread that is GC thread.
-        if (threadID == WTF::currentThread()) {
-            ASSERT(!static_cast<DOMWrapperMap<T>*>(threadData->getDOMWrapperMap(mapType))->contains(object));
-            continue;
-        }
+    WTF::MutexLocker locker(domDataListMutex());
+    DOMDataList& list = domDataList();
+    for (size_t i = 0; i < list.size(); ++i) {
+        ThreadSpecificDOMData* threadData = list[i];
 
         ThreadSpecificDOMData::InternalDOMWrapperMap<T>* domMap = static_cast<ThreadSpecificDOMData::InternalDOMWrapperMap<T>*>(threadData->getDOMWrapperMap(mapType));
         if (domMap->contains(object)) {
@@ -514,7 +505,7 @@ static void derefObject(V8ClassIndex::V8WrapperType type, void* domObject)
 
 static void derefDelayedObjects()
 {
-    WTF::MutexLocker locker(domThreadMapMutex());
+    WTF::MutexLocker locker(domDataListMutex());
 
     getThreadSpecificDOMData().setDelayedProcessingScheduled(false);
 
@@ -573,7 +564,7 @@ static void removeAllDOMObjectsInCurrentThreadHelper()
 #endif
 }
 
-static void removeAllDOMObjectsInCurrentThread()
+void removeAllDOMObjectsInCurrentThread()
 {
     // Use the locker only if it has already been invoked before, as by worker thread.
     if (v8::Locker::IsActive()) {
index 909fbcd..1a33188 100644 (file)
@@ -53,6 +53,9 @@ namespace WebCore {
     // A map from a DOM object to its JS wrapper for DOM objects which can have pending activity.
     DOMWrapperMap<void>& getActiveDOMObjectMap();
 
+    // This should be called to remove all DOM objects associated with the current thread when it is tearing down.
+    void removeAllDOMObjectsInCurrentThread();
+
 #if ENABLE(SVG)
     // A map for SVGElementInstances to its JS wrapper.
     DOMWrapperMap<SVGElementInstance>& getDOMSVGElementInstanceMap();
index 45e362d..17cd86d 100644 (file)
@@ -126,17 +126,6 @@ void WorkerContextExecutionProxy::dispose()
         m_context.Dispose();
         m_context.Clear();
     }
-
-    // Remove the wrapping between JS object and DOM object. This is because
-    // the worker context object is going to be disposed immediately when a
-    // worker thread is tearing down. We do not want to re-delete the real object
-    // when JS object is garbage collected.
-    v8::Locker locker;
-    v8::HandleScope scope;
-    v8::Persistent<v8::Object> wrapper = domObjectMap().get(m_workerContext);
-    if (!wrapper.IsEmpty())
-        V8Proxy::SetDOMWrapper(wrapper, V8ClassIndex::INVALID_CLASS_INDEX, 0);
-    domObjectMap().forget(m_workerContext);
 }
 
 WorkerContextExecutionProxy* WorkerContextExecutionProxy::retrieve()
@@ -204,6 +193,7 @@ void WorkerContextExecutionProxy::initContextIfNeeded()
     V8Proxy::SetDOMWrapper(jsWorkerContext, V8ClassIndex::ToInt(V8ClassIndex::WORKERCONTEXT), m_workerContext);
 
     V8Proxy::SetJSWrapperForDOMObject(m_workerContext, v8::Persistent<v8::Object>::New(jsWorkerContext));
+    m_workerContext->ref();
 
     // Insert the object instance as the prototype of the shadow object.
     v8::Handle<v8::Object> globalObject = m_context->Global();
index 85bee0a..79322b7 100644 (file)
@@ -55,6 +55,7 @@ WorkerScriptController::WorkerScriptController(WorkerContext* workerContext)
 
 WorkerScriptController::~WorkerScriptController()
 {
+    removeAllDOMObjectsInCurrentThread();
 }
 
 ScriptValue WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode)