Changeset 252897 in webkit

Timestamp:

Nov 27, 2019 12:55:48 AM (4 years ago)

Author:

youenn@apple.com

Message:

Protect ServiceWorkerThreadProxy while being stopped
​https://bugs.webkit.org/show_bug.cgi?id=204610
<rdar://problem/57476332>

Reviewed by Chris Dumez.

Reuse stop implementation in terminateWorker for stopAllServiceWorkers.
Move the worker map value from RefPtr to Ref.
Covered by existing tests no longer crashing.

• workers/service/context/SWContextManager.cpp:

(WebCore::SWContextManager::terminateWorker):
(WebCore::SWContextManager::stopWorker):
(WebCore::SWContextManager::forEachServiceWorkerThread):
(WebCore::SWContextManager::stopAllServiceWorkers):

• workers/service/context/SWContextManager.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• workers/service/context/SWContextManager.cpp (modified) (3 diffs)
• workers/service/context/SWContextManager.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-11-27  youenn fablet  <youenn@apple.com>
+
+        Protect ServiceWorkerThreadProxy while being stopped
+        https://bugs.webkit.org/show_bug.cgi?id=204610
+        <rdar://problem/57476332>
+
+        Reviewed by Chris Dumez.
+
+        Reuse stop implementation in terminateWorker for stopAllServiceWorkers.
+        Move the worker map value from RefPtr to Ref.
+        Covered by existing tests no longer crashing.
+
+        * workers/service/context/SWContextManager.cpp:
+        (WebCore::SWContextManager::terminateWorker):
+        (WebCore::SWContextManager::stopWorker):
+        (WebCore::SWContextManager::forEachServiceWorkerThread):
+        (WebCore::SWContextManager::stopAllServiceWorkers):
+        * workers/service/context/SWContextManager.h:
+
 2019-11-26  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/workers/service/context/SWContextManager.cpp

@@ -126 +126 @@
         return;
     }
+    stopWorker(*serviceWorker, timeout, WTFMove(completionHandler));
+}
 
-    serviceWorker->setAsTerminatingOrTerminated();
+void SWContextManager::stopWorker(ServiceWorkerThreadProxy& serviceWorker, Seconds timeout, Function<void()>&& completionHandler)
+{
+    auto identifier = serviceWorker.identifier();
+    serviceWorker.setAsTerminatingOrTerminated();
 
     m_pendingServiceWorkerTerminationRequests.add(identifier, makeUnique<ServiceWorkerTerminationRequest>(*this, identifier, timeout));
 
-    auto& thread = serviceWorker->thread();
-    thread.stop([this, identifier, serviceWorker = WTFMove(serviceWorker), completionHandler = WTFMove(completionHandler)]() mutable {
+    auto& thread = serviceWorker.thread();
+    thread.stop([this, identifier, serviceWorker = makeRef(serviceWorker), completionHandler = WTFMove(completionHandler)]() mutable {
         m_pendingServiceWorkerTerminationRequests.remove(identifier);
 
@@ -150 +155 @@
 {
     for (auto& workerThread : m_workerMap.values())
-        apply(*workerThread);
+        apply(workerThread);
 }
 
@@ -181 +186 @@
 {
     auto serviceWorkers = WTFMove(m_workerMap);
-    for (auto& serviceWorker : serviceWorkers.values()) {
-        serviceWorker->setAsTerminatingOrTerminated();
-        m_pendingServiceWorkerTerminationRequests.add(serviceWorker->identifier(), makeUnique<ServiceWorkerTerminationRequest>(*this, serviceWorker->identifier(), workerTerminationTimeout));
-        serviceWorker->thread().stop([] { });
-    }
+    for (auto& serviceWorker : serviceWorkers.values())
+        stopWorker(serviceWorker, workerTerminationTimeout, [] { });
 }
 

trunk/Source/WebCore/workers/service/context/SWContextManager.h

@@ -107 +107 @@
     NO_RETURN_DUE_TO_CRASH void serviceWorkerFailedToTerminate(ServiceWorkerIdentifier);
 
-    HashMap<ServiceWorkerIdentifier, RefPtr<ServiceWorkerThreadProxy>> m_workerMap;
+    void stopWorker(ServiceWorkerThreadProxy&, Seconds, Function<void()>&&);
+
+    HashMap<ServiceWorkerIdentifier, Ref<ServiceWorkerThreadProxy>> m_workerMap;
     std::unique_ptr<Connection> m_connection;
     ServiceWorkerCreationCallback* m_serviceWorkerCreationCallback { nullptr };