Bug 231897

Summary: GPU process occasionally crashes under `StreamConnectionWorkQueue::processStreams()` in layout tests
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit2Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, kkinnunen, simon.fraser, thorton, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=231939
https://bugs.webkit.org/show_bug.cgi?id=231940
Attachments:
Description Flags
Crash log
none
Fixes the tests none

Wenson Hsieh
Reported 2021-10-18 08:58:48 PDT
Seems to happen somewhat randomly on EWS.
Attachments
Crash log (7.57 KB, text/plain)
2021-10-18 09:00 PDT, Wenson Hsieh 		no flags
Details
Fixes the tests (7.96 KB, patch)
2021-10-18 15:51 PDT, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Wenson Hsieh
Comment 1 2021-10-18 09:00:36 PDT
Created attachment 441611 [details] Crash log
Wenson Hsieh
Comment 2 2021-10-18 11:15:48 PDT
I can intermittently reproduce by running the tests in `compositing` on 4 iOS 15 simulators in parallel for ~3-4 iterations.
Wenson Hsieh
Comment 3 2021-10-18 15:51:42 PDT
Created attachment 441650 [details] Fixes the tests
Wenson Hsieh
Comment 4 2021-10-18 19:40:49 PDT
Comment on attachment 441650 [details] Fixes the tests Thanks for the review!
EWS
Comment 5 2021-10-18 19:44:55 PDT
Committed r284427 (243191@main): <https://commits.webkit.org/243191@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441650 [details].
Radar WebKit Bug Importer
Comment 6 2021-10-18 19:45:20 PDT
<rdar://problem/84398678>
Kimmo Kinnunen
Comment 7 2021-10-19 00:22:24 PDT
Comment on attachment 441650 [details] Fixes the tests View in context: https://bugs.webkit.org/attachment.cgi?id=441650&action=review > Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:102 > + m_workQueue->stop(); Is there a reason to do the ref-related changes? Wasn't the stop the only thing that was missing? I don't think the ref made above lives longer than the stop.
Wenson Hsieh
Comment 8 2021-10-19 16:03:01 PDT
(In reply to Kimmo Kinnunen from comment #7) > Comment on attachment 441650 [details] > Fixes the tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=441650&action=review > > > Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:102 > > + m_workQueue->stop(); > > Is there a reason to do the ref-related changes? > Wasn't the stop the only thing that was missing? > I don't think the ref made above lives longer than the stop. It's true that this call to `stop()` was technically the only change required to avoid the crash. However, given that StreamConnectionWorkQueue is ref-counted, it was also wrong to be storing `m_workQueue` directly as a member on RemoteRenderingBackend, rather than through some kind of Ref-counted template class (i.e. Ref or RefPtr). This would be important if (for example) we were to make `stop()` non-blocking in the future.
Note You need to log in before you can comment on or make changes to this bug.