Bug 200734 – Potentially non thread-safe usage of WebCore::MediaSample

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 376308 [details]
Patch

[Simon Fraser (smfr)]

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]

(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 on attachment 376308 [details]
Patch

Clearing flags on attachment: 376308

Committed r248703: <https://trac.webkit.org/changeset/248703>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54330534>