Changeset 225491 in webkit

Timestamp:

Dec 4, 2017 1:56:25 PM (6 years ago)

Author:

Chris Dumez

Message:

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• workers/service/ServiceWorkerGlobalScope.cpp (modified) (1 diff)
• workers/service/ServiceWorkerGlobalScope.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp

@@ -51 +51 @@
 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();
                 });
             });

trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.h

@@ -78 +78 @@
     HashMap<ServiceWorkerClientIdentifier, ServiceWorkerClient*> m_clientMap;
     Vector<Ref<ExtendableEvent>> m_extendedEvents;
+
+    uint64_t m_lastRequestIdentifier { 0 };
+    HashMap<uint64_t, RefPtr<DeferredPromise>> m_pendingSkipWaitingPromises;
 };