Bug 230662 - REGRESSION (r282865?): ASSERTION FAILED: Unsafe to ref/deref from different threads m_isOwnedByMainThread == isMainThread() (230644)
Summary: REGRESSION (r282865?): ASSERTION FAILED: Unsafe to ref/deref from different t...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL:
Keywords: InRadar
: 230644 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-09-22 20:06 PDT by Jean-Yves Avenard [:jya]
Modified: 2021-09-28 05:28 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.99 KB, patch)
2021-09-22 20:21 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (4.22 KB, patch)
2021-09-22 20:42 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Yves Avenard [:jya] 2021-09-22 20:06:14 PDT
Seen in WK1 tests:
2021-09-22 12:21:01.415 DumpRenderTree[58389:29549883] *** WARNING: Method convertPointToBase: in class NSView is deprecated on 10.7 and later. It should not be used in new applications. 
ASSERTION FAILED: Unsafe to ref/deref from different threads
m_isOwnedByMainThread == isMainThread()
/Volumes/Data/worker/bigsur-debug/build/WebKitBuild/Debug/usr/local/include/wtf/RefCounted.h(114) : void WTF::RefCountedBase::applyRefDerefThreadingCheck() const
1   0x107107539 WTFCrash
2   0x12849df91 WTF::RefCountedBase::applyRefDerefThreadingCheck() const
3   0x12849dd6c WTF::RefCountedBase::derefBase() const
4   0x12850920f WTF::RefCounted<WebCore::SharedBuffer, std::__1::default_delete<WebCore::SharedBuffer> >::deref() const
5   0x1285091dc WTF::Ref<WebCore::SharedBuffer, WTF::RawPtrTraits<WebCore::SharedBuffer> >::~Ref()
6   0x1284fe755 WTF::Ref<WebCore::SharedBuffer, WTF::RawPtrTraits<WebCore::SharedBuffer> >::~Ref()
7   0x1289ba0ea WebCore::SourceBufferParserAVFObjC::appendData(WebCore::SourceBufferParser::Segment&&, WTF::CompletionHandler<void ()>&&, WebCore::SourceBufferParser::AppendFlags)
8   0x1289cbf7c WebCore::SourceBufferPrivateAVFObjC::append(WTF::Ref<WebCore::SharedBuffer, WTF::RawPtrTraits<WebCore::SharedBuffer> >&&)::$_13::operator()()
9   0x1289cbeac invocation function for block in WebCore::SourceBufferPrivateAVFObjC::append(WTF::Ref<WebCore::SharedBuffer, WTF::RawPtrTraits<WebCore::SharedBuffer> >&&)
10  0x7fff2020d623 _dispatch_call_block_and_release
11  0x7fff2020e806 _dispatch_client_callout
12  0x7fff202111b0 _dispatch_continuation_pop
13  0x7fff20210887 _dispatch_async_redirect_invoke
14  0x7fff2021d818 _dispatch_root_queue_drain
15  0x7fff2021df70 _dispatch_worker_thread2
16  0x7fff203b5417 _pthread_wqthread
17  0x7fff203b442f start_wqthread

The issue at hand is that the call to dispatch_async takes an objective-C block ; which doesn't move the rvalue but instead copy it.
So we end up with all the captured objects being copied which increase the refcount to the SharedBuffer.

Due to a race with the task being run (and destructed) on the source buffer parser thread before the dispatch_async returns, and trigger the assertion.

The quick fix is to make SharedBuffer have a thread-safe refcount ; long term fix would be to have the SourceBufferPrivateAVFObjC use a WorkQueue instead, but due to how it would be used with the SourceBufferPrivate, it needs extra method (such as WaitUntilIdle() )
Comment 1 Jean-Yves Avenard [:jya] 2021-09-22 20:06:24 PDT
rdar://83419269
Comment 2 Jean-Yves Avenard [:jya] 2021-09-22 20:21:11 PDT
Created attachment 439006 [details]
Patch
Comment 3 Jean-Yves Avenard [:jya] 2021-09-22 20:42:23 PDT
Created attachment 439009 [details]
Patch

Reverse now unnecessary test expectations
Comment 4 EWS 2021-09-22 22:05:09 PDT
Committed r282924 (242038@main): <https://commits.webkit.org/242038@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439009 [details].
Comment 5 Jean-Yves Avenard [:jya] 2021-09-25 22:01:01 PDT
*** Bug 230644 has been marked as a duplicate of this bug. ***