Bug 219321 - GPU process remains alive even after UI process exited
Summary: GPU process remains alive even after UI process exited
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-27 13:09 PST by Fujii Hironori
Modified: 2020-11-30 19:23 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.01 KB, patch)
2020-11-27 13:17 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2020-11-27 13:09:33 PST
[WinCairo?] GPU process remains alive even after web process exited

GPUProcess overrides AuxiliaryProcess::didClose but it does nothing.
Is this a WinCairo specific issue?
Comment 1 Fujii Hironori 2020-11-27 13:17:06 PST
Created attachment 414969 [details]
Patch
Comment 2 Fujii Hironori 2020-11-27 20:29:43 PST
Committed r270213: <https://trac.webkit.org/changeset/270213>
Comment 3 Radar WebKit Bug Importer 2020-11-27 20:30:17 PST
<rdar://problem/71770347>
Comment 4 Fujii Hironori 2020-11-27 20:30:40 PST
Landed in r270210, and reverted in r270213.
It broke TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo on iOS.
Comment 5 Fujii Hironori 2020-11-27 20:31:17 PST
Reopened.
Comment 6 Wenson Hsieh 2020-11-27 20:34:49 PST
Hmm...would this mean that if there are two web content processes (and therefore two connections) and one of the web processes die, then the GPU process will also terminate? If so, I think we'd still need the GPU process to stay alive in this scenario, since it's still required to service the other web content process that is still alive.
Comment 7 Tim Horton 2020-11-28 17:24:45 PST
(In reply to Wenson Hsieh from comment #6)
> Hmm...would this mean that if there are two web content processes (and
> therefore two connections) and one of the web processes die, then the GPU
> process will also terminate? If so, I think we'd still need the GPU process
> to stay alive in this scenario, since it's still required to service the
> other web content process that is still alive.

No, the connection this is referring to /should/ be the UI->GPU connection, right?
Comment 8 Wenson Hsieh 2020-11-29 11:17:28 PST
(In reply to Tim Horton from comment #7)
> (In reply to Wenson Hsieh from comment #6)
> > Hmm...would this mean that if there are two web content processes (and
> > therefore two connections) and one of the web processes die, then the GPU
> > process will also terminate? If so, I think we'd still need the GPU process
> > to stay alive in this scenario, since it's still required to service the
> > other web content process that is still alive.
> 
> No, the connection this is referring to /should/ be the UI->GPU connection,
> right?

Ah, if that's the case then this makes a lot more sense. I was unclear on which process the `IPC::Connection` was referring to.

I suppose this doesn't affect Cocoa platforms because the GPU process is a child process of the UI (application) process, so if the app dies then the GPU process goes along with it.
Comment 9 Fujii Hironori 2020-11-30 13:35:36 PST
Thank you for clarifying. I didn't understand GPU process model.

I confirmed GPUConnectionToWebProcess::didClose is called when Web process exits, and GPUProcess::didClose is called when UI process exits on WinCairo.

Why does TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo fail on iOS with my patch?
Comment 10 Tim Horton 2020-11-30 13:57:19 PST
(In reply to Fujii Hironori from comment #9)
> Thank you for clarifying. I didn't understand GPU process model.
> 
> I confirmed GPUConnectionToWebProcess::didClose is called when Web process
> exits, and GPUProcess::didClose is called when UI process exits on WinCairo.
> 
> Why does TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo fail on iOS with my
> patch?

Do you have a link to the test output? Also, +cdumez
Comment 11 Fujii Hironori 2020-11-30 14:09:56 PST
(In reply to Tim Horton from comment #10)
> Do you have a link to the test output? Also, +cdumez

I'm sorry. It did misunderstand. TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo is flaky without my patch.

https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo&platform=ios

I'm going to land it again.
Comment 12 Chris Dumez 2020-11-30 14:23:53 PST
(In reply to Fujii Hironori from comment #11)
> (In reply to Tim Horton from comment #10)
> > Do you have a link to the test output? Also, +cdumez
> 
> I'm sorry. It did misunderstand.
> TestWebKitAPI.GPUProcess.CrashWhilePlayingVideo is flaky without my patch.
> 
> https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.GPUProcess.
> CrashWhilePlayingVideo&platform=ios
> 
> I'm going to land it again.

Yes, I noticed GPUProcess.CrashWhilePlayingVideo is flaky on the iOS bots too. I have not been able to reproduce this flakiness though.
Comment 13 Fujii Hironori 2020-11-30 18:10:15 PST
Committed r270279: <https://trac.webkit.org/changeset/270279>
Comment 14 Darin Adler 2020-11-30 18:28:04 PST
For future reference: A bug like this should *not* have the [WinCairo?] prefix.

Platform-independent fix to cross-platform issue!
Comment 15 Fujii Hironori 2020-11-30 19:23:15 PST
I got it.