Bug 209863 - ASSERTION FAILED: m_wrapper on webgl/max-active-contexts-webglcontextlost-prevent-default.html
Summary: ASSERTION FAILED: m_wrapper on webgl/max-active-contexts-webglcontextlost-pre...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-01 10:48 PDT by Alexey Proskuryakov
Modified: 2020-04-01 14:51 PDT (History)
13 users (show)

See Also:


Attachments
Patch (6.34 KB, patch)
2020-04-01 12:26 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (5.75 KB, patch)
2020-04-01 14:10 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2020-04-01 10:48:56 PDT
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()
Comment 1 Radar WebKit Bug Importer 2020-04-01 10:49:09 PDT
<rdar://problem/61164936>
Comment 2 Chris Dumez 2020-04-01 10:55:37 PDT
Which revision?
Comment 3 Alexey Proskuryakov 2020-04-01 11:01:12 PDT
r259292
Comment 4 Chris Dumez 2020-04-01 11:29:13 PDT
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.
Comment 5 Chris Dumez 2020-04-01 11:33:22 PDT
The provided command reproduced the issue for me, thanks.
Comment 6 Chris Dumez 2020-04-01 12:26:40 PDT
Created attachment 395192 [details]
Patch
Comment 7 Darin Adler 2020-04-01 13:29:42 PDT
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?
Comment 8 Chris Dumez 2020-04-01 13:43:50 PDT
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.
Comment 9 Chris Dumez 2020-04-01 13:50:22 PDT
Seems to be causing some crashes on the bots for some reason. I am investigating.
Comment 10 Chris Dumez 2020-04-01 14:10:39 PDT
Created attachment 395202 [details]
Patch
Comment 11 EWS 2020-04-01 14:51:23 PDT
Committed r259364: <https://trac.webkit.org/changeset/259364>

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