| Summary: | REGRESSION (r245043) [Mac WK2 Debug] ASSERTION FAILED: m_services.isEmpty() && transports.size() <= maxTransportNumber seen with two http/wpt/webauthn/public-key-credential-* tests | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Shawn Roberts <sroberts> | ||||||
| Component: | Tools / Tests | Assignee: | Brent Fulgham <bfulgham> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, commit-queue, jiewen_tan, jlewis3, ryanhaddad, tsavell, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=196377 https://bugs.webkit.org/show_bug.cgi?id=194780 |
||||||||
| Attachments: |
|
||||||||
|
Description
Shawn Roberts
2019-05-15 10:33:55 PDT
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 &) Skipped tests on Debug in https://trac.webkit.org/changeset/245340/webkit Expectations for Release WK2 are still set to Pass / Fail Created attachment 370865 [details]
Patch
The changes in Bug 198308 resolved the underlying issue that triggered this failure. Local testing confirms debug runs do not assert on these tests. 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
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. (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. *** Bug 194780 has been marked as a duplicate of this bug. *** *** Bug 196377 has been marked as a duplicate of this bug. *** Created attachment 371918 [details]
Patch
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. 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. 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. Comment on attachment 371918 [details]
Patch
Thanks Brent for r+ this patch.
Comment on attachment 371918 [details] Patch Clearing flags on attachment: 371918 Committed r246369: <https://trac.webkit.org/changeset/246369> All reviewed patches have been landed. Closing bug. |