Title: [225291] trunk/Source/WebCore
Revision
225291
Author
commit-qu...@webkit.org
Date
2017-11-29 13:39:56 -0800 (Wed, 29 Nov 2017)

Log Message

Flaky crash in WebCore::DOMGuardedObject::clear() during service worker tests
https://bugs.webkit.org/show_bug.cgi?id=180045
<rdar://problem/35737288>

Patch by Youenn Fablet <you...@apple.com> on 2017-11-29
Reviewed by Chris Dumez.

Manually tested by running concurrently service worker tests using FetchEvents which store promise references.

Before the patch, on workers, clearing of DOMGuardedObjects happens at the time WorkerGlobalScope is destroyed.
This is too late as it is expected that the JSDOMGlobalObject is still alive.

This patch adds a clearDOMGuardedObjects method on JSWorkerGlobalScopeBase.
It is called when stopping a WorkerThread, just before releasing the strong reference to JSWorkerGlobalScopeBase.

* bindings/js/JSDOMGuardedObject.h:
* bindings/js/JSWorkerGlobalScopeBase.cpp:
(WebCore::JSWorkerGlobalScopeBase::clearDOMGuardedObjects):
* bindings/js/JSWorkerGlobalScopeBase.h:
* bindings/js/WorkerScriptController.cpp:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (225290 => 225291)


--- trunk/Source/WebCore/ChangeLog	2017-11-29 21:31:07 UTC (rev 225290)
+++ trunk/Source/WebCore/ChangeLog	2017-11-29 21:39:56 UTC (rev 225291)
@@ -1,3 +1,25 @@
+2017-11-29  Youenn Fablet  <you...@apple.com>
+
+        Flaky crash in WebCore::DOMGuardedObject::clear() during service worker tests
+        https://bugs.webkit.org/show_bug.cgi?id=180045
+        <rdar://problem/35737288>
+
+        Reviewed by Chris Dumez.
+
+        Manually tested by running concurrently service worker tests using FetchEvents which store promise references.
+
+        Before the patch, on workers, clearing of DOMGuardedObjects happens at the time WorkerGlobalScope is destroyed.
+        This is too late as it is expected that the JSDOMGlobalObject is still alive.
+
+        This patch adds a clearDOMGuardedObjects method on JSWorkerGlobalScopeBase.
+        It is called when stopping a WorkerThread, just before releasing the strong reference to JSWorkerGlobalScopeBase.
+
+        * bindings/js/JSDOMGuardedObject.h:
+        * bindings/js/JSWorkerGlobalScopeBase.cpp:
+        (WebCore::JSWorkerGlobalScopeBase::clearDOMGuardedObjects):
+        * bindings/js/JSWorkerGlobalScopeBase.h:
+        * bindings/js/WorkerScriptController.cpp:
+
 2017-11-29  Alex Christensen  <achristen...@webkit.org>
 
         Fix Mac CMake build.

Modified: trunk/Source/WebCore/bindings/js/JSDOMGuardedObject.h (225290 => 225291)


--- trunk/Source/WebCore/bindings/js/JSDOMGuardedObject.h	2017-11-29 21:31:07 UTC (rev 225290)
+++ trunk/Source/WebCore/bindings/js/JSDOMGuardedObject.h	2017-11-29 21:39:56 UTC (rev 225291)
@@ -45,10 +45,11 @@
     JSC::JSValue guardedObject() const { return m_guarded.get(); }
     JSDOMGlobalObject* globalObject() const { return m_globalObject.get(); }
 
+    void clear();
+
 protected:
     DOMGuardedObject(JSDOMGlobalObject&, JSC::JSCell&);
 
-    void clear();
     void contextDestroyed() override;
     bool isEmpty() { return !m_guarded; }
 

Modified: trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp (225290 => 225291)


--- trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp	2017-11-29 21:31:07 UTC (rev 225290)
+++ trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp	2017-11-29 21:39:56 UTC (rev 225291)
@@ -79,6 +79,13 @@
     ASSERT(inherits(vm, info()));
 }
 
+void JSWorkerGlobalScopeBase::clearDOMGuardedObjects()
+{
+    auto guardedObjects = m_guardedObjects;
+    for (auto& guarded : guardedObjects)
+        guarded->clear();
+}
+
 void JSWorkerGlobalScopeBase::visitChildren(JSCell* cell, SlotVisitor& visitor)
 {
     JSWorkerGlobalScopeBase* thisObject = jsCast<JSWorkerGlobalScopeBase*>(cell);

Modified: trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h (225290 => 225291)


--- trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h	2017-11-29 21:31:07 UTC (rev 225290)
+++ trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h	2017-11-29 21:39:56 UTC (rev 225291)
@@ -67,6 +67,8 @@
     static JSC::RuntimeFlags _javascript_RuntimeFlags(const JSC::JSGlobalObject*);
     static void queueTaskToEventLoop(JSC::JSGlobalObject&, Ref<JSC::Microtask>&&);
 
+    void clearDOMGuardedObjects();
+
 protected:
     JSWorkerGlobalScopeBase(JSC::VM&, JSC::Structure*, RefPtr<WorkerGlobalScope>&&);
     void finishCreation(JSC::VM&, JSC::JSProxy*);

Modified: trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp (225290 => 225291)


--- trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp	2017-11-29 21:31:07 UTC (rev 225290)
+++ trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp	2017-11-29 21:39:56 UTC (rev 225291)
@@ -61,6 +61,7 @@
 {
     JSLockHolder lock(vm());
     if (m_workerGlobalScopeWrapper) {
+        m_workerGlobalScopeWrapper->clearDOMGuardedObjects();
         m_workerGlobalScopeWrapper->setConsoleClient(nullptr);
         m_consoleClient = nullptr;
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to