Bug 200734

Summary: Potentially non thread-safe usage of WebCore::MediaSample
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: MediaAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, eric.carlson, jer.noble, sabouhallawa, simon.fraser, 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=206068
Attachments:
Description Flags
Patch none

Chris Dumez
Reported 2019-08-14 13:48:00 PDT
I hit this assertion when running the layout tests locally. Thread 5 Crashed:: Dispatch queue: org.webkit.ImageDecoder 0 com.apple.JavaScriptCore 0x0000000431f89eee WTFCrash + 14 (Assertions.cpp:305) 1 com.apple.WebCore 0x000000041a1a0a2a WTF::RefCountedBase::applyRefDerefThreadingCheck() const + 186 (RefCounted.h:114) 2 com.apple.WebCore 0x000000041a1a07f9 WTF::RefCountedBase::derefBase() const + 25 (RefCounted.h:130) 3 com.apple.WebCore 0x000000041a1cfd0f WTF::RefCounted<WebCore::MediaSample, std::__1::default_delete<WebCore::MediaSample> >::deref() const + 31 (RefCounted.h:189) 4 com.apple.WebCore 0x000000041a1d0555 void WTF::derefIfNotNull<WebCore::MediaSample>(WebCore::MediaSample*) + 53 (RefPtr.h:45) 5 com.apple.WebCore 0x000000041a1d0519 WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> >::~RefPtr() + 41 (RefPtr.h:69) 6 com.apple.WebCore 0x000000041a1c5d75 WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> >::~RefPtr() + 21 (RefPtr.h:69) 7 com.apple.WebCore 0x000000041a31ed63 std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >::~pair() + 35 (utility:316) 8 com.apple.WebCore 0x000000041a31ed35 std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >::~pair() + 21 (utility:316) 9 com.apple.WebCore 0x000000041a31ed19 void std::__1::allocator_traits<WTF::FastAllocator<std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*> > >::__destroy<std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > >(std::__1::integral_constant<bool, false>, WTF::FastAllocator<std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*> >&, std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >*) + 25 (memory:1749) 10 com.apple.WebCore 0x000000041a31ec4d void std::__1::allocator_traits<WTF::FastAllocator<std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*> > >::destroy<std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > >(WTF::FastAllocator<std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*> >&, std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >*) + 29 (memory:1596) 11 com.apple.WebCore 0x000000041a31ebcb std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*>*) + 123 (__tree:1855) 12 com.apple.WebCore 0x000000041a31eb86 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*>*) + 54 (__tree:1852) 13 com.apple.WebCore 0x000000041a31eb86 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*>*) + 54 (__tree:1852) 14 com.apple.WebCore 0x000000041a31eb86 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*>*) + 54 (__tree:1852) 15 com.apple.WebCore 0x000000041a31eb86 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*>*) + 54 (__tree:1852) 16 com.apple.WebCore 0x000000041a31eb86 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*>*) + 54 (__tree:1852) 17 com.apple.WebCore 0x000000041a31eb86 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, void*>*) + 54 (__tree:1852) 18 com.apple.WebCore 0x000000041a31eb45 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::~__tree() + 37 (__tree:1843) 19 com.apple.WebCore 0x000000041a31eb15 std::__1::__tree<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::__map_value_compare<WTF::MediaTime, std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > >, std::__1::less<WTF::MediaTime>, true>, WTF::FastAllocator<std::__1::__value_type<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::~__tree() + 21 (__tree:1843) 20 com.apple.WebCore 0x000000041a31eaf5 std::__1::map<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> >, std::__1::less<WTF::MediaTime>, WTF::FastAllocator<std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::~map() + 21 (map:899) 21 com.apple.WebCore 0x000000041a31ead5 std::__1::map<WTF::MediaTime, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> >, std::__1::less<WTF::MediaTime>, WTF::FastAllocator<std::__1::pair<WTF::MediaTime const, WTF::RefPtr<WebCore::MediaSample, WTF::DumbPtrTraits<WebCore::MediaSample> > > > >::~map() + 21 (map:899) 22 com.apple.WebCore 0x000000041a31eab5 WebCore::PresentationOrderSampleMap::~PresentationOrderSampleMap() + 21 (SampleMap.h:37) 23 com.apple.WebCore 0x000000041a31ea75 WebCore::PresentationOrderSampleMap::~PresentationOrderSampleMap() + 21 (SampleMap.h:37) 24 com.apple.WebCore 0x000000041a31ea43 WebCore::DecodeOrderSampleMap::~DecodeOrderSampleMap() + 35 (SampleMap.h:73) 25 com.apple.WebCore 0x000000041a31ea15 WebCore::DecodeOrderSampleMap::~DecodeOrderSampleMap() + 21 (SampleMap.h:73) 26 com.apple.WebCore 0x000000041a31e9f5 WebCore::SampleMap::~SampleMap() + 21 (SampleMap.h:109) 27 com.apple.WebCore 0x000000041a319175 WebCore::SampleMap::~SampleMap() + 21 (SampleMap.h:109) 28 com.apple.WebCore 0x000000041a319261 WebCore::ImageDecoderAVFObjC::~ImageDecoderAVFObjC() + 49 (ImageDecoderAVFObjC.mm:345) 29 com.apple.WebCore 0x000000041a319315 WebCore::ImageDecoderAVFObjC::~ImageDecoderAVFObjC() + 21 (ImageDecoderAVFObjC.mm:345) 30 com.apple.WebCore 0x000000041a319339 WebCore::ImageDecoderAVFObjC::~ImageDecoderAVFObjC() + 25 (ImageDecoderAVFObjC.mm:345) 31 com.apple.WebCore 0x000000041a31e8d2 WTF::ThreadSafeRefCounted<WebCore::ImageDecoder, (WTF::DestructionThread)0>::deref() const::'lambda'()::operator()() const + 50 (ThreadSafeRefCounted.h:78) 32 com.apple.WebCore 0x000000041a31e88d WTF::ThreadSafeRefCounted<WebCore::ImageDecoder, (WTF::DestructionThread)0>::deref() const + 61 (ThreadSafeRefCounted.h:96) 33 com.apple.WebCore 0x000000041d635fb5 void WTF::derefIfNotNull<WebCore::ImageDecoder>(WebCore::ImageDecoder*) + 53 (RefPtr.h:45) 34 com.apple.WebCore 0x000000041d635f79 WTF::RefPtr<WebCore::ImageDecoder, WTF::DumbPtrTraits<WebCore::ImageDecoder> >::~RefPtr() + 41 (RefPtr.h:69) 35 com.apple.WebCore 0x000000041d62b155 WTF::RefPtr<WebCore::ImageDecoder, WTF::DumbPtrTraits<WebCore::ImageDecoder> >::~RefPtr() + 21 (RefPtr.h:69) 36 com.apple.WebCore 0x000000041d62b0c6 WebCore::ImageSource::~ImageSource() + 262 (ImageSource.cpp:72) 37 com.apple.WebCore 0x000000041d62b195 WebCore::ImageSource::~ImageSource() + 21 (ImageSource.cpp:72) 38 com.apple.WebCore 0x000000041d598a6a WTF::ThreadSafeRefCounted<WebCore::ImageSource, (WTF::DestructionThread)0>::deref() const::'lambda'()::operator()() const + 42 (ThreadSafeRefCounted.h:77) 39 com.apple.WebCore 0x000000041d598a2d WTF::ThreadSafeRefCounted<WebCore::ImageSource, (WTF::DestructionThread)0>::deref() const + 61 (ThreadSafeRefCounted.h:96) 40 com.apple.WebCore 0x000000041d5989df WTF::Ref<WebCore::ImageSource, WTF::DumbPtrTraits<WebCore::ImageSource> >::~Ref() + 47 (Ref.h:61) 41 com.apple.WebCore 0x000000041d5766e5 WTF::Ref<WebCore::ImageSource, WTF::DumbPtrTraits<WebCore::ImageSource> >::~Ref() + 21 (Ref.h:61) 42 com.apple.WebCore 0x000000041d6334ac WebCore::ImageSource::startAsyncDecodingQueue()::$_1::~$_1() + 92 (ImageSource.cpp:350) 43 com.apple.WebCore 0x000000041d62d185 WebCore::ImageSource::startAsyncDecodingQueue()::$_1::~$_1() + 21 (ImageSource.cpp:350) 44 com.apple.WebCore 0x000000041d638bf1 WTF::Detail::CallableWrapper<WebCore::ImageSource::startAsyncDecodingQueue()::$_1, void>::~CallableWrapper() + 49 (Function.h:46) 45 com.apple.WebCore 0x000000041d6386d5 WTF::Detail::CallableWrapper<WebCore::ImageSource::startAsyncDecodingQueue()::$_1, void>::~CallableWrapper() + 21 (Function.h:46) 46 com.apple.WebCore 0x000000041d6386f9 WTF::Detail::CallableWrapper<WebCore::ImageSource::startAsyncDecodingQueue()::$_1, void>::~CallableWrapper() + 25 (Function.h:46) 47 com.apple.JavaScriptCore 0x0000000431f9faef std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> >::operator()(WTF::Detail::CallableWrapperBase<void>*) const + 47 (memory:2340) 48 com.apple.JavaScriptCore 0x0000000431f9fa6f std::__1::unique_ptr<WTF::Detail::CallableWrapperBase<void>, std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> > >::reset(WTF::Detail::CallableWrapperBase<void>*) + 95 (memory:2653) 49 com.apple.JavaScriptCore 0x0000000431f9fa09 std::__1::unique_ptr<WTF::Detail::CallableWrapperBase<void>, std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> > >::~unique_ptr() + 25 (memory:2606) 50 com.apple.JavaScriptCore 0x0000000431f9f9e5 std::__1::unique_ptr<WTF::Detail::CallableWrapperBase<void>, std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> > >::~unique_ptr() + 21 (memory:2606) 51 com.apple.JavaScriptCore 0x0000000431f9f9c5 WTF::Function<void ()>::~Function() + 21 (Function.h:59) 52 com.apple.JavaScriptCore 0x0000000431f9ee65 WTF::Function<void ()>::~Function() + 21 (Function.h:59) 53 com.apple.JavaScriptCore 0x000000043209a813 WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0::~$_0() + 35 (WorkQueueCocoa.cpp:36) 54 com.apple.JavaScriptCore 0x0000000432099e75 WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0::~$_0() + 21 (WorkQueueCocoa.cpp:36) 55 com.apple.JavaScriptCore 0x000000043209a540 WTF::BlockPtr<void ()> WTF::BlockPtr<void ()>::fromCallable<WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0>(WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0)::'lambda'(void const*)::operator()(void const*) const + 32 (BlockPtr.h:84) 56 com.apple.JavaScriptCore 0x000000043209a4c5 WTF::BlockPtr<void ()> WTF::BlockPtr<void ()>::fromCallable<WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0>(WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0)::'lambda'(void const*)::__invoke(void const*) + 21 (BlockPtr.h:82) 57 libsystem_blocks.dylib 0x00007fff6e46ebed _Block_release + 101 58 libdispatch.dylib 0x00007fff6e3c75ee _dispatch_client_callout + 8 59 libdispatch.dylib 0x00007fff6e3ccbae _dispatch_lane_serial_drain + 597 60 libdispatch.dylib 0x00007fff6e3cd532 _dispatch_lane_invoke + 363 61 libdispatch.dylib 0x00007fff6e3d6ba1 _dispatch_workloop_worker_thread + 582 62 libsystem_pthread.dylib 0x00007fff6e620773 _pthread_wqthread + 290 63 libsystem_pthread.dylib 0x00007fff6e6205cf start_wqthread + 15 WebCore::MediaSample is not ThreadSafeRefCounted but appears to be ref'd / deref'd from several threads.
Attachments
Patch (2.97 KB, patch)
2019-08-14 14:23 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2019-08-14 14:23:24 PDT
Created attachment 376308 [details] Patch
Simon Fraser (smfr)
Comment 2 2019-08-14 14:24:45 PDT
Comment on attachment 376308 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376308&action=review > Source/WebCore/platform/graphics/ImageSource.cpp:51 > + ASSERT(isMainThread()); Do these do the right thing in UIWebView?
Chris Dumez
Comment 3 2019-08-14 14:29:49 PDT
(In reply to Simon Fraser (smfr) from comment #2) > Comment on attachment 376308 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=376308&action=review > > > Source/WebCore/platform/graphics/ImageSource.cpp:51 > > + ASSERT(isMainThread()); > > Do these do the right thing in UIWebView? I believe so. For UIView, this check will be a isWebThread or isUIThreadHoldingWebThreadLock check, which I believe is what we want.
WebKit Commit Bot
Comment 4 2019-08-14 19:21:01 PDT
Comment on attachment 376308 [details] Patch Clearing flags on attachment: 376308 Committed r248703: <https://trac.webkit.org/changeset/248703>
WebKit Commit Bot
Comment 5 2019-08-14 19:21:03 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 6 2019-08-14 19:22:18 PDT
<rdar://problem/54330534>
Note You need to log in before you can comment on or make changes to this bug.