Bug 228038 - REGRESSION: [iOS] ASSERTION FAILED: !m_messageReceiverMapCount under WebKit::RemoteAudioHardwareListener::~RemoteAudioHardwareListener()
Summary: REGRESSION: [iOS] ASSERTION FAILED: !m_messageReceiverMapCount under WebKit::...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
: 228111 228117 228167 228570 228667 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-07-16 15:20 PDT by Ryan Haddad
Modified: 2022-02-12 20:32 PST (History)
11 users (show)

See Also:


Attachments
Patch (1.93 KB, patch)
2021-07-26 14:24 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (2.17 KB, patch)
2021-07-26 14:33 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (1.98 KB, patch)
2021-07-26 15:20 PDT, Jer Noble
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2021-07-16 15:20:28 PDT
ASSERTION FAILED: !m_messageReceiverMapCount
/Volumes/Data/worker/ios-simulator-14-debug/build/Source/WebKit/Platform/IPC/MessageReceiver.h(41) : virtual IPC::MessageReceiver::~MessageReceiver()
1   0x1255e53d9 WTFCrash
2   0x10bf5c46b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x10bfa1478 IPC::MessageReceiver::~MessageReceiver()
4   0x10dc60e1e WebKit::RemoteAudioHardwareListener::~RemoteAudioHardwareListener()
5   0x10dc60f65 WebKit::RemoteAudioHardwareListener::~RemoteAudioHardwareListener()
6   0x10dc60fcc WebKit::RemoteAudioHardwareListener::~RemoteAudioHardwareListener()
7   0x12e47249f std::__1::default_delete<WebCore::AudioHardwareListener>::operator()(WebCore::AudioHardwareListener*) const
8   0x12e472462 WTF::RefCounted<WebCore::AudioHardwareListener, std::__1::default_delete<WebCore::AudioHardwareListener> >::deref() const
9   0x12e4723e7 WTF::DefaultRefDerefTraits<WebCore::AudioHardwareListener>::derefIfNotNull(WebCore::AudioHardwareListener*)
10  0x12fad96c4 WTF::RefPtr<WebCore::AudioHardwareListener, WTF::RawPtrTraits<WebCore::AudioHardwareListener>, WTF::DefaultRefDerefTraits<WebCore::AudioHardwareListener> >::operator=(std::nullptr_t)
11  0x12fad967e WebCore::MediaSessionManagerCocoa::removeSession(WebCore::PlatformMediaSession&)
12  0x13222c86f WebCore::PlatformMediaSession::stopSession()
13  0x131583c9c WebCore::HTMLMediaElement::stop()
14  0x1311cc7ad auto WebCore::ScriptExecutionContext::stopActiveDOMObjects()::$_3::operator()<WebCore::ActiveDOMObject>(WebCore::ActiveDOMObject&) const
15  0x1311cc743 WTF::Detail::CallableWrapper<WebCore::ScriptExecutionContext::stopActiveDOMObjects()::$_3, WebCore::ScriptExecutionContext::ShouldContinue, WebCore::ActiveDOMObject&>::call(WebCore::ActiveDOMObject&)
16  0x1311b4ffa WTF::Function<WebCore::ScriptExecutionContext::ShouldContinue (WebCore::ActiveDOMObject&)>::operator()(WebCore::ActiveDOMObject&) const
17  0x1311b4e50 WebCore::ScriptExecutionContext::forEachActiveDOMObject(WTF::Function<WebCore::ScriptExecutionContext::ShouldContinue (WebCore::ActiveDOMObject&)> const&) const
18  0x1311b5367 WebCore::ScriptExecutionContext::stopActiveDOMObjects()
19  0x130f7423f WebCore::Document::stopActiveDOMObjects()
20  0x130f73fdc WebCore::Document::commonTeardown()
21  0x130f814b4 WebCore::Document::willBeRemovedFromFrame()
22  0x131f28803 WebCore::Frame::setView(WTF::RefPtr<WebCore::FrameView, WTF::RawPtrTraits<WebCore::FrameView>, WTF::DefaultRefDerefTraits<WebCore::FrameView> >&&)
23  0x131cecace WebCore::FrameLoader::closeAndRemoveChild(WebCore::Frame&)
24  0x131cec9d3 WebCore::FrameLoader::detachFromParent()
25  0x131cdfaca WebCore::FrameLoader::detachChildren()
26  0x131cec93c WebCore::FrameLoader::detachFromParent()
27  0x131ced16d WebCore::FrameLoader::frameDetached()
28  0x13151f959 WebCore::HTMLFrameOwnerElement::disconnectContentFrame()
29  0x130f1e401 WebCore::disconnectSubframes(WebCore::ContainerNode&, WebCore::SubframeDisconnectPolicy)
30  0x130f19585 WebCore::disconnectSubframesIfNeeded(WebCore::ContainerNode&, WebCore::SubframeDisconnectPolicy)
31  0x130f15efd WebCore::ContainerNode::removeAllChildrenWithScriptAssertion(WebCore::ContainerNode::ChildChange::Source, WebCore::ContainerNode::DeferChildrenChanged)

