| Summary: | REGRESSION: [iOS] ASSERTION FAILED: !m_messageReceiverMapCount under WebKit::RemoteAudioHardwareListener::~RemoteAudioHardwareListener() | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||
| Component: | Media | Assignee: | Jer Noble <jer.noble> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ayumi_kojima, cdumez, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | Other | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=226248 https://bugs.webkit.org/show_bug.cgi?id=228659 |
||||||||||
| Attachments: |
|
||||||||||
|
Description
Ryan Haddad
2021-07-16 15:20:28 PDT
Looks like this is a flaky crash that happens with various tests: https://build.webkit.org/results/Apple-iOS-14-Simulator-Debug-WK2-Tests/r279992%20(2026)/results.html fast/mediastream/audio-bad-sampleRate.html media/media-can-play-av1.html media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html media/modern-media-controls/mute-button/mute-button.html The same crash is seen with fast/mediastream/apply-constraints-video.html History: https://results.webkit.org/?suite=layout-tests&test=fast%2Fmediastream%2Fapply-constraints-video.html https://build.webkit.org/results/Apple-iPadOS-14-Simulator-Debug-WK2-Tests/r279869%20(1386)/fast/mediastream/apply-constraints-video-crash-log.txt This could be related to https://trac.webkit.org/changeset/279584/webkit *** Bug 228111 has been marked as a duplicate of this bug. *** *** Bug 228117 has been marked as a duplicate of this bug. *** From one of the dupes, this apparently reproduced the crash: run-webkit-tests --iterations 50 --exit-after-n-failures 2 --exit-after-n-crashes-or-timeouts 2 fast/mediastream/apply-constraints-video.html --debug -f --iphone-simulator *** Bug 228167 has been marked as a duplicate of this bug. *** In the meantime, updated test expectations for 7 tests here https://trac.webkit.org/changeset/280192/webkit RemoteAudioHardwareListener holds a WeakRef to the holder of its MessageReceiverMap, GPUProcessConnection. If GPUProcessConnection is destroyed before RemoteAudioHardwareListener is, then the listener will never get a chance to remove itself from the GPUProcessConnection map, and will hit this assertion. Tried to reproduce the crash again on fast/mediastream/apply-constraints-video.html, but I was not able to reproduce the crash on arm64 remote machine with iOS 14 simulator using: run-webkit-tests --iterations 50 --exit-after-n-failures 2 --exit-after-n-crashes-or-timeouts 2 fast/mediastream/apply-constraints-video.html --debug -f --iphone-simulator run-webkit-tests --no-build --clobber-old-results --exit-after-n-failures 500 --exit-after-n-crashes-or-timeouts 500 --debug -f --force --iphone-simulator --child-processes 1 --test-list <testlist> media/media-blocked-by-willsendrequest.html Also shows the same crash (ASSERTION FAILED: !m_messageReceiverMapCount) History: https://results.webkit.org/?suite=layout-tests&test=media%2Fmedia-blocked-by-willsendrequest.html Updated test expectations: https://trac.webkit.org/changeset/280266/webkit Created attachment 434240 [details]
Patch
Created attachment 434242 [details]
Patch
Comment on attachment 434240 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434240&action=review > Source/WebKit/WebProcess/GPU/media/RemoteAudioHardwareListener.cpp:69 > + m_gpuProcessConnection->messageReceiverMap().removeMessageReceiver(*this); Don't we want to null out m_gpuProcessConnection too to make sure that the RemoteAudioHardwareListener destructor doesn't attempt to do the same thing if the GPUProcessConnection is still alive? I think it would likely hit assertions if we called messageReceiverMap().removeMessageReceiver(*this) unnecessarily. Comment on attachment 434240 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434240&action=review >> Source/WebKit/WebProcess/GPU/media/RemoteAudioHardwareListener.cpp:69 >> + m_gpuProcessConnection->messageReceiverMap().removeMessageReceiver(*this); > > Don't we want to null out m_gpuProcessConnection too to make sure that the RemoteAudioHardwareListener destructor doesn't attempt to do the same thing if the GPUProcessConnection is still alive? I think it would likely hit assertions if we called messageReceiverMap().removeMessageReceiver(*this) unnecessarily. Sure, this would also protect against this close method from being called twice (accidentally). Created attachment 434248 [details]
Patch
Committed r280328 (239975@main): <https://commits.webkit.org/239975@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 434248 [details]. *** Bug 228570 has been marked as a duplicate of this bug. *** Removed test expectations https://trac.webkit.org/changeset/284647/webkit *** Bug 228667 has been marked as a duplicate of this bug. *** |