Summary: | [GPUProcess] Crash under RemoteAudioDestination::render() | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | Web Audio | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, peng.liu6, philipj, sergio, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Chris Dumez
2021-04-16 13:00:13 PDT
Created attachment 426263 [details]
Patch
Comment on attachment 426263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426263&action=review > Source/WebKit/ChangeLog:10 > + GPUConnectionToWebProcess::didClose() would get called and en up destroying s/en up/end up/g > Source/WebKit/ChangeLog:43 > + was not thread safe. Even if m_isPlaying was atomic, m_isPlaying get set to true I feel “m_isPlaying" here is the root cause of this crash. But I like your refactoring. Created attachment 426268 [details]
Patch
(In reply to Peng Liu from comment #3) > Comment on attachment 426263 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426263&action=review > > > Source/WebKit/ChangeLog:10 > > + GPUConnectionToWebProcess::didClose() would get called and en up destroying > > s/en up/end up/g > > > Source/WebKit/ChangeLog:43 > > + was not thread safe. Even if m_isPlaying was atomic, m_isPlaying get set to true > > I feel “m_isPlaying" here is the root cause of this crash. But I like your > refactoring. The root cause of this crash is that we weren't calling m_audioOutputUnitAdaptor.stop() in the destructor (or before destroying). There was no way to make this safe using m_isPlaying. Comment on attachment 426263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426263&action=review >>> Source/WebKit/ChangeLog:10 >>> + GPUConnectionToWebProcess::didClose() would get called and en up destroying >> >> s/en up/end up/g > > The root cause of this crash is that we weren't calling m_audioOutputUnitAdaptor.stop() in the destructor (or before destroying). There was no way to make this safe using m_isPlaying. Hmm, just curious, if we call `m_audioOutputUnitAdaptor.stop()` before destroying and make `m_isPlaying` atomic, will the crash be fixed? (In reply to Peng Liu from comment #6) > Comment on attachment 426263 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426263&action=review > > >>> Source/WebKit/ChangeLog:10 > >>> + GPUConnectionToWebProcess::didClose() would get called and en up destroying > >> > >> s/en up/end up/g > > > > The root cause of this crash is that we weren't calling m_audioOutputUnitAdaptor.stop() in the destructor (or before destroying). There was no way to make this safe using m_isPlaying. > > Hmm, just curious, if we call `m_audioOutputUnitAdaptor.stop()` before > destroying and make `m_isPlaying` atomic, will the crash be fixed? I you call m_audioOutputUnitAdaptor.stop() before destroying, the crash would be fixed. You would not need to make m_isPlaying atomic to address the crash. Comment on attachment 426268 [details]
Patch
Will add a test.
Created attachment 426284 [details]
Patch
Committed r276188 (236670@main): <https://commits.webkit.org/236670@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426284 [details]. |