ServiceWorkerGlobalScope::skipWaiting(Ref<DeferredPromise>&&) is unsafe

https://bugs.webkit.org/show_bug.cgi?id=180372

Reviewed by Youenn Fablet.

Ref the WorkerThread and capture it in the lambda. Keep the pending promises in
a HashMap on the ServiceWorkerGlobalScope so that they stay on the worker thread.

* workers/service/ServiceWorkerGlobalScope.cpp:
(WebCore::ServiceWorkerGlobalScope::skipWaiting):
* workers/service/ServiceWorkerGlobalScope.h:

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index acf1eb6..3aea2c5 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2017-12-04  Chris Dumez  <cdumez@apple.com>
+
+        ServiceWorkerGlobalScope::skipWaiting(Ref<DeferredPromise>&&) is unsafe
+        https://bugs.webkit.org/show_bug.cgi?id=180372
+
+        Reviewed by Youenn Fablet.
+
+        Ref the WorkerThread and capture it in the lambda. Keep the pending promises in
+        a HashMap on the ServiceWorkerGlobalScope so that they stay on the worker thread.
+
+        * workers/service/ServiceWorkerGlobalScope.cpp:
+        (WebCore::ServiceWorkerGlobalScope::skipWaiting):
+        * workers/service/ServiceWorkerGlobalScope.h:
+
 2017-12-04  Brady Eidson  <beidson@apple.com>
 
         Get a directory path to SWServers for storing ServiceWorker registrations.

diff --git a/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp b/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp
index d7628de..48828d9 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp
+++ b/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp
@@ -50,11 +50,16 @@ ServiceWorkerGlobalScope::~ServiceWorkerGlobalScope() = default;
 
 void ServiceWorkerGlobalScope::skipWaiting(Ref<DeferredPromise>&& promise)
 {
-    callOnMainThread([this, protectedThis = makeRef(*this), threadIdentifier = thread().identifier(), promise = WTFMove(promise)]() mutable {
+    uint64_t requestIdentifier = ++m_lastRequestIdentifier;
+    m_pendingSkipWaitingPromises.add(requestIdentifier, WTFMove(promise));
+
+    callOnMainThread([workerThread = makeRef(thread()), requestIdentifier]() mutable {
         if (auto* connection = SWContextManager::singleton().connection()) {
-            connection->skipWaiting(threadIdentifier, [this, protectedThis = WTFMove(protectedThis), promise = WTFMove(promise)]() mutable {
-                thread().runLoop().postTask([promise = WTFMove(promise), protectedThis = WTFMove(protectedThis)](auto&) {
-                    promise->resolve();
+            connection->skipWaiting(workerThread->identifier(), [workerThread = WTFMove(workerThread), requestIdentifier] {
+                workerThread->runLoop().postTask([requestIdentifier](auto& context) {
+                    auto& scope = downcast<ServiceWorkerGlobalScope>(context);
+                    if (auto promise = scope.m_pendingSkipWaitingPromises.take(requestIdentifier))
+                        promise->resolve();
                 });
             });
         }

diff --git a/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h b/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h
index a7db0cc..211431e 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h
+++ b/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h
@@ -77,6 +77,9 @@ private:
     Ref<ServiceWorkerClients> m_clients;
     HashMap<ServiceWorkerClientIdentifier, ServiceWorkerClient*> m_clientMap;
     Vector<Ref<ExtendableEvent>> m_extendedEvents;
+
+    uint64_t m_lastRequestIdentifier { 0 };
+    HashMap<uint64_t, RefPtr<DeferredPromise>> m_pendingSkipWaitingPromises;
 };
 
 } // namespace WebCore