Bug 218811 - Terminate WebProcesses if GPUProcess crashes more than twice in 30 seconds
Summary: Terminate WebProcesses if GPUProcess crashes more than twice in 30 seconds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 218806
Blocks:
  Show dependency treegraph
 
Reported: 2020-11-11 10:53 PST by Chris Dumez
Modified: 2020-12-09 16:31 PST (History)
6 users (show)

See Also:


Attachments
Patch (14.29 KB, patch)
2020-11-11 12:49 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (14.27 KB, patch)
2020-11-11 13:31 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2020-11-11 10:53:22 PST
Terminate WebProcesses if GPUProcess crashes more than twice in 30 seconds.
Comment 1 Radar WebKit Bug Importer 2020-11-11 11:46:22 PST
<rdar://problem/71292424>
Comment 2 Chris Dumez 2020-11-11 12:49:05 PST
Created attachment 413856 [details]
Patch
Comment 3 Simon Fraser (smfr) 2020-11-11 13:29:21 PST
Comment on attachment 413856 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413856&action=review

> Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:111
> +    RELEASE_LOG_ERROR(Process, "AuxiliaryProcessProxy::terminate");

Can this log the pid of the thing being terminated?
Comment 4 Chris Dumez 2020-11-11 13:30:11 PST
(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 413856 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=413856&action=review
> 
> > Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:111
> > +    RELEASE_LOG_ERROR(Process, "AuxiliaryProcessProxy::terminate");
> 
> Can this log the pid of the thing being terminated?

good idea. Will fix.
Comment 5 Chris Dumez 2020-11-11 13:31:40 PST
Created attachment 413858 [details]
Patch
Comment 6 Geoffrey Garen 2020-11-11 14:06:55 PST
Comment on attachment 413858 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413858&action=review

> Source/WebKit/UIProcess/WebProcessPool.cpp:270
> +#if ENABLE(GPU_PROCESS)
> +    , m_resetGPUProcessCrashCountTimer(RunLoop::main(), this, &WebProcessPool::resetGPUProcessCrashCount)
> +#endif

DeferrableTaskTimer is slightly nicer for this kind of thing because it doesn't require an initializer.

> Source/WebKit/UIProcess/WebProcessPool.cpp:519
> +void WebProcessPool::resetGPUProcessCrashCount()
> +{
> +    m_recentGPUProcessCrashCount = 0;
>  }

DeferrableTaskTimer is slightly nicer for this kind of thing because it doesn't require a separate function.
Comment 7 EWS 2020-11-11 14:49:54 PST
Committed r269703: <https://trac.webkit.org/changeset/269703>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413858 [details].
Comment 8 Chris Dumez 2020-11-11 16:32:08 PST
(In reply to Geoffrey Garen from comment #6)
> Comment on attachment 413858 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=413858&action=review
> 
> > Source/WebKit/UIProcess/WebProcessPool.cpp:270
> > +#if ENABLE(GPU_PROCESS)
> > +    , m_resetGPUProcessCrashCountTimer(RunLoop::main(), this, &WebProcessPool::resetGPUProcessCrashCount)
> > +#endif
> 
> DeferrableTaskTimer is slightly nicer for this kind of thing because it
> doesn't require an initializer.
> 
> > Source/WebKit/UIProcess/WebProcessPool.cpp:519
> > +void WebProcessPool::resetGPUProcessCrashCount()
> > +{
> > +    m_recentGPUProcessCrashCount = 0;
> >  }
> 
> DeferrableTaskTimer is slightly nicer for this kind of thing because it
> doesn't require a separate function.

Following up in https://bugs.webkit.org/show_bug.cgi?id=218828.