The assert is attributed to the following tests on an iOS simulator debug test run:
fast/events/tabindex-focus-blur-all.html
media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html
media/modern-media-controls/mute-button/mute-button.html
media/non-existent-video-playback-interrupted.html

https://build.webkit.org/results/Apple-iOS-14-Simulator-Debug-WK2-Tests/r279988%20(2025)/results.html
Comment 1 Radar WebKit Bug Importer 2021-07-16 15:20:46 PDT
<rdar://problem/80705471>
Comment 2 Ryan Haddad 2021-07-16 15:22:32 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
Comment 4 Ryan Haddad 2021-07-20 13:57:28 PDT
This could be related to https://trac.webkit.org/changeset/279584/webkit
Comment 5 Alexey Proskuryakov 2021-07-20 18:05:00 PDT
*** Bug 228111 has been marked as a duplicate of this bug. ***
Comment 6 Alexey Proskuryakov 2021-07-20 18:08:28 PDT
*** Bug 228117 has been marked as a duplicate of this bug. ***
Comment 7 Ryan Haddad 2021-07-21 17:06:47 PDT
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
Comment 8 Alexey Proskuryakov 2021-07-21 18:15:26 PDT
*** Bug 228167 has been marked as a duplicate of this bug. ***
Comment 9 ayumi_kojima 2021-07-22 12:17:52 PDT
In the meantime, updated test expectations for 7 tests here https://trac.webkit.org/changeset/280192/webkit
Comment 10 Jer Noble 2021-07-22 14:34:07 PDT
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.
Comment 11 ayumi_kojima 2021-07-22 15:01:26 PDT
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>
Comment 12 ayumi_kojima 2021-07-23 15:55:19 PDT
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
Comment 13 Jer Noble 2021-07-26 14:24:52 PDT
Created attachment 434240 [details]
Patch
Comment 14 Jer Noble 2021-07-26 14:33:39 PDT
Created attachment 434242 [details]
Patch
Comment 15 Chris Dumez 2021-07-26 15:01:54 PDT
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 16 Jer Noble 2021-07-26 15:19:27 PDT
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).
Comment 17 Jer Noble 2021-07-26 15:20:47 PDT
Created attachment 434248 [details]
Patch
Comment 18 EWS 2021-07-26 17:06:59 PDT
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].
Comment 19 ayumi_kojima 2021-07-28 17:11:17 PDT
*** Bug 228570 has been marked as a duplicate of this bug. ***
Comment 20 ayumi_kojima 2021-10-21 14:43:58 PDT
Removed test expectations https://trac.webkit.org/changeset/284647/webkit
Comment 21 Brent Fulgham 2022-02-12 20:32:28 PST
*** Bug 228667 has been marked as a duplicate of this bug. ***