Changeset 248987 in webkit

Timestamp:

Aug 21, 2019, 6:25:43 PM (7 years ago)

Author:

Kocsen Chung

Message:

Cherry-pick r248959. rdar://problem/54579630

Crash under StringImpl::~StringImpl() in NetworkProcess::deleteWebsiteDataForRegistrableDomains()
​https://bugs.webkit.org/show_bug.cgi?id=200986
<rdar://problem/32850192>

Reviewed by Brent Fulgham.

Code was calling postStorageTask() with a lambda that captures Strings that are not isolated copied.
The lambda would get executed on another thread so this is not safe. The CrossThreadTask constructor
does not take care of this for you, despite its name (the createCrossThreadTask() function does though).

• NetworkProcess/NetworkProcess.cpp: (WebKit::NetworkProcess::fetchWebsiteData): (WebKit::NetworkProcess::deleteWebsiteDataForRegistrableDomains): (WebKit::NetworkProcess::registrableDomainsWithWebsiteData):

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

Location:

branches/safari-608-branch/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/NetworkProcess.cpp (modified) (3 diffs)

branches/safari-608-branch/Source/WebKit/ChangeLog

@@ +1 @@
+2019-08-21  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Cherry-pick r248959. rdar://problem/54579630
+
+    Crash under StringImpl::~StringImpl() in NetworkProcess::deleteWebsiteDataForRegistrableDomains()
+    https://bugs.webkit.org/show_bug.cgi?id=200986
+    <rdar://problem/32850192>
+    
+    Reviewed by Brent Fulgham.
+    
+    Code was calling postStorageTask() with a lambda that captures Strings that are not isolated copied.
+    The lambda would get executed on another thread so this is not safe. The CrossThreadTask constructor
+    does not take care of this for you, despite its name (the createCrossThreadTask() function does though).
+    
+    * NetworkProcess/NetworkProcess.cpp:
+    (WebKit::NetworkProcess::fetchWebsiteData):
+    (WebKit::NetworkProcess::deleteWebsiteDataForRegistrableDomains):
+    (WebKit::NetworkProcess::registrableDomainsWithWebsiteData):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248959 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-08-21  Chris Dumez  <cdumez@apple.com>
+
+            Crash under StringImpl::~StringImpl() in NetworkProcess::deleteWebsiteDataForRegistrableDomains()
+            https://bugs.webkit.org/show_bug.cgi?id=200986
+            <rdar://problem/32850192>
+
+            Reviewed by Brent Fulgham.
+
+            Code was calling postStorageTask() with a lambda that captures Strings that are not isolated copied.
+            The lambda would get executed on another thread so this is not safe. The CrossThreadTask constructor
+            does not take care of this for you, despite its name (the createCrossThreadTask() function does though).
+
+            * NetworkProcess/NetworkProcess.cpp:
+            (WebKit::NetworkProcess::fetchWebsiteData):
+            (WebKit::NetworkProcess::deleteWebsiteDataForRegistrableDomains):
+            (WebKit::NetworkProcess::registrableDomainsWithWebsiteData):
+
 2019-08-20  Babak Shafiei  <bshafiei@apple.com>
 

branches/safari-608-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp

@@ -1340 +1340 @@
     if (!path.isEmpty() && websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases)) {
         // FIXME: Pick the right database store based on the session ID.
-        postStorageTask(CrossThreadTask([this, callbackAggregator = callbackAggregator.copyRef(), path = WTFMove(path)]() mutable {
+        postStorageTask(CrossThreadTask([this, callbackAggregator = callbackAggregator.copyRef(), path = crossThreadCopy(path)]() mutable {
             RunLoop::main().dispatch([callbackAggregator = WTFMove(callbackAggregator), securityOrigins = indexedDatabaseOrigins(path)] {
                 for (const auto& securityOrigin : securityOrigins)
@@ -1718 +1718 @@
     if (!path.isEmpty() && websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases)) {
         // FIXME: Pick the right database store based on the session ID.
-        postStorageTask(CrossThreadTask([this, sessionID, callbackAggregator = callbackAggregator.copyRef(), path = WTFMove(path), domainsToDeleteAllButCookiesFor]() mutable {
-            RunLoop::main().dispatch([this, sessionID, domainsToDeleteAllButCookiesFor = crossThreadCopy(domainsToDeleteAllButCookiesFor), callbackAggregator = callbackAggregator.copyRef(), securityOrigins = indexedDatabaseOrigins(path)] {
+        postStorageTask(CrossThreadTask([this, sessionID, callbackAggregator = callbackAggregator.copyRef(), path = crossThreadCopy(path), domainsToDeleteAllButCookiesFor = crossThreadCopy(domainsToDeleteAllButCookiesFor)]() mutable {
+            RunLoop::main().dispatch([this, sessionID, domainsToDeleteAllButCookiesFor = WTFMove(domainsToDeleteAllButCookiesFor), callbackAggregator = callbackAggregator.copyRef(), securityOrigins = indexedDatabaseOrigins(path)] {
                 Vector<SecurityOriginData> entriesToDelete;
                 for (const auto& securityOrigin : securityOrigins) {
@@ -1863 +1863 @@
     if (!path.isEmpty() && websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases)) {
         // FIXME: Pick the right database store based on the session ID.
-        postStorageTask(CrossThreadTask([this, callbackAggregator = callbackAggregator.copyRef(), path = WTFMove(path)]() mutable {
+        postStorageTask(CrossThreadTask([this, callbackAggregator = callbackAggregator.copyRef(), path = crossThreadCopy(path)]() mutable {
             RunLoop::main().dispatch([callbackAggregator = callbackAggregator.copyRef(), securityOrigins = indexedDatabaseOrigins(path)] {
                 for (const auto& securityOrigin : securityOrigins)