Bug 209863 – ASSERTION FAILED: m_wrapper on webgl/max-active-contexts-webglcontextlost-prevent-default.html

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

I get a reproducible assertion failure on webgl/max-active-contexts-webglcontextlost-prevent-default.html

run-webkit-tests -v --repeat-each 10 -1 --additional-env-var=JSC_slowPathAllocsBetweenGCs=25 LayoutTests/webgl/max-active-contexts-*.html --child-processes=1

ASSERTION FAILED: m_wrapper
./bindings/js/JSEventListener.h(125) : JSC::JSObject *WebCore::JSEventListener::ensureJSFunction(WebCore::ScriptExecutionContext &) const
1   0x10eb71779 WTFCrash
2   0x1289cd03b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x12abd376f WebCore::JSEventListener::ensureJSFunction(WebCore::ScriptExecutionContext&) const
4   0x12abd2add WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&)
5   0x12b21f107 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   0x12b21b364 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)
7   0x12b2915a2 WebCore::Node::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)
8   0x12b209bf1 WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const
9   0x12b20a6bf WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&)
10  0x12b20a1f7 WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
11  0x12b2915fd WebCore::Node::dispatchEvent(WebCore::Event&)
12  0x12b7cf9a7 WebCore::WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent()::$_13::operator()() const
13  0x12b7cf90e WTF::Detail::CallableWrapper<WebCore::WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent()::$_13, void>::call()
14  0x1289df8a2 WTF::Function<void ()>::operator()() const
15  0x12b7cef7e void WebCore::ActiveDOMObject::queueTaskKeepingObjectAlive<WebCore::HTMLCanvasElement>(WebCore::HTMLCanvasElement&, WebCore::TaskSource, WTF::Function<void ()>&&)::'lambda'()::operator()() const
16  0x12b7cecae WTF::Detail::CallableWrapper<void WebCore::ActiveDOMObject::queueTaskKeepingObjectAlive<WebCore::HTMLCanvasElement>(WebCore::HTMLCanvasElement&, WebCore::TaskSource, WTF::Function<void ()>&&)::'lambda'(), void>::call()
17  0x1289df8a2 WTF::Function<void ()>::operator()() const
18  0x12b20f89e WebCore::EventLoopFunctionDispatchTask::execute()
19  0x12b20ce05 WebCore::EventLoop::run()
20  0x12b36229c WebCore::WindowEventLoop::didReachTimeToRun()
21  0x12b365307 decltype(*(std::__1::forward<WebCore::WindowEventLoop*&>(fp0)).*fp()) std::__1::__invoke<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&, void>(void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&)
22  0x12b365280 std::__1::__bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, 0ul, std::__1::tuple<> >(void (WebCore::WindowEventLoop::*&)(), std::__1::tuple<WebCore::WindowEventLoop*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&)
23  0x12b365239 std::__1::__bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<> >::value>::type std::__1::__bind<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*>::operator()<>()
24  0x12b3651de WTF::Detail::CallableWrapper<std::__1::__bind<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*>, void>::call()
25  0x1289df8a2 WTF::Function<void ()>::operator()() const
26  0x128a9986e WebCore::Timer::fired()
27  0x12c006c54 WebCore::ThreadTimers::sharedTimerFiredInternal()
28  0x12c00e361 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const
29  0x12c00e30e WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, void>::call()
30  0x1289df8a2 WTF::Function<void ()>::operator()() const
31  0x12bfd362b WebCore::MainThreadSharedTimer::fired()

[Radar WebKit Bug Importer]

<rdar://problem/61164936>

[Chris Dumez]

Which revision?

[Alexey Proskuryakov]

r259292

[Chris Dumez]

Since r259130, WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent() keeps the wrapper alive while the event is pending. Give that we're crashing still, I believe this indicates the JS wrapper can get destroyed before scheduleTaskToDispatchContextLostEvent() even gets called.

[Chris Dumez]

The provided command reproduced the issue for me, thanks.

[Chris Dumez]

Created attachment 395192 [details]
Patch

[Darin Adler]

Comment on attachment 395192 [details]
Patch

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

> Source/WebCore/html/HTMLCanvasElement.cpp:963
> +    if (is<WebGLRenderingContextBase>(m_context.get())) {
> +        // WebGL rendering context may fire contextlost / contextchange / contextrestored events at any point.
> +        return m_hasRelevantWebGLEventListener && !downcast<WebGLRenderingContextBase>(*m_context).isContextUnrecoverablyLost();
> +    }

Why not include is<WebGLRenderingContextBase>(m_context.get()) in the cached m_hasRelevantWebGLEventListener boolean to have one less check to do here?

> Source/WebCore/html/HTMLCanvasElement.cpp:980
> +#if ENABLE(WEBGL)
> +    m_hasRelevantWebGLEventListener = hasEventListeners(eventNames().webglcontextchangedEvent)
> +        || hasEventListeners(eventNames().webglcontextlostEvent)
> +        || hasEventListeners(eventNames().webglcontextrestoredEvent);
> +#endif

We’re caching this because this is too expensive to do inside virtualHasPendingActivity?

[Chris Dumez]

Comment on attachment 395192 [details]
Patch

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

>> Source/WebCore/html/HTMLCanvasElement.cpp:963
>> +    }
> 
> Why not include is<WebGLRenderingContextBase>(m_context.get()) in the cached m_hasRelevantWebGLEventListener boolean to have one less check to do here?

I like having the is<WebGLRenderingContextBase>() check in this function since I am doing a downcast<WebGLRenderingContextBase>() right after. I think doing it as you suggest would probably look less safe.

>> Source/WebCore/html/HTMLCanvasElement.cpp:980
>> +#endif
> 
> We’re caching this because this is too expensive to do inside virtualHasPendingActivity?

2 reasons:
- Performance
- virtualHasPendingActivity() gets called on a GC thread.

[Chris Dumez]

Seems to be causing some crashes on the bots for some reason. I am investigating.

[Chris Dumez]

Created attachment 395202 [details]
Patch

[EWS]

Committed r259364: <https://trac.webkit.org/changeset/259364>

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