Bug 171529

Summary: ASSERTION FAILED: wasRemoved in WebCore::RealtimeMediaSourceCenter::removeDevicesChangedObserver(DevicesChangedObserverToken)
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Eric Carlson <eric.carlson>
Status: ASSIGNED    
Severity: Normal CC: ap, commit-queue, eric.carlson, jer.noble, jlewis3, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Crashlog
none
Crashlog
none
Proposed patch.
none
Patch for landing.
none
Patch for landing. none

Ryan Haddad
Reported 2017-05-01 17:50:16 PDT
This is a flaky assertion failure seen with LayoutTest fast/dom/Window/remove-timeout-crash.html https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r216041%20(909)/results.html https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fdom%2FWindow%2Fremove-timeout-crash.html ASSERTION FAILED: wasRemoved /Volumes/Data/slave/sierra-debug/build/Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp(171) : void WebCore::RealtimeMediaSourceCenter::removeDevicesChangedObserver(DevicesChangedObserverToken) 1 0x10494fa0d WTFCrash 2 0x114e1c2d6 WebCore::RealtimeMediaSourceCenter::removeDevicesChangedObserver(unsigned int) 3 0x114a79861 WebCore::MediaDevices::~MediaDevices() 4 0x114a798e5 WebCore::MediaDevices::~MediaDevices() 5 0x114a79929 WebCore::MediaDevices::~MediaDevices() 6 0x11435b4bf WTF::RefCounted<WebCore::MediaDevices>::deref() const 7 0x114c72ff5 void WTF::derefIfNotNull<WebCore::MediaDevices>(WebCore::MediaDevices*) 8 0x114c72fb3 WTF::RefPtr<WebCore::MediaDevices>::~RefPtr() 9 0x114c72695 WTF::RefPtr<WebCore::MediaDevices>::~RefPtr() 10 0x114c7265c WebCore::NavigatorMediaDevices::~NavigatorMediaDevices() 11 0x114c726b5 WebCore::NavigatorMediaDevices::~NavigatorMediaDevices() 12 0x114c726f9 WebCore::NavigatorMediaDevices::~NavigatorMediaDevices() 13 0x114c6ccb3 WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > >::~KeyValuePair() 14 0x114c6cbe5 WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > >::~KeyValuePair() 15 0x114c6cb74 WTF::HashTable<char const*, WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >, WTF::PtrHash<char const*>, WTF::HashMap<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > >, WTF::PtrHash<char const*>, WTF::HashTraits<char const*>, WTF::HashTraits<std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >::KeyValuePairTraits, WTF::HashTraits<char const*> >::deallocateTable(WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > >*, unsigned int) 16 0x114c6c969 WTF::HashTable<char const*, WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >, WTF::PtrHash<char const*>, WTF::HashMap<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > >, WTF::PtrHash<char const*>, WTF::HashTraits<char const*>, WTF::HashTraits<std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >::KeyValuePairTraits, WTF::HashTraits<char const*> >::~HashTable() 17 0x114c6c925 WTF::HashTable<char const*, WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >, WTF::PtrHash<char const*>, WTF::HashMap<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > >, WTF::PtrHash<char const*>, WTF::HashTraits<char const*>, WTF::HashTraits<std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >::KeyValuePairTraits, WTF::HashTraits<char const*> >::~HashTable() 18 0x114c6c905 WTF::HashMap<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > >, WTF::PtrHash<char const*>, WTF::HashTraits<char const*>, WTF::HashTraits<std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >::~HashMap() 19 0x114c6c8e5 WTF::HashMap<char const*, std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > >, WTF::PtrHash<char const*>, WTF::HashTraits<char const*>, WTF::HashTraits<std::__1::unique_ptr<WebCore::Supplement<WebCore::Navigator>, std::__1::default_delete<WebCore::Supplement<WebCore::Navigator> > > > >::~HashMap() 20 0x114c6c0d5 WebCore::Supplementable<WebCore::Navigator>::~Supplementable() 21 0x114c6c04c WebCore::Navigator::~Navigator() 22 0x114c6c0f5 WebCore::Navigator::~Navigator() 23 0x114c6c139 WebCore::Navigator::~Navigator() 24 0x113495ccf WTF::RefCounted<WebCore::NavigatorBase>::deref() const 25 0x113495c71 WTF::Ref<WebCore::Navigator>::~Ref() 26 0x113486e55 WTF::Ref<WebCore::Navigator>::~Ref() 27 0x1143cdd29 WebCore::JSDOMWrapper<WebCore::Navigator>::~JSDOMWrapper() 28 0x1143cdd05 WebCore::JSNavigator::~JSNavigator() 29 0x1143ca7d5 WebCore::JSNavigator::~JSNavigator() 30 0x1143ca42d WebCore::JSNavigator::destroy(JSC::JSCell*) 31 0x104307b5a JSC::(anonymous namespace)::DestroyFunc::operator()(JSC::VM&, JSC::JSCell*) const
Attachments
Crashlog (123.68 KB, text/plain)
2017-05-02 13:43 PDT, Ryan Haddad 		no flags
Details
Crashlog (103.88 KB, text/plain)
2017-05-02 13:49 PDT, Ryan Haddad 		no flags
Details
Proposed patch. (4.08 KB, patch)
2017-05-12 15:11 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch for landing. (5.96 KB, patch)
2017-05-15 09:11 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Patch for landing. (5.97 KB, patch)
2017-05-15 10:00 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ryan Haddad
Comment 1 2017-05-01 17:57:31 PDT
This assert was added in https://trac.webkit.org/changeset/215929/webkit
Ryan Haddad
Comment 2 2017-05-02 13:43:01 PDT
Also saw this with fast/dom/Window/timeout-callback-scope.html https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r216079%20(926)/results.html
Ryan Haddad
Comment 3 2017-05-02 13:43:11 PDT
Created attachment 308848 [details] Crashlog
Radar WebKit Bug Importer
Comment 4 2017-05-02 13:43:45 PDT
<rdar://problem/31945791>
Eric Carlson
Comment 5 2017-05-02 13:47:37 PDT
The attached crashlog doesn't seem to be related: Thread 24 Crashed:: WTF::AutomaticThread 0 com.apple.JavaScriptCore 0x000000010c61cb8c JSC::MarkedBlock::vm() const + 12 (MarkedBlock.h:416) 1 com.apple.JavaScriptCore 0x000000010c6a27d1 JSC::HeapCell::vm() const + 81 (HeapCellInlines.h:67) 2 com.apple.JavaScriptCore 0x000000010c6a67af JSC::JSCell::structure() const + 31 (JSCellInlines.h:110)
Ryan Haddad
Comment 6 2017-05-02 13:48:55 PDT
(In reply to Eric Carlson from comment #5) > The attached crashlog doesn't seem to be related: > > Thread 24 Crashed:: WTF::AutomaticThread > 0 com.apple.JavaScriptCore 0x000000010c61cb8c JSC::MarkedBlock::vm() > const + 12 (MarkedBlock.h:416) > 1 com.apple.JavaScriptCore 0x000000010c6a27d1 JSC::HeapCell::vm() > const + 81 (HeapCellInlines.h:67) > 2 com.apple.JavaScriptCore 0x000000010c6a67af > JSC::JSCell::structure() const + 31 (JSCellInlines.h:110) Attached the wrong one, sorry. Will fix.
Ryan Haddad
Comment 7 2017-05-02 13:49:37 PDT
Created attachment 308849 [details] Crashlog
Alexey Proskuryakov
Comment 8 2017-05-06 20:48:31 PDT
EWS hits these assertions a lot, and is noticeably less reliable because of them. A quick fix would be appreciated.
Eric Carlson
Comment 9 2017-05-12 15:11:04 PDT
Created attachment 309952 [details] Proposed patch.
Jer Noble
Comment 10 2017-05-12 15:28:32 PDT
Comment on attachment 309952 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=309952&action=review > Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:205 > - auto callbacks = m_devicesChangedObservers; > - for (auto it : callbacks) > + // Copy the hash map because the observer callback may call back in and modify the map. > + auto callbacks = observerMap(); > + for (auto& it : callbacks) This solves a certain class of problems, but with this change, it's still possible for a callback to call a freshly deleted object. So please make sure that the classes registering for observers use a strong- or weak-pointer to protect against being called back after destruction time.
Eric Carlson
Comment 11 2017-05-15 09:11:55 PDT
Created attachment 310140 [details] Patch for landing.
Eric Carlson
Comment 12 2017-05-15 10:00:11 PDT
Created attachment 310143 [details] Patch for landing.
WebKit Commit Bot
Comment 13 2017-05-15 11:26:11 PDT
Comment on attachment 310143 [details] Patch for landing. Clearing flags on attachment: 310143 Committed r216866: <http://trac.webkit.org/changeset/216866>
Note You need to log in before you can comment on or make changes to this bug.