Skip to content

Commit

Permalink
GPU process is terminated during debugging
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=277731
rdar://133372529

Reviewed by Mike Wyrzykowski.

GPU process would not be debuggable due to WCP timing out a send or wait
to GPUP. This would cause following distinct problems
- WCP would ask UI to terminate GPUP
- Even if GPUP was not terminated, the WCP would already be in undefined
  state due to timing out a send or a wait.

Add a setting ChildProcessDebuggabilityEnabled which should be set when
the developer is intending to stop the GPU process in debugger.

The added implementation will set stream IPC connection timeout to
infinity. Later commits will modify other, less frequent normal IPC
sendSync and wait timeouts.

Run with:
  lldb -w -n GPU.Development -o c
  run-webkit-tests --internal-feature=ChildProcessDebuggabilityEnabled --no-timeout test.html

  run-minibrowser
  Set "Internal -> Child Process Debuggability" and restart

* Source/WTF/Scripts/GeneratePreferences.rb:
* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.cpp:
(WebKit::RemoteGraphicsContextGLProxy::create):
* Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
(WebKit::RemoteRenderingBackendProxy::ensureGPUProcessConnection):
* Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteGPUProxy.cpp:
(WebKit::RemoteGPUProxy::create):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::updatePreferences):
* Source/WebKit/WebProcess/WebProcess.cpp:
(WebKit::WebProcess::gpuProcessTimeoutDuration const):
(WebKit::WebProcess::setGPUProcessDebuggabilityEnabled):
* Source/WebKit/WebProcess/WebProcess.h:

Canonical link: https://commits.webkit.org/282280@main
  • Loading branch information
kkinnunen-apple committed Aug 15, 2024
1 parent 0e9bbd9 commit f6f4d72
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 7 deletions.
7 changes: 6 additions & 1 deletion Introduction.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ When debugging a debug build in LLDB, there are also a few functions that can be
* showTreeForThis()
* showNodePathForThis()

Debugging child processes such as GPU process, "Child Process Debuggability" internal feature must be in use. For the test runner, you can
specify `run-webkit-tests --internal-feature=ChildProcessDebuggabilityEnabled`. For MiniBrowser, set the Debug > Internal > Child Process Debuggability
menu item and restart.

## Correctness Testing in WebKit

WebKit is really big on test driven development, we have many types of tests.
Expand Down Expand Up @@ -1759,7 +1763,8 @@ You may want to specify OS_ACTIVITY_MODE environmental variable to “disable”
in order to suppress all the system logging that happens during the debugging session.

You may also want to specify `--no-timeout` option to prevent WebKitTestRunner or DumpRenderTree
to stop the test after 30 seconds if you’re stepping through code.
to stop the test after 30 seconds if you’re stepping through code. Specify additional
`--internal-feature=ChildProcessDebuggabilityEnabled` when stepping through child process such as GPU process.

Once this is done, you can run WebKitTestRunner or DumpRenderTree by going to Product > Perform Action > Run without Building.

Expand Down
15 changes: 15 additions & 0 deletions Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1707,6 +1707,21 @@ CaretBrowsingEnabled:
WebCore:
default: false

ChildProcessDebuggabilityEnabled:
comment: Disables GPU process IPC timeouts
type: bool
status: internal
humanReadableName: "Child Process Debuggability"
humanReadableDescription: "Enable stopping child processes with a debugger"
exposed: [ WebKit ]
defaultValue:
WebKitLegacy:
default: false
WebKit:
default: false
WebCore:
default: false

