Bug 211300

Summary: [Catalina] ASSERTION FAILED: Unsafe to ref/deref from different threads seen with TestWebKitAPI.MultipleClientCertificateConnections.Failure
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ap, darin, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
crash log
none
Patch
none
Patch
none
Patch none

Description Ryan Haddad 2020-05-01 10:29:07 PDT 
Created attachment 398199 [details]
crash log

TestWebKitAPI.MultipleClientCertificateConnections.Failure appears to be intermittently crashing on Catalina Debug bots with the following assertion failure:

    TestWebKitAPI.MultipleClientCertificateConnections.Failure
        ASSERTION FAILED: Unsafe to ref/deref from different threads
        m_isOwnedByMainThread == isMainThread()
        /Volumes/Data/slave/catalina-debug/build/WebKitBuild/Debug/usr/local/include/wtf/RefCounted.h(114) : void WTF::RefCountedBase::applyRefDerefThreadingCheck() const
        1   0x110d71959 WTFCrash
        2   0x10da13621 WTF::RefCountedBase::applyRefDerefThreadingCheck() const
        3   0x10da133fc WTF::RefCountedBase::derefBase() const
        4   0x10dd0d02f WTF::RefCounted<TestWebKitAPI::HTTPServer::RequestData, std::__1::default_delete<TestWebKitAPI::HTTPServer::RequestData> >::deref() const
        5   0x10dd0d001 void WTF::derefIfNotNull<TestWebKitAPI::HTTPServer::RequestData>(TestWebKitAPI::HTTPServer::RequestData*)
        6   0x10dd0cfc9 WTF::RefPtr<TestWebKitAPI::HTTPServer::RequestData, WTF::DumbPtrTraits<TestWebKitAPI::HTTPServer::RequestData> >::~RefPtr()
        7   0x10dd02f55 WTF::RefPtr<TestWebKitAPI::HTTPServer::RequestData, WTF::DumbPtrTraits<TestWebKitAPI::HTTPServer::RequestData> >::~RefPtr()
        8   0x10dd0b6cc __destroy_helper_block_e8_32c61_ZTSN3WTF9RetainPtrIU27objcproto16OS_nw_connection8NSObjectEE40c85_ZTSN3WTF6RefPtrIN13TestWebKitAPI10HTTPServer11RequestDataENS_13DumbPtrTraitsIS3_EEEE
        9   0x7fff6d727bd5 _Block_release
        10  0x7fff6c0b4516 nw_write_request_report
        11  0x7fff6c0b403a nw_write_request_list_prune
        12  0x7fff6c0ab8b5 nw_flow_prepare_output_frames
        13  0x7fff6c0a58ee nw_flow_service_writes
        14  0x7fff6c0a4d58 nw_endpoint_handler_service_writes
        15  0x7fff6c0a05cc nw_connection_add_write_request_on_queue
        16  0x7fff6d67e6c4 _dispatch_call_block_and_release
        17  0x7fff6d67f658 _dispatch_client_callout
        18  0x7fff6d686909 _dispatch_workloop_invoke
        19  0x7fff6d68ec09 _dispatch_workloop_worker_thread
        20  0x7fff6d8d9a3d _pthread_wqthread

https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.MultipleClientCertificateConnections.Failure
Comment 1 Ryan Haddad 2020-05-01 10:29:54 PDT 
The first crash I see in the result history is @ r260689 on 4/24/2020.
Comment 2 Radar WebKit Bug Importer 2020-05-01 10:30:12 PDT 
<rdar://problem/62735361>
Comment 3 Alex Christensen 2020-05-01 15:33:34 PDT 
Created attachment 398254 [details]
Patch
Comment 4 Alexey Proskuryakov 2020-05-01 17:04:45 PDT 
Comment on attachment 398254 [details]
Patch

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

> Tools/ChangeLog:9
> +        To make them safe to destroy on non-main threads, make RequestData ThreadSafeRefCounted.

Wouldn't that keep derefing Strings and HTTPResponses from requestMap on the wrong thread?
Comment 5 Darin Adler 2020-05-01 18:38:13 PDT 
Comment on attachment 398254 [details]
Patch

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

>> Tools/ChangeLog:9
>> +        To make them safe to destroy on non-main threads, make RequestData ThreadSafeRefCounted.
> 
> Wouldn't that keep derefing Strings and HTTPResponses from requestMap on the wrong thread?

It would.
Comment 6 youenn fablet 2020-05-04 07:20:29 PDT 
Comment on attachment 398254 [details]
Patch

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

> Tools/TestWebKitAPI/cocoa/HTTPServer.mm:41
> +struct HTTPServer::RequestData : public ThreadSafeRefCounted<RequestData> {

Should we make sure to destroy them in Main Thread using DestructionThread::MainRunLoop?
Comment 7 Alex Christensen 2020-05-04 10:25:15 PDT 
Created attachment 398386 [details]
Patch
Comment 8 youenn fablet 2020-05-04 10:53:48 PDT 
Comment on attachment 398386 [details]
Patch

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

> Tools/ChangeLog:9
> +        To make them safe to destroy on non-main threads, make RequestData ThreadSafeRefCounted.

s/destroy/ref and unref/
Comment 9 Alex Christensen 2020-05-04 10:55:24 PDT 
Created attachment 398392 [details]
Patch
Comment 10 EWS 2020-05-04 12:46:31 PDT 
Committed r261101: <https://trac.webkit.org/changeset/261101>

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