Bug 199667

Summary: [RealtimeMediaSource] Revert back to using a WeakRef when deffering a task
Product: WebKit Reporter: Claudio Saavedra <csaavedra>
Component: MediaAssignee: Thibault Saunier <tsaunier>
Status: RESOLVED FIXED    
Severity: Normal CC: pnormand, tsaunier, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Claudio Saavedra 2019-07-10 05:47:39 PDT
The commit:

https://trac.webkit.org/changeset/247211/webkit

Here's the stacktrace:

#0  0x0000558e6770d468 in std::__atomic_base<unsigned char>::compare_exchange_weak (__m2=std::memory_order_acquire, __m1=std::memory_order_acquire, __i2=1 '\001', __i1=@0x7fff3f4c226b: 0 '\000', this=0x0)
    at /usr/include/c++/8/bits/atomic_base.h:434
#1  std::__atomic_base<unsigned char>::compare_exchange_weak (__m=std::memory_order_acquire, __i2=1 '\001', __i1=<optimized out>, this=0x0) at /usr/include/c++/8/bits/atomic_base.h:456
#2  WTF::Atomic<unsigned char>::compareExchangeWeak (this=0x0, expected=0 '\000', desired=1 '\001', order=std::memory_order_acquire) at DerivedSources/ForwardingHeaders/wtf/Atomics.h:86
#3  0x0000558e677083d7 in WTF::LockAlgorithm<unsigned char, (unsigned char)1, (unsigned char)2, WTF::EmptyLockHooks<unsigned char> >::lockFastAssumingZero (lock=...)
    at DerivedSources/ForwardingHeaders/wtf/LockAlgorithm.h:53
#4  0x0000558e67704e74 in WTF::Lock::lock (this=0x0) at DerivedSources/ForwardingHeaders/wtf/Lock.h:58
#5  0x0000558e67712c5a in std::lock_guard<WTF::Lock>::lock_guard (this=0x7fff3f4c2328, __m=...) at /usr/include/c++/8/bits/std_mutex.h:162
#6  0x00007f25029e3fe1 in WTF::addIterator<WebCore::RealtimeMediaSource::Observer*, WebCore::RealtimeMediaSource::Observer*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > (table=0x7f24ee2f7750, it=0x7fff3f4c2400) at DerivedSources/ForwardingHeaders/wtf/HashTable.h:1470
#7  0x00007f25029e578c in WTF::HashTableConstIterator<WebCore::RealtimeMediaSource::Observer*, WebCore::RealtimeMediaSource::Observer*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> >::HashTableConstIterator (this=0x7fff3f4c2400, table=0x7f24ee2f7750, position=0xbbadbf2f, 
    endPosition=0xbbadbf2f) at DerivedSources/ForwardingHeaders/wtf/HashTable.h:135
#8  0x00007f25029e4d1e in WTF::HashTable<WebCore::RealtimeMediaSource::Observer*, WebCore::RealtimeMediaSource::Observer*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> >::makeKnownGoodConstIterator (this=0x7f24ee2f7750, pos=0xbbadbf2f)
    at DerivedSources/ForwardingHeaders/wtf/HashTable.h:489
#9  0x00007f25029e4cd2 in WTF::HashTable<WebCore::RealtimeMediaSource::Observer*, WebCore::RealtimeMediaSource::Observer*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> >::end (this=0x7f24ee2f7750) at DerivedSources/ForwardingHeaders/wtf/HashTable.h:381
#10 0x00007f25029e58ef in WTF::HashTable<WebCore::RealtimeMediaSource::Observer*, WebCore::RealtimeMediaSource::Observer*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> >::begin (this=0x7f24ee2f7750) at DerivedSources/ForwardingHeaders/wtf/HashTable.h:380
#11 0x00007f25029e4f07 in WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> >::begin (
    this=0x7f24ee2f7750) at DerivedSources/ForwardingHeaders/wtf/HashSet.h:187