ClearSiteDataHTTPHeaderEnabled:
type: bool
status: stable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,9 @@ RefPtr<RemoteGraphicsContextGLProxy> RemoteGraphicsContextGLProxy::create(const
{
constexpr unsigned defaultConnectionBufferSizeLog2 = 21;
unsigned connectionBufferSizeLog2 = defaultConnectionBufferSizeLog2;
constexpr Seconds defaultTimeoutDuration = 30_s;
if (attributes.failContextCreationForTesting == WebCore::GraphicsContextGLAttributes::SimulatedCreationFailure::IPCBufferOOM)
connectionBufferSizeLog2 = 50; // Expect this to fail.
auto connectionPair = IPC::StreamClientConnection::create(connectionBufferSizeLog2, defaultTimeoutDuration);
auto connectionPair = IPC::StreamClientConnection::create(connectionBufferSizeLog2, WebProcess::singleton().gpuProcessTimeoutDuration());
if (!connectionPair)
return nullptr;
auto [clientConnection, serverConnectionHandle] = WTFMove(*connectionPair);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ void RemoteRenderingBackendProxy::ensureGPUProcessConnection()
{
if (m_connection)
return;
static constexpr auto connectionBufferSizeLog2 = 21;
auto connectionPair = IPC::StreamClientConnection::create(connectionBufferSizeLog2, defaultTimeout);
constexpr unsigned connectionBufferSizeLog2 = 21u;
auto connectionPair = IPC::StreamClientConnection::create(connectionBufferSizeLog2, WebProcess::singleton().gpuProcessTimeoutDuration());
if (!connectionPair)
CRASH();
auto [streamConnection, serverHandle] = WTFMove(*connectionPair);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ RefPtr<RemoteGPUProxy> RemoteGPUProxy::create(WebGPU::ConvertToBackingContext& c
RefPtr<RemoteGPUProxy> RemoteGPUProxy::create(WebGPU::ConvertToBackingContext& convertToBackingContext, RemoteRenderingBackendProxy& renderingBackend, SerialFunctionDispatcher& dispatcher)
{
constexpr size_t connectionBufferSizeLog2 = 21;
constexpr Seconds defaultTimeoutDuration = 30_s;
auto connectionPair = IPC::StreamClientConnection::create(connectionBufferSizeLog2, defaultTimeoutDuration);
auto connectionPair = IPC::StreamClientConnection::create(connectionBufferSizeLog2, WebProcess::singleton().gpuProcessTimeoutDuration());
if (!connectionPair)
return nullptr;
auto [clientConnection, serverConnectionHandle] = WTFMove(*connectionPair);
Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4701,6 +4701,8 @@ void WebPage::updatePreferences(const WebPreferencesStore& store)
if (m_drawingArea)
m_drawingArea->updatePreferences(store);

WebProcess::singleton().setChildProcessDebuggabilityEnabled(store.getBoolValueForKey(WebPreferencesKey::childProcessDebuggabilityEnabledKey()));

#if ENABLE(GPU_PROCESS)
static_cast<WebMediaStrategy&>(platformStrategies()->mediaStrategy()).setUseGPUProcess(m_shouldPlayMediaInGPUProcess);
#if ENABLE(VIDEO)
Expand Down
11 changes: 11 additions & 0 deletions Source/WebKit/WebProcess/WebProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1377,6 +1377,12 @@ GPUProcessConnection& WebProcess::ensureGPUProcessConnection()
return *m_gpuProcessConnection;
}

Seconds WebProcess::gpuProcessTimeoutDuration() const
{
constexpr Seconds defaultTimeoutDuration = 15_s;
return m_childProcessDebuggabilityEnabled ? Seconds::infinity() : defaultTimeoutDuration;
}

void WebProcess::gpuProcessConnectionClosed()
{
ASSERT(m_gpuProcessConnection);
Expand Down Expand Up @@ -2212,6 +2218,11 @@ void WebProcess::updateDomainsWithStorageAccessQuirks(HashSet<WebCore::Registrab
m_domainsWithStorageAccessQuirks.add(domain);
}

void WebProcess::setChildProcessDebuggabilityEnabled(bool childProcessDebuggabilityEnabled)
{
m_childProcessDebuggabilityEnabled = childProcessDebuggabilityEnabled;
}

#if ENABLE(GPU_PROCESS)
void WebProcess::setUseGPUProcessForCanvasRendering(bool useGPUProcessForCanvasRendering)
{
Expand Down
6 changes: 6 additions & 0 deletions Source/WebKit/WebProcess/WebProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ class WebProcess : public AuxiliaryProcess
#if ENABLE(GPU_PROCESS)
GPUProcessConnection& ensureGPUProcessConnection();
GPUProcessConnection* existingGPUProcessConnection() { return m_gpuProcessConnection.get(); }
// Returns timeout duration for GPU process connections. Thread-safe.
Seconds gpuProcessTimeoutDuration() const;
void gpuProcessConnectionClosed();
void gpuProcessConnectionDidBecomeUnresponsive();

Expand Down Expand Up @@ -383,6 +385,8 @@ class WebProcess : public AuxiliaryProcess
void updatePageScreenProperties();
#endif

void setChildProcessDebuggabilityEnabled(bool);

#if ENABLE(GPU_PROCESS)
void setUseGPUProcessForCanvasRendering(bool);
void setUseGPUProcessForDOMRendering(bool);
Expand Down Expand Up @@ -805,6 +809,8 @@ class WebProcess : public AuxiliaryProcess
RetainPtr<NSMutableDictionary> m_accessibilityRemoteFrameTokenCache;
#endif

bool m_childProcessDebuggabilityEnabled { false };

#if ENABLE(GPU_PROCESS)
bool m_useGPUProcessForCanvasRendering { false };
bool m_useGPUProcessForDOMRendering { false };
Expand Down

0 comments on commit f6f4d72

Please sign in to comment.