Bug 213252

Summary: [ Mac wk2 Debug ] http/tests/inspector/network/intercept tests are crashing with alert - WTFCrash + 14 (Assertions.cpp:295) - WTF::CompletionHandler<void (WebCore::ResourceRequest const&)>::~CompletionHandler
Product: WebKit Reporter: Jason Lawrence <Lawrence.j>
Component: Web InspectorAssignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, hi, inspector-bugzilla-changes, joepeck, pfeldman, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac   
OS: macOS 10.14   
See Also: https://bugs.webkit.org/show_bug.cgi?id=207446
Attachments:
Description Flags
intercept-aborted-request-crash-log
none
Patch
none
Patch
none
Patch none

Description Jason Lawrence 2020-06-16 09:36:02 PDT
Created attachment 402013 [details]
intercept-aborted-request-crash-log

http/tests/inspector/network/intercept-aborted-request.html
http/tests/inspector/network/intercept-request-main-resource-with-response.html
http/tests/inspector/network/intercept-request-subresource-with-error.html
http/tests/inspector/network/intercept-request-subresource-with-response.html
http/tests/inspector/network/intercept-request-with-response.html

Description:
These tests are consistently crashing on Mac wk2 Debug. The crashes are apparent since the tests were introduced here: https://trac.webkit.org/changeset/263072/webkit

History:
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&test=http%2Ftests%2Finspector%2Fnetwork%2Fintercept-aborted-request.html&test=http%2Ftests%2Finspector%2Fnetwork%2Fintercept-request-main-resource-with-response.html&test=http%2Ftests%2Finspector%2Fnetwork%2Fintercept-request-subresource-with-error.html&test=http%2Ftests%2Finspector%2Fnetwork%2Fintercept-request-subresource-with-response.html&test=http%2Ftests%2Finspector%2Fnetwork%2Fintercept-request-with-response.html&style=debug

