Changeset 246606 in webkit

Timestamp:

Jun 19, 2019, 12:44:10 PM (7 years ago)

Author:

sihui_liu@apple.com

Message:

Crash at com.apple.WebKit: WebKit::WebsiteDataStore::processPools const
​https://bugs.webkit.org/show_bug.cgi?id=198935
<rdar://problem/51549308>

Reviewed by Geoffrey Garen.

When WebProcessProxy is in WebProcessCache or is pre-warmed, it does not hold a strong reference of
WebProcessPool. In this case, we should not store the raw pointer of WebProcessPool and perform websiteDataStore
operations with it.
This patch should fix the crash at dereferencing null pointer of WebProcessPool in
WebsiteDataStore::processPools, but it is unclear why websiteDataStore comes to observe cached or prewarmed web
process that should not have web page. The release log may help us find the cause.

• UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::processPoolIfExists const):

• UIProcess/WebProcessProxy.h:
• UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::processPools const):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebProcessProxy.cpp (modified) (1 diff)
• UIProcess/WebProcessProxy.h (modified) (1 diff)
• UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-06-19  Sihui Liu  <sihui_liu@apple.com>
+
+        Crash at com.apple.WebKit: WebKit::WebsiteDataStore::processPools const
+        https://bugs.webkit.org/show_bug.cgi?id=198935
+        <rdar://problem/51549308>
+
+        Reviewed by Geoffrey Garen.
+
+        When WebProcessProxy is in WebProcessCache or is pre-warmed, it does not hold a strong reference of 
+        WebProcessPool. In this case, we should not store the raw pointer of WebProcessPool and perform websiteDataStore
+        operations with it.
+        This patch should fix the crash at dereferencing null pointer of WebProcessPool in 
+        WebsiteDataStore::processPools, but it is unclear why websiteDataStore comes to observe cached or prewarmed web 
+        process that should not have web page. The release log may help us find the cause.
+
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::processPoolIfExists const):
+        * UIProcess/WebProcessProxy.h:
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::processPools const):
+
 2019-06-19  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -1517 +1517 @@
 }
 
+WebProcessPool* WebProcessProxy::processPoolIfExists() const
+{
+    if (m_isPrewarmed || m_isInProcessCache)
+        RELEASE_LOG_ERROR(Process, "%p - WebProcessProxy::processPoolIfExists: trying to get WebProcessPool from an inactive WebProcessProxy %i", this, processIdentifier());
+    else
+        ASSERT(m_processPool);
+    return m_processPool.get();
+}
+
 WebProcessPool& WebProcessProxy::processPool() const
 {

trunk/Source/WebKit/UIProcess/WebProcessProxy.h

@@ -120 +120 @@
     void decrementSuspendedPageCount();
 
+    WebProcessPool* processPoolIfExists() const;
     WebProcessPool& processPool() const;
 

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

@@ -1718 +1718 @@
 {
     HashSet<RefPtr<WebProcessPool>> processPools;
-    for (auto& process : processes())
-        processPools.add(&process->processPool());
+    for (auto& process : processes()) {
+        if (auto* processPool = process->processPoolIfExists())
+            processPools.add(processPool);
+    }
 
     if (processPools.isEmpty()) {