Bug 197917 – REGRESSION (r245043) [Mac WK2 Debug] ASSERTION FAILED: m_services.isEmpty() && transports.size() <= maxTransportNumber seen with two http/wpt/webauthn/public-key-credential-* tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Shawn Roberts]

The following layout tests are flaky on Mac WK2 Debug

http/wpt/webauthn/public-key-credential-create-success-hid.https.html
http/wpt/webauthn/public-key-credential-get-success-hid.https.html

Probable cause:

After changes in https://trac.webkit.org/changeset/245043/webkit the above layout tests went from flaky failures in Mac WK2 Debug runs to crashes. 

Tests were marked as flaky in previous bugs 196377, 194780 for flaky failures in Mac WK2 Release and Debug builds. Now tests will crash instead of fail on Debug. 

Reproduced with:

run-webkit-tests http/wpt/webauthn/public-key-credential-get-success-hid.https.html --iter 100 -f --debug --exit-after-n-crashes=1

Will crash with builds including 245043. Previous builds will only flakily fail with 1500 iterations. 

Crash is the same for both tests

Flakiness Dashboard:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Fwpt%2Fwebauthn%2Fpublic-key-credential-create-success-hid.https.html%20http%2Fwpt%2Fwebauthn%2Fpublic-key-credential-get-success-hid.https.html

Crash Log: https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r245275%20(2606)/http/wpt/webauthn/public-key-credential-create-success-hid.https-crash-log.txt

0   com.apple.JavaScriptCore      	0x0000000104e80ae0 WTFCrash + 16 (Assertions.cpp:305)
1   com.apple.WebKit              	0x00000001116d4f0b WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebKit              	0x000000011255822b WebKit::AuthenticatorManager::startDiscovery(WTF::HashSet<WebCore::AuthenticatorTransport, WTF::IntHash<WebCore::AuthenticatorTransport>, WTF::StrongEnumHashTraits<WebCore::AuthenticatorTransport> > const&) + 171 (AuthenticatorManager.cpp:224)
3   com.apple.WebKit              	0x0000000112557a99 WebKit::AuthenticatorManager::makeCredential(WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&, WTF::CompletionHandler<void (WTF::Variant<WebCore::PublicKeyCredentialData, WebCore::ExceptionData>&&)>&&) + 649 (AuthenticatorManager.cpp:143)
4   com.apple.WebKit              	0x000000011255a38e WebKit::WebAuthenticatorCoordinatorProxy::makeCredential(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&) + 142 (WebAuthenticatorCoordinatorProxy.cpp:69)
5   com.apple.WebKit              	0x000000011296e98f void IPC::callMemberFunctionImpl<WebKit::WebAuthenticatorCoordinatorProxy, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&), std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialCreationOptions>, 0ul, 1ul, 2ul>(WebKit::WebAuthenticatorCoordinatorProxy*, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&), std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialCreationOptions>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul>) + 207 (HandleMessage.h:42)
6   com.apple.WebKit              	0x00000001129669c0 void IPC::callMemberFunction<WebKit::WebAuthenticatorCoordinatorProxy, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&), std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialCreationOptions>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul> >(std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialCreationOptions>&&, WebKit::WebAuthenticatorCoordinatorProxy*, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&)) + 112 (HandleMessage.h:48)
7   com.apple.WebKit              	0x0000000112966580 void IPC::handleMessage<Messages::WebAuthenticatorCoordinatorProxy::MakeCredential, WebKit::WebAuthenticatorCoordinatorProxy, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&)>(IPC::Decoder&, WebKit::WebAuthenticatorCoordinatorProxy*, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialCreationOptions const&)) + 336 (HandleMessage.h:121)
8   com.apple.WebKit              	0x00000001129662e5 WebKit::WebAuthenticatorCoordinatorProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 133 (WebAuthenticatorCoordinatorProxyMessageReceiver.cpp:46)

[Radar WebKit Bug Importer]

<rdar://problem/50815987>

[Ryan Haddad]

ASSERTION FAILED: m_services.isEmpty() && transports.size() <= maxTransportNumber
/Volumes/Data/slave/highsierra-debug/build/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp(224) : void WebKit::AuthenticatorManager::startDiscovery(const WebKit::AuthenticatorManager::TransportSet &)

[Shawn Roberts]

Skipped tests on Debug in https://trac.webkit.org/changeset/245340/webkit

Expectations for Release WK2 are still set to Pass / Fail

[Brent Fulgham]

Created attachment 370865 [details]
Patch

[Brent Fulgham]

The changes in Bug 198308 resolved the underlying issue that triggered this failure. Local testing confirms debug runs do not assert on these tests.

