Bug 209655 – [StressGC] ASSERTION FAILED: m_wrapper under WebCore::MainThreadGenericEventQueue::dispatchOneEvent

• Component: Bindings
• Status: RESOLVED FIXED

[Chris Dumez]

[StressGC] ASSERTION FAILED: m_wrapper under WebCore::MainThreadGenericEventQueue::dispatchOneEvent:
ASSERTION FAILED: m_wrapper
./bindings/js/JSEventListener.h(123) : JSC::JSObject *WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext &) const
1   0x1091b70b9 WTFCrash
2   0x11ee6388b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x12101e9f8 WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext&) const
4   0x12101dd1b WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&)
5   0x12166b87c WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase)
6   0x121667b12 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)
7   0x12166b45d WebCore::EventTarget::dispatchEvent(WebCore::Event&)
8   0x1216865f0 WebCore::MainThreadGenericEventQueue::dispatchOneEvent()
9   0x12168b8a1 decltype(*(std::__1::forward<WebCore::MainThreadGenericEventQueue*&>(fp0)).*fp()) std::__1::__invoke<void (WebCore::MainThreadGenericEventQueue::*&)(), WebCore::MainThreadGenericEventQueue*&, void>(void (WebCore::MainThreadGenericEventQueue::*&&&)(), WebCore::MainThreadGenericEventQueue*&&&)
10  0x12168b820 std::__1::__bind_return<void (WebCore::MainThreadGenericEventQueue::*)(), std::__1::tuple<WebCore::MainThreadGenericEventQueue*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::MainThreadGenericEventQueue::*)(), std::__1::tuple<WebCore::MainThreadGenericEventQueue*>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (WebCore::MainThreadGenericEventQueue::*)(), std::__1::tuple<WebCore::MainThreadGenericEventQueue*>, 0ul, std::__1::tuple<> >(void (WebCore::MainThreadGenericEventQueue::*&)(), std::__1::tuple<WebCore::MainThreadGenericEventQueue*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&)
11  0x12168b7cc std::__1::__bind_return<void (WebCore::MainThreadGenericEventQueue::*)(), std::__1::tuple<WebCore::MainThreadGenericEventQueue*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::MainThreadGenericEventQueue::*)(), std::__1::tuple<WebCore::MainThreadGenericEventQueue*>, std::__1::tuple<> >::value>::type std::__1::__bind<void (WebCore::MainThreadGenericEventQueue::*)(), WebCore::MainThreadGenericEventQueue*>::operator()<>()
12  0x12168b769 WTF::Detail::CallableWrapper<std::__1::__bind<void (WebCore::MainThreadGenericEventQueue::*)(), WebCore::MainThreadGenericEventQueue*>, void>::call()
13  0x11ee710fa WTF::Function<void ()>::operator()() const
14  0x11f133a20 WebCore::GenericTaskQueue<WebCore::Timer>::enqueueTask(WTF::Function<void ()>&&)::'lambda'()::operator()() const
15  0x11f133869 WTF::Detail::CallableWrapper<WebCore::GenericTaskQueue<WebCore::Timer>::enqueueTask(WTF::Function<void ()>&&)::'lambda'(), void>::call()
16  0x11ee710fa WTF::Function<void ()>::operator()() const
17  0x12242071e WebCore::TaskDispatcher<WebCore::Timer>::dispatchOneTask()
18  0x1224203c5 WebCore::TaskDispatcher<WebCore::Timer>::sharedTimerFired()
19  0x122424c61 WebCore::TaskDispatcher<WebCore::Timer>::sharedTimer()::$_1::operator()() const
20  0x122424c29 WTF::Detail::CallableWrapper<WebCore::TaskDispatcher<WebCore::Timer>::sharedTimer()::$_1, void>::call()
21  0x11ee710fa WTF::Function<void ()>::operator()() const
22  0x11ef29ee9 WebCore::Timer::fired()
23  0x1224682ea WebCore::ThreadTimers::sharedTimerFiredInternal()
24  0x12246fec1 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const
25  0x12246fe79 WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, void>::call()
26  0x11ee710fa WTF::Function<void ()>::operator()() const
27  0x122435ff7 WebCore::MainThreadSharedTimer::fired()
28  0x1224ca486 WebCore::timerFired(__CFRunLoopTimer*, void*)
29  0x7fff4f3805d5 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
30  0x7fff4f380181 __CFRunLoopDoTimer
31  0x7fff4f37fcba __CFRunLoopDoTimers

[Chris Dumez]

<rdar://problem/60541442>

[Chris Dumez]

Created attachment 394721 [details]
Patch

[Geoffrey Garen]

Comment on attachment 394721 [details]
Patch

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

r=me

> Source/WebCore/html/track/VideoTrackList.h:41
> +        list->suspendIfNeeded();

Can these suspend calls happen in the constructors instead?

[Chris Dumez]

(In reply to Geoffrey Garen from comment #3)
> Comment on attachment 394721 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394721&action=review
> 
> r=me
> 
> > Source/WebCore/html/track/VideoTrackList.h:41
> > +        list->suspendIfNeeded();
> 
> Can these suspend calls happen in the constructors instead?

I am trying to remember why but I am 100% sure that what you are suggesting is bad practice because it does not work in all cases and leads to assertions. The good practice is definitely to do it in the factory like I did. I definitely had to write patches to move the suspendIfNeeded() calls to the constructor call site to address crashes in the past.

[Chris Dumez]

(In reply to Chris Dumez from comment #4)
> (In reply to Geoffrey Garen from comment #3)
> > Comment on attachment 394721 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=394721&action=review
> > 
> > r=me
> > 
> > > Source/WebCore/html/track/VideoTrackList.h:41
> > > +        list->suspendIfNeeded();
> > 
> > Can these suspend calls happen in the constructors instead?
> 
> I am trying to remember why but I am 100% sure that what you are suggesting
> is bad practice because it does not work in all cases and leads to
> assertions. The good practice is definitely to do it in the factory like I
> did. I definitely had to write patches to move the suspendIfNeeded() calls
> to the constructor call site to address crashes in the past.

Ok, found an example:
http://trac.webkit.org/r252497

From my changelog:
"""
    Call suspendIfNeeded() in the factory and not in the constructor. It is never safe to call
    suspendIfNeeded() from inside the constructor because it may call the suspend() method, which
    may ref |this|.
"""

[Geoffrey Garen]

Okeedokee.

[EWS]

Committed r259122: <https://trac.webkit.org/changeset/259122>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394721 [details].