#12 0x00007f25029e4147 in WTF::Mapper<WTF::copyToVectorOf<WebCore::RealtimeMediaSource::Observer*, WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > >(WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > const&)::{lambda(auto:1 const&)#1}, WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > const&, void>::map(WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > const, {lambda(auto:1 const&)#1} const&) (source=..., mapFunction=...) at DerivedSources/ForwardingHeaders/wtf/Vector.h:1665
#13 0x00007f25029e2c49 in WTF::map<WTF::copyToVectorOf<WebCore::RealtimeMediaSource::Observer*, WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > >(WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > const&)::{lambda(auto:1 const&)#1}, WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > const&>(WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > const&, WTF::copyToVectorOf<WebCore::RealtimeMediaSource::Observer*, WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > >(WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > const&)::{lambda(auto:1 const&)#1}&&) (
    source=..., mapFunction=...) at DerivedSources/ForwardingHeaders/wtf/Vector.h:1690
#14 0x00007f25029e0ed8 in WTF::copyToVectorOf<WebCore::RealtimeMediaSource::Observer*, WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > > (collection=...) at DerivedSources/ForwardingHeaders/wtf/Vector.h:1707
#15 0x00007f25029dee80 in WTF::copyToVector<WTF::HashSet<WebCore::RealtimeMediaSource::Observer*, WTF::PtrHash<WebCore::RealtimeMediaSource::Observer*>, WTF::HashTraits<WebCore::RealtimeMediaSource::Observer*> > > (collection=...) at DerivedSources/ForwardingHeaders/wtf/Vector.h:1718
#16 0x00007f25029d07e1 in WebCore::RealtimeMediaSource::forEachObserver(WTF::Function<void (WebCore::RealtimeMediaSource::Observer&)> const&) const (this=0x7f24ee2f7700, apply=...)
    at ../../Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:129
#17 0x00007f25029d094e in WebCore::RealtimeMediaSource::<lambda()>::operator()(void) const (__closure=0x7f24ee2f9b98) at ../../Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:161
#18 0x00007f25029ddfca in WTF::Detail::CallableWrapper<WebCore::RealtimeMediaSource::notifySettingsDidChangeObservers(WTF::OptionSet<WebCore::RealtimeMediaSourceSettings::Flag>)::<lambda()>, void>::call(void) (
    this=0x7f24ee2f9b90) at DerivedSources/ForwardingHeaders/wtf/Function.h:52
#19 0x0000558e67708e72 in WTF::Function<void ()>::operator()() const (this=0x7f24ee2bb178) at DerivedSources/ForwardingHeaders/wtf/Function.h:79
#20 0x00007f25029d452c in WebCore::RealtimeMediaSource::<lambda()>::operator()(void) const (__closure=0x7f24ee2bb170) at ../../Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:1042
#21 0x00007f25029ddc3e in WTF::Detail::CallableWrapper<WebCore::RealtimeMediaSource::scheduleDeferredTask(WTF::Function<void()>&&)::<lambda()>, void>::call(void) (this=0x7f24ee2bb168)
    at DerivedSources/ForwardingHeaders/wtf/Function.h:52
#22 0x0000558e67708e72 in WTF::Function<void ()>::operator()() const (this=0x7fff3f4c2708) at DerivedSources/ForwardingHeaders/wtf/Function.h:79
#23 0x00007f2504a32ee6 in WTF::dispatchFunctionsFromMainThread () at ../../Source/WTF/wtf/MainThread.cpp:114
#24 0x00007f2504abc171 in WTF::MainThreadDispatcher::fired (this=0x7f250b541980 <WTF::scheduleDispatchFunctionsOnMainThread()::dispatcher>) at ../../Source/WTF/wtf/generic/MainThreadGeneric.cpp:67
#25 0x00007f2504abc2b8 in WTF::RunLoop::Timer<WTF::MainThreadDispatcher>::fired (this=0x7f250b541980 <WTF::scheduleDispatchFunctionsOnMainThread()::dispatcher>) at ../../Source/WTF/wtf/RunLoop.h:152
#26 0x00007f2504ac137f in WTF::RunLoop::TimerBase::<lambda(gpointer)>::operator()(gpointer) const (__closure=0x0, userData=0x7f250b541980 <WTF::scheduleDispatchFunctionsOnMainThread()::dispatcher>)
    at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:171
#27 0x00007f2504ac13d7 in WTF::RunLoop::TimerBase::<lambda(gpointer)>::_FUN(gpointer) () at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:177
#28 0x00007f2504ac0ae2 in WTF::<lambda(GSource*, GSourceFunc, gpointer)>::operator()(GSource *, GSourceFunc, gpointer) const (__closure=0x0, source=0x558e69eea440, 
    callback=0x7f2504ac13ba <WTF::RunLoop::TimerBase::<lambda(gpointer)>::_FUN(gpointer)>, userData=0x7f250b541980 <WTF::scheduleDispatchFunctionsOnMainThread()::dispatcher>)
    at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:45