Crash log:
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000045333bf7e WTFCrash + 14 (Assertions.cpp:295)
1   com.apple.WebCore             	0x0000000438d4be97 WTF::CompletionHandler<void (WebCore::ResourceRequest const&)>::~CompletionHandler() + 87 (CompletionHandler.h:53)
2   com.apple.WebCore             	0x0000000438d4be35 WTF::CompletionHandler<void (WebCore::ResourceRequest const&)>::~CompletionHandler() + 21 (CompletionHandler.h:54)
3   com.apple.WebCore             	0x0000000438d4be06 WebCore::InspectorNetworkAgent::PendingInterceptRequest::~PendingInterceptRequest() + 38 (InspectorNetworkAgent.h:163)
4   com.apple.WebCore             	0x0000000438d4bdb5 WebCore::InspectorNetworkAgent::PendingInterceptRequest::~PendingInterceptRequest() + 21 (InspectorNetworkAgent.h:163)
5   com.apple.WebCore             	0x0000000438d4bd6b std::__1::default_delete<WebCore::InspectorNetworkAgent::PendingInterceptRequest>::operator()(WebCore::InspectorNetworkAgent::PendingInterceptRequest*) const + 43 (memory:2338)
6   com.apple.WebCore             	0x0000000438d4bcef std::__1::unique_ptr<WebCore::InspectorNetworkAgent::PendingInterceptRequest, std::__1::default_delete<WebCore::InspectorNetworkAgent::PendingInterceptRequest> >::reset(WebCore::InspectorNetworkAgent::PendingInterceptRequest*) + 95 (memory:2652)
7   com.apple.WebCore             	0x0000000438d4bc89 std::__1::unique_ptr<WebCore::InspectorNetworkAgent::PendingInterceptRequest, std::__1::default_delete<WebCore::InspectorNetworkAgent::PendingInterceptRequest> >::~unique_ptr() + 25 (memory:2605)
8   com.apple.WebCore             	0x0000000438d20285 std::__1::unique_ptr<WebCore::InspectorNetworkAgent::PendingInterceptRequest, std::__1::default_delete<WebCore::InspectorNetworkAgent::PendingInterceptRequest> >::~unique_ptr() + 21 (memory:2605)
9   com.apple.WebCore             	0x0000000438d220b0 WebCore::InspectorNetworkAgent::interceptRequestWithResponse(WTF::String&, WTF::String const&, WTF::String const&, bool, WTF::String const&, int, WTF::String const&, WTF::JSONImpl::Object const&) + 1648
10  com.apple.WebCore             	0x0000000438d22318 non-virtual thunk to WebCore::InspectorNetworkAgent::interceptRequestWithResponse(WTF::String&, WTF::String const&, WTF::String const&, bool, WTF::String const&, int, WTF::String const&, WTF::JSONImpl::Object const&) + 152
11  com.apple.JavaScriptCore      	0x00000004545f3b13 Inspector::NetworkBackendDispatcher::interceptRequestWithResponse(long, WTF::RefPtr<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&) + 1507 (InspectorBackendDispatchers.cpp:5552)
12  com.apple.JavaScriptCore      	0x00000004545eff59 Inspector::NetworkBackendDispatcher::dispatch(long, WTF::String const&, WTF::Ref<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&) + 905 (InspectorBackendDispatchers.cpp:5135)
13  com.apple.JavaScriptCore      	0x00000004545c88a4 Inspector::BackendDispatcher::dispatch(WTF::String const&) + 1844 (InspectorBackendDispatcher.cpp:179)
14  com.apple.WebCore             	0x0000000438bdb22e WebCore::InspectorController::dispatchMessageFromFrontend(WTF::String const&) + 46 (InspectorController.cpp:393)
15  com.apple.WebKit              	0x00000004295f295c WebKit::WebPageInspectorTarget::sendMessageToTargetBackend(WTF::String const&) + 76 (WebPageInspectorTarget.cpp:70)
16  com.apple.WebKit              	0x00000004295f2cbf WebKit::WebPageInspectorTargetController::sendMessageToTargetBackend(WTF::String const&, WTF::String const&) + 79 (WebPageInspectorTargetController.cpp:89)
17  com.apple.WebKit              	0x000000042995f786 WebKit::WebPage::sendMessageToTargetBackend(WTF::String const&, WTF::String const&) + 54 (WebPage.cpp:3085)
Comment 1 Radar WebKit Bug Importer 2020-06-16 09:40:07 PDT
<rdar://problem/64410036>
Comment 2 Ryan Haddad 2020-06-16 10:01:59 PDT
ASSERTION FAILED: Completion handler should always be called
!m_function
/Volumes/Data/slave/catalina-debug/build/WebKitBuild/Debug/usr/local/include/wtf/CompletionHandler.h(53) : WTF::CompletionHandler<void (const WebCore::ResourceRequest &)>::~CompletionHandler()
Comment 3 Pavel Feldman 2020-06-16 11:30:31 PDT
Created attachment 402027 [details]
Patch
Comment 4 Pavel Feldman 2020-06-16 12:36:59 PDT
Created attachment 402033 [details]
Patch
Comment 5 Devin Rousso 2020-06-16 13:36:34 PDT
Comment on attachment 402033 [details]
Patch

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

r=me

As an alternative (not saying you should do this, just thinking out loud), we could create a `CompletionHandler::markAsCompleted` or `CompletionHandler::clear` that inspector code could call to satisfy the assertion, but that is more work than this solution so what you have is fine too IMO :)

> Source/WebCore/ChangeLog:3
> +        Web Inspector: replace completion handler with a function in interception.

This could use a comment/description.  Something like:
```
    Don't use a `CompletionHandler` as it asserts that it's been called when it's destroyed.  Both `Network.interceptRequestWithResponse` and `Network.interceptRequestWithError` essentially "skip" the network pipeline, so the `CompletionHandler` is not invoked for those commands.
```

> Source/WebKit/ChangeLog:8
> +        * WebProcess/Network/WebLoaderStrategy.cpp:

This doesn't appear to have changed.
Comment 6 Pavel Feldman 2020-06-16 14:43:26 PDT
Created attachment 402041 [details]
Patch
Comment 7 Pavel Feldman 2020-06-16 14:43:48 PDT
All Done.
Comment 8 Devin Rousso 2020-06-16 14:48:01 PDT
Comment on attachment 402041 [details]
Patch

r=me
Comment 9 EWS 2020-06-16 15:23:55 PDT
Committed r263119: <https://trac.webkit.org/changeset/263119>

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