ServiceWorker WebProcess sometimes crashes in JSVMClientData::~JSVMClientData()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Nov 2017 06:13:40 +0000 (06:13 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Nov 2017 06:13:40 +0000 (06:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180173

Reviewed by Alex Christensen.

The leak was caused by EventListeners remaining when destroying the VM, because
JSEventListener refs the DOMWrapperWorld. To address the issue, we now call
removeAllEventListeners() in the stop() method of ServiceWorkerContainer,
ServiceWorkerRegistration and ServiceWorker. Those event listeners are no
longer needed after ActiveDOMObject::stop() is called since the script
execution context is about to be destroyed.

This is the same pattern used in IDBDatabase::stop(), IDBRequest::stop().

No new tests, already covered by existing test.

* workers/service/ServiceWorker.cpp:
(WebCore::ServiceWorker::stop):
* workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::stop):
* workers/service/ServiceWorkerContainer.h:
* workers/service/ServiceWorkerRegistration.cpp:
(WebCore::ServiceWorkerRegistration::stop):

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

Source/WebCore/ChangeLog
Source/WebCore/workers/service/ServiceWorker.cpp
Source/WebCore/workers/service/ServiceWorkerContainer.cpp
Source/WebCore/workers/service/ServiceWorkerContainer.h
Source/WebCore/workers/service/ServiceWorkerRegistration.cpp

index 7132df1..9b94f99 100644 (file)
@@ -1,3 +1,29 @@
+2017-11-29  Chris Dumez  <cdumez@apple.com>
+
+        ServiceWorker WebProcess sometimes crashes in JSVMClientData::~JSVMClientData()
+        https://bugs.webkit.org/show_bug.cgi?id=180173
+
+        Reviewed by Alex Christensen.
+
+        The leak was caused by EventListeners remaining when destroying the VM, because
+        JSEventListener refs the DOMWrapperWorld. To address the issue, we now call
+        removeAllEventListeners() in the stop() method of ServiceWorkerContainer,
+        ServiceWorkerRegistration and ServiceWorker. Those event listeners are no
+        longer needed after ActiveDOMObject::stop() is called since the script
+        execution context is about to be destroyed.
+
+        This is the same pattern used in IDBDatabase::stop(), IDBRequest::stop().
+
+        No new tests, already covered by existing test.
+
+        * workers/service/ServiceWorker.cpp:
+        (WebCore::ServiceWorker::stop):
+        * workers/service/ServiceWorkerContainer.cpp:
+        (WebCore::ServiceWorkerContainer::stop):
+        * workers/service/ServiceWorkerContainer.h:
+        * workers/service/ServiceWorkerRegistration.cpp:
+        (WebCore::ServiceWorkerRegistration::stop):
+
 2017-11-29  Filip Pizlo  <fpizlo@apple.com>
 
         GC should support isoheaps
index 12970ea..c443fe8 100644 (file)
@@ -149,6 +149,7 @@ bool ServiceWorker::canSuspendForDocumentSuspension() const
 void ServiceWorker::stop()
 {
     m_isStopped = true;
+    removeAllEventListeners();
     scriptExecutionContext()->unregisterServiceWorker(*this);
     updatePendingActivityForEventDispatch();
 }
index 75ec664..3f29c6b 100644 (file)
@@ -445,6 +445,12 @@ void ServiceWorkerContainer::scheduleTaskToFireControllerChangeEvent()
     });
 }
 
+void ServiceWorkerContainer::stop()
+{
+    m_isStopped = true;
+    removeAllEventListeners();
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(SERVICE_WORKER)
index a0a8e75..bb5da69 100644 (file)
@@ -105,7 +105,7 @@ private:
     EventTargetInterface eventTargetInterface() const final { return ServiceWorkerContainerEventTargetInterfaceType; }
     void refEventTarget() final;
     void derefEventTarget() final;
-    void stop() final { m_isStopped = true; }
+    void stop() final;
 
     ReadyPromise m_readyPromise;
 
index d292591..bf0b479 100644 (file)
@@ -221,6 +221,7 @@ bool ServiceWorkerRegistration::canSuspendForDocumentSuspension() const
 void ServiceWorkerRegistration::stop()
 {
     m_isStopped = true;
+    removeAllEventListeners();
     updatePendingActivityForEventDispatch();
 }