#29 0x00007f2504ac0b12 in WTF::<lambda(GSource*, GSourceFunc, gpointer)>::_FUN(GSource *, GSourceFunc, gpointer) () at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:46
#30 0x00007f24f441c9c8 in g_main_dispatch (context=0x558e69535650) at ../../Source/glib-2.58.1/glib/gmain.c:3182
#31 g_main_context_dispatch (context=context@entry=0x558e69535650) at ../../Source/glib-2.58.1/glib/gmain.c:3847
#32 0x00007f24f441cd88 in g_main_context_iterate (context=0x558e69535650, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../../Source/glib-2.58.1/glib/gmain.c:3920
#33 0x00007f24f441d072 in g_main_loop_run (loop=0x558e69536850) at ../../Source/glib-2.58.1/glib/gmain.c:4116
#34 0x00007f2504ac100e in WTF::RunLoop::run () at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:96
#35 0x0000558e6774a7c4 in WTR::TestController::platformRunUntil (this=0x7fff3f4c3550, done=@0x558e69537e6b: false, timeout=...) at ../../Tools/WebKitTestRunner/wpe/TestControllerWPE.cpp:83
#36 0x0000558e676fa20d in WTR::TestController::runUntil (this=0x7fff3f4c3550, done=@0x558e69537e6b: false, timeout=...) at ../../Tools/WebKitTestRunner/TestController.cpp:1726
#37 0x0000558e67722492 in WTR::TestInvocation::invoke (this=0x558e69537ce0) at ../../Tools/WebKitTestRunner/TestInvocation.cpp:185
#38 0x0000558e676f9f07 in WTR::TestController::runTest (this=0x7fff3f4c3550, inputLine=0x7fff3f4c2cc0 "/home/zan/Work/webkit/git/LayoutTests/fast/mediastream/MediaDevices-addEventListener.html'--timeout'30000")
    at ../../Tools/WebKitTestRunner/TestController.cpp:1656
#39 0x0000558e676fa0ff in WTR::TestController::runTestingServerLoop (this=0x7fff3f4c3550) at ../../Tools/WebKitTestRunner/TestController.cpp:1702
#40 0x0000558e676fa140 in WTR::TestController::run (this=0x7fff3f4c3550) at ../../Tools/WebKitTestRunner/TestController.cpp:1710
#41 0x0000558e676f36cd in WTR::TestController::TestController (this=0x7fff3f4c3550, argc=2, argv=0x7fff3f4c3888) at ../../Tools/WebKitTestRunner/TestController.cpp:164
#42 0x0000558e6774ada6 in main (argc=2, argv=0x7fff3f4c3888) at ../../Tools/WebKitTestRunner/wpe/main.cpp:35
Comment 1 Thibault Saunier 2019-07-10 16:07:56 PDT
Created attachment 373872 [details]
Patch
Comment 2 youenn fablet 2019-07-10 18:15:34 PDT
Comment on attachment 373872 [details]
Patch

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

> Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:1041
> +    callOnMainThread([weakThis = makeWeakPtr(*this), function = WTFMove(function)] {

makeWeakPtr is not thread safe so we cannot do that.
Even if makeWeakPtr was thread safe, if 'this' is being or about to be destroyed on the main thread, calling makeWeakPtr might not be a good idea anyway.

I wonder why nothing is showing up in Mac bots.

Looking at the crash log, it seems that it is a call to RealtimeMediaSource::notifySettingsDidChangeObservers that crashes.
It should only be called on the main thread (there is a debug assert for that).
I wonder then why the call to scheduleDeferredTask and the makeRef() would fail.
Comment 3 Philippe Normand 2019-07-11 04:05:13 PDT
Is this related with https://bugs.webkit.org/show_bug.cgi?id=194326 ?
Comment 4 Thibault Saunier 2019-08-26 13:28:40 PDT
Fixed by https://bugs.webkit.org/show_bug.cgi?id=194326 - closing.
Comment 5 Radar WebKit Bug Importer 2019-08-26 13:29:29 PDT
<rdar://problem/54719631>