[webkit-changes] [WebKit/WebKit] e4fcb9: Regression(267014 at main) Flaky crash under WebKit::...

Chris Dumez noreply at github.com
Wed Oct 11 11:14:06 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: e4fcb9c6b04bedb7d92df0b301198c9ab601562b
      https://github.com/WebKit/WebKit/commit/e4fcb9c6b04bedb7d92df0b301198c9ab601562b
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2023-10-11 (Wed, 11 Oct 2023)

  Changed paths:
    M Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp
    M Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h
    M Source/WebKit/GPUProcess/GPUProcess.cpp
    M Source/WebKit/GPUProcess/GPUProcess.h
    M Source/WebKit/GPUProcess/GPUProcess.messages.in
    M Source/WebKit/Shared/GPUProcessConnectionParameters.h
    M Source/WebKit/Shared/GPUProcessConnectionParameters.serialization.in
    M Source/WebKit/Shared/GPUProcessPreferencesForWebProcess.h
    M Source/WebKit/Shared/GPUProcessPreferencesForWebProcess.serialization.in
    M Source/WebKit/UIProcess/API/APIPageConfiguration.cpp
    M Source/WebKit/UIProcess/API/APIPageConfiguration.h
    M Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp
    M Source/WebKit/UIProcess/GPU/GPUProcessProxy.h
    M Source/WebKit/UIProcess/RemotePageProxy.h
    M Source/WebKit/UIProcess/SuspendedPageProxy.cpp
    M Source/WebKit/UIProcess/SuspendedPageProxy.h
    M Source/WebKit/UIProcess/WebPageProxy.cpp
    M Source/WebKit/UIProcess/WebPageProxy.h
    M Source/WebKit/UIProcess/WebProcessCache.cpp
    M Source/WebKit/UIProcess/WebProcessCache.h
    M Source/WebKit/UIProcess/WebProcessPool.cpp
    M Source/WebKit/UIProcess/WebProcessPool.h
    M Source/WebKit/UIProcess/WebProcessProxy.cpp
    M Source/WebKit/UIProcess/WebProcessProxy.h
    M Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp
    M Source/WebKit/WebProcess/WebProcess.h

  Log Message:
  -----------
  Regression(267014 at main) Flaky crash under WebKit::GPUConnectionToWebProcess::didReceiveMessage()
https://bugs.webkit.org/show_bug.cgi?id=262981
rdar://114245301

Reviewed by Simon Fraser.

In 267014 at main, we added support for [EnabledIf=Setting] in our IPC messages.in files and
started using it for a few IPC messages. Since then we've had flaky crashes on the bots
inside GPUConnectionToWebProcess::didReceiveMessage() because these IPC messages are
sometimes not handled.

The issue is that the checks were racy. The GPUProcess would for example receive the
CreateRemoteGPU IPC and check the `isWebGPUEnabled()` setting. If the setting is disabled,
it would refuse to handle the IPC message and we would crash in debug.

The reason this is racy is that the tests keep enabling / disabling these settings. These
settings are propagated via asynchronous IPC from the UIProcess to the GPUProcess. However,
the IPCs that check for those settings in the GPUProcess are coming from the WebProcess,
not the UIProcess. As a result, it is possible for this to happen:
1. A page gets created with WebGPU enabled
2. The UIProcess sends an async IPC to the GPUProcess to let it know WebGPU is now enabled
3. The WebProcess starts using WebGPU and sends a WebGPU-related IPC to the GPUProcess
4. The GPUProcess receives the WebGPU-related IPC from the WebProcess
5. The GPUProcess receives the IPC from the UIProcess to let it know WebGPU is now enabled

Note that the IPC from the UIProcess (step 5) can get received too late, since there is
no synchronization. The same issue occurs if a page goes away and a setting gets turned
off as a result.

To address the issue, this patch makes it so that the GPUProcessPreferencesForWebProcess
no longer change for a particular WebProcess. This means that if a new page gets created
with different GPUProcess preferences, it will now use a separate WebProcess. Because the
GPUProcess preferences for a particular WebProcess no longer change, there is no race
anymore. When the WebProcess requests a connection to the GPUProcess, it goes via the
UIProcess (synchronously), which sends an IPC to the GPUProcess to create the connection.
The GPUProcessPreferencesForWebProcess are passed with this IPC so that the GPUProcess has
those preferences before receiving any other IPC from the WebProcess.

* Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp:
* Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h:
(WebKit::GPUConnectionToWebProcess::allowTestOnlyIPC const):
(WebKit::GPUConnectionToWebProcess::updatePreferences): Deleted.
* Source/WebKit/GPUProcess/GPUProcess.cpp:
(WebKit::GPUProcess::updatePreferencesForWebProcess): Deleted.
* Source/WebKit/GPUProcess/GPUProcess.h:
* Source/WebKit/GPUProcess/GPUProcess.messages.in:
* Source/WebKit/Shared/GPUProcessConnectionParameters.h:
* Source/WebKit/Shared/GPUProcessConnectionParameters.serialization.in:
* Source/WebKit/Shared/GPUProcessPreferencesForWebProcess.h:
* Source/WebKit/Shared/GPUProcessPreferencesForWebProcess.serialization.in:
* Source/WebKit/UIProcess/API/APIPageConfiguration.cpp:
(API::PageConfiguration::preferencesForGPUProcess const):
* Source/WebKit/UIProcess/API/APIPageConfiguration.h:
* Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:
(WebKit::GPUProcessProxy::updatePreferencesForWebProcess): Deleted.
* Source/WebKit/UIProcess/GPU/GPUProcessProxy.h:
* Source/WebKit/UIProcess/RemotePageProxy.h:
(WebKit::RemotePageProxy::page const):
* Source/WebKit/UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::findReusableSuspendedPageProcess):
* Source/WebKit/UIProcess/SuspendedPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::hasSameGPUProcessPreferencesAs const):
(WebKit::WebPageProxy::launchProcess):
(WebKit::WebPageProxy::preferencesForGPUProcess const):
(WebKit::WebPageProxy::triggerBrowsingContextGroupSwitchForNavigation):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebProcessCache.cpp:
(WebKit::WebProcessCache::takeProcess):
* Source/WebKit/UIProcess/WebProcessCache.h:
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::createGPUProcessConnection):
(WebKit::WebProcessPool::processForRegistrableDomain):
(WebKit::WebProcessPool::createWebPage):
(WebKit::WebProcessPool::processForNavigationInternal):
* Source/WebKit/UIProcess/WebProcessPool.h:
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::initializePreferencesForGPUProcess):
(WebKit::WebProcessProxy::addProvisionalPageProxy):
(WebKit::WebProcessProxy::addRemotePageProxy):
(WebKit::WebProcessProxy::addExistingWebPage):
(WebKit::WebProcessProxy::removeWebPage):
(WebKit::WebProcessProxy::createGPUProcessConnection):
(WebKit::WebProcessProxy::computePreferencesForGPUProcess const): Deleted.
(WebKit::WebProcessProxy::updatePreferencesForGPUProcess): Deleted.
* Source/WebKit/UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::preferencesForGPUProcess const):
* Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:
(WebKit::GPUProcessConnection::create):
* Source/WebKit/WebProcess/WebProcess.h:
(WebKit::WebProcess::hasWebPages const):

Canonical link: https://commits.webkit.org/269209@main




More information about the webkit-changes mailing list