Changeset 286800 in webkit

Timestamp:

Dec 9, 2021, 1:14:04 PM (5 years ago)

Author:

Chris Dumez

Message:

[WPE] Crash under WebProcessProxy::setIsInProcessCache when closing web view in debug builds
​https://bugs.webkit.org/show_bug.cgi?id=233933

Reviewed by Geoffrey Garen.

The crash would occur because the WebProcessPool destructor would call WebProcessCache::clear()
which would destroy WebProcessCache::CachedProcess objects, causing
WebProcessProxy::setIsInProcessCache(false) to get called. Previously, this call to
setIsInProcessCache() would convert the WeakPtr the WebProcessProxy held to its process pool
into a RefPtr, thus causing the WebProcessPool to get ref'd while in the middle of destruction.

To address the issue, the setIsInProcessCache() setter now takes a WillShutDown flag that gets
set in the CachedProcess destructor and which causes setIsInProcessCache() to return early
right after setting the m_isInProcessCache flag, without trying to send IPC to the WebProcess
or trying to ref the WebProcessPool.

• UIProcess/WebProcessCache.cpp:

(WebKit::WebProcessCache::CachedProcess::~CachedProcess):

• UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::setIsInProcessCache):

• UIProcess/WebProcessProxy.h:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebProcessCache.cpp (modified) (1 diff)
• UIProcess/WebProcessProxy.cpp (modified) (2 diffs)
• UIProcess/WebProcessProxy.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-12-09  Chris Dumez  <cdumez@apple.com>
+
+        [WPE] Crash under WebProcessProxy::setIsInProcessCache when closing web view in debug builds
+        https://bugs.webkit.org/show_bug.cgi?id=233933
+
+        Reviewed by Geoffrey Garen.
+
+        The crash would occur because the WebProcessPool destructor would call WebProcessCache::clear()
+        which would destroy WebProcessCache::CachedProcess objects, causing
+        WebProcessProxy::setIsInProcessCache(false) to get called. Previously, this call to
+        setIsInProcessCache() would convert the WeakPtr the WebProcessProxy held to its process pool
+        into a RefPtr, thus causing the WebProcessPool to get ref'd while in the middle of destruction.
+
+        To address the issue, the setIsInProcessCache() setter now takes a WillShutDown flag that gets
+        set in the CachedProcess destructor and which causes setIsInProcessCache() to return early
+        right after setting the m_isInProcessCache flag, without trying to send IPC to the WebProcess
+        or trying to ref the WebProcessPool.
+
+        * UIProcess/WebProcessCache.cpp:
+        (WebKit::WebProcessCache::CachedProcess::~CachedProcess):
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::setIsInProcessCache):
+        * UIProcess/WebProcessProxy.h:
+
 2021-12-08  BJ Burg  <bburg@apple.com>
 

trunk/Source/WebKit/UIProcess/WebProcessCache.cpp

@@ -295 +295 @@
         m_process->platformResumeProcess();
 #endif
-    m_process->setIsInProcessCache(false);
+    m_process->setIsInProcessCache(false, WebProcessProxy::WillShutDown::Yes);
     m_process->shutDown();
 }

trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -310 +310 @@
 #endif
 
-void WebProcessProxy::setIsInProcessCache(bool value)
+void WebProcessProxy::setIsInProcessCache(bool value, WillShutDown willShutDown)
 {
     WEBPROCESSPROXY_RELEASE_LOG(Process, "setIsInProcessCache(%d)", value);
@@ -321 +321 @@
     ASSERT(m_isInProcessCache != value);
     m_isInProcessCache = value;
+
+    // No point in doing anything else if the process is about to shut down.
+    ASSERT(willShutDown == WillShutDown::No || !value);
+    if (willShutDown == WillShutDown::Yes)
+        return;
 
     send(Messages::WebProcess::SetIsInProcessCache(m_isInProcessCache), 0);

trunk/Source/WebKit/UIProcess/WebProcessProxy.h

@@ -157 +157 @@
     WebCore::RegistrableDomain registrableDomain() const { return m_registrableDomain.value_or(WebCore::RegistrableDomain { }); }
     const std::optional<WebCore::RegistrableDomain>& optionalRegistrableDomain() const { return m_registrableDomain; }
-    void setIsInProcessCache(bool);
+
+    enum class WillShutDown : bool { No, Yes };
+    void setIsInProcessCache(bool, WillShutDown = WillShutDown::No);
     bool isInProcessCache() const { return m_isInProcessCache; }