[Brent Fulgham]

You can also hit this in GetAssertion:

    #0 0x107327b2d in WTFCrash Assertions.cpp:301
    #1 0x10cd9c01a in WTFCrashWithInfo(int, char const*, char const*, int) Assertions.h:568
    #2 0x10dc559f8 in WebKit::AuthenticatorManager::getAssertion(WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&, WTF::CompletionHandler<void (WTF::Variant<WebCore::PublicKeyCredentialData, WebCore::ExceptionData>&&)>&&) AuthenticatorManager.cpp:162
    #3 0x10dc5756d in WebKit::WebAuthenticatorCoordinatorProxy::getAssertion(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&) WebAuthenticatorCoordinatorProxy.cpp:85
    #4 0x10e01d17e in void IPC::callMemberFunctionImpl<WebKit::WebAuthenticatorCoordinatorProxy, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&), std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialRequestOptions>, 0ul, 1ul, 2ul>(WebKit::WebAuthenticatorCoordinatorProxy*, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&), std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialRequestOptions>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul>) HandleMessage.h:41
    #5 0x10e0198cf in void IPC::callMemberFunction<WebKit::WebAuthenticatorCoordinatorProxy, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&), std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialRequestOptions>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul> >(std::__1::tuple<unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul>, WebCore::PublicKeyCredentialRequestOptions>&&, WebKit::WebAuthenticatorCoordinatorProxy*, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&)) HandleMessage.h:47
    #6 0x10e011277 in void IPC::handleMessage<Messages::WebAuthenticatorCoordinatorProxy::GetAssertion, WebKit::WebAuthenticatorCoordinatorProxy, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&)>(IPC::Decoder&, WebKit::WebAuthenticatorCoordinatorProxy*, void (WebKit::WebAuthenticatorCoordinatorProxy::*)(unsigned long long, WTF::Vector<unsigned char, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::PublicKeyCredentialRequestOptions const&)) HandleMessage.h:120
    #7 0x10e010eac in WebKit::WebAuthenticatorCoordinatorProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) WebAuthenticatorCoordinatorProxyMessageReceiver.cpp:49
    #8 0x10ce87d48 in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) MessageReceiverMap.cpp:123

[Brent Fulgham]

Actually, I can still hit a debug assertion if I run the tests enough times (1000x or so), so perhaps this isn't ready yet.

[Jiewen Tan]

(In reply to Brent Fulgham from comment #7)
> Actually, I can still hit a debug assertion if I run the tests enough times
> (1000x or so), so perhaps this isn't ready yet.

I think this is a different issue. This is a race condition that when a new request comes in right between the previous one finishes and the clearStateAsync is queue after. Therefore, when the new request starts discovery, it will still see previous request's state.

You can assign this back to me, and I will investigate a fix.

[Brent Fulgham]

*** Bug 194780 has been marked as a duplicate of this bug. ***

[Brent Fulgham]

*** Bug 196377 has been marked as a duplicate of this bug. ***

[Jiewen Tan]

Created attachment 371918 [details]
Patch

[Brent Fulgham]

Comment on attachment 371918 [details]
Patch

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

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:178
> +        return;

I'm not sure I follow this new behavior. Does calling the completion handler (e.g., lines 132 and 152 above) cause the completion handler to be cleared? I guess it must.

[Jiewen Tan]

Comment on attachment 371918 [details]
Patch

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

>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:178
>> +        return;
> 
> I'm not sure I follow this new behavior. Does calling the completion handler (e.g., lines 132 and 152 above) cause the completion handler to be cleared? I guess it must.

Yes, it does. And m_pendingCompletionHandler should be called before every clearState(). Therefore, if clearState() is called with m_pendingCompletionHandler, that means clearState() is called from clearStateAsync() and there is currently an active request. Therefore, the method should just return.

[Brent Fulgham]

Comment on attachment 371918 [details]
Patch

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

>>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:178
>>> +        return;
>> 
>> I'm not sure I follow this new behavior. Does calling the completion handler (e.g., lines 132 and 152 above) cause the completion handler to be cleared? I guess it must.
> 
> Yes, it does. And m_pendingCompletionHandler should be called before every clearState(). Therefore, if clearState() is called with m_pendingCompletionHandler, that means clearState() is called from clearStateAsync() and there is currently an active request. Therefore, the method should just return.

Okay -- makes sense.

[Jiewen Tan]

Comment on attachment 371918 [details]
Patch

Thanks Brent for r+ this patch.

[WebKit Commit Bot]

Comment on attachment 371918 [details]
Patch

Clearing flags on attachment: 371918

Committed r246369: <https://trac.webkit.org/changeset/246369>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.