Bug 226896 – Some http/tests/ are crashing with ASSERTION FAILED: isInRoutingArbitrationForToken(token) || m_setupArbitrationOngoing

• Component: New Bugs
• Status: RESOLVED FIXED

[Truitt Savell]

I have found 2 tests so far crashing with this assert, will add more as I find them:
http/tests/media/media-source/mediasource-play-then-seek-back-with-remote-control.html
http/tests/canvas/webgl/origin-clean-conformance.html

History:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=http%2Ftests%2Fmedia%2Fmedia-source%2Fmediasource-play-then-seek-back-with-remote-control.html&test=http%2Ftests%2Fcanvas%2Fwebgl%2Forigin-clean-conformance.html

log:
https://build.webkit.org/results/Apple-BigSur-Debug-WK2-Tests/r278699%20(2294)/http/tests/canvas/webgl/origin-clean-conformance-stderr.txt

[Radar WebKit Bug Importer]

<rdar://problem/79156052>

[Ryan Haddad]

Maybe this started with https://trac.webkit.org/changeset/278374/webkit?

[Ryan Haddad]

ASSERTION FAILED: isInRoutingArbitrationForToken(token) || m_setupArbitrationOngoing
./platform/audio/mac/SharedRoutingArbitrator.mm(123) : void WebCore::SharedRoutingArbitrator::endRoutingArbitrationForToken(const WebCore::SharedRoutingArbitrator::Token &)
1   0x113791579 WTFCrash
2   0x12d45b1fb WTFCrashWithInfo(int, char const*, char const*, int)
3   0x12f076cb1 WebCore::SharedRoutingArbitrator::endRoutingArbitrationForToken(WebCore::SharedRoutingArbitrator::Token const&)
4   0x11d1d329c WebKit::AudioSessionRoutingArbitratorProxy::endRoutingArbitration()
5   0x11bc44234 void IPC::callMemberFunctionImpl<WebKit::AudioSessionRoutingArbitratorProxy, void (WebKit::AudioSessionRoutingArbitratorProxy::*)(), std::__1::tuple<> >(WebKit::AudioSessionRoutingArbitratorProxy*, void (WebKit::AudioSessionRoutingArbitratorProxy::*)(), std::__1::tuple<>&&, std::__1::integer_sequence<unsigned long>)
6   0x11bc43e40 void IPC::callMemberFunction<WebKit::AudioSessionRoutingArbitratorProxy, void (WebKit::AudioSessionRoutingArbitratorProxy::*)(), std::__1::tuple<>, std::__1::integer_sequence<unsigned long> >(std::__1::tuple<>&&, WebKit::AudioSessionRoutingArbitratorProxy*, void (WebKit::AudioSessionRoutingArbitratorProxy::*)())
7   0x11bc3f808 void IPC::handleMessage<Messages::AudioSessionRoutingArbitratorProxy::EndRoutingArbitration, WebKit::AudioSessionRoutingArbitratorProxy, void (WebKit::AudioSessionRoutingArbitratorProxy::*)()>(IPC::Decoder&, WebKit::AudioSessionRoutingArbitratorProxy*, void (WebKit::AudioSessionRoutingArbitratorProxy::*)())
8   0x11bc3f573 WebKit::AudioSessionRoutingArbitratorProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
9   0x11c19f4e5 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
10  0x11d0fafde WebKit::AuxiliaryProcessProxy::dispatchMessage(IPC::Connection&, IPC::Decoder&)
11  0x11d3d6ecf WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)

[Alexey Proskuryakov]

Is this different from bug 226787?

[Peng Liu]

(In reply to Alexey Proskuryakov from comment #4)
> Is this different from bug 226787?

I think so.

[Peng Liu]

Created attachment 431502 [details]
Patch

[EWS]

Committed r278922 (238853@main): <https://commits.webkit.org/238853@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431502 [details].

[Darin Adler]

Comment on attachment 431502 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=431502&action=review

> Source/WebCore/platform/audio/mac/SharedRoutingArbitrator.mm:124
> +    if (m_tokens.contains(token))
> +        m_tokens.remove(token);

Why check contains before calling remove? This adds an additional hash table lookup and has no other effect other than making the code run more slowly.

The point of this patch was to remove an incorrect assertion. Why change the code outside the assertion at all?

[Peng Liu]

Comment on attachment 431502 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=431502&action=review

>> Source/WebCore/platform/audio/mac/SharedRoutingArbitrator.mm:124
>> +        m_tokens.remove(token);
> 
> Why check contains before calling remove? This adds an additional hash table lookup and has no other effect other than making the code run more slowly.
> 
> The point of this patch was to remove an incorrect assertion. Why change the code outside the assertion at all?

Oops. I thought the remove() function assumes the item must exist in the hash set. So I added the check. Will remove it.

[Peng Liu]

Reopen this bug to land a follow-up fix.

[Peng Liu]

Created attachment 431573 [details]
A follow-up patch

[EWS]

Committed r278958 (238886@main): <https://commits.webkit.org/238886@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431573 [details].