Bug 164883

Summary: ASSERTION FAILED: hasParserBlockingScript() seen with js/dom/modules/module-will-fire-beforeload.html
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, esprehn+autocc, gyuyoung.kim, hyatt, mmaxfield, rniwa, saam, sam, simon.fraser, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Crash log
none
Patch rniwa: review+

Description Ryan Haddad 2016-11-17 12:32:13 PST
Created attachment 295069 [details]
Crash log

Seen here with js/dom/modules/module-will-fire-beforeload.html (crash was attributed to js/kde/Array.html during the test run)

https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r208850%20(9494)/results.html

ASSERTION FAILED: hasParserBlockingScript()
/Volumes/Data/slave/elcapitan-debug/build/Source/WebCore/html/parser/HTMLScriptRunner.cpp(177) : void WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad(WebCore::PendingScript &)
1   0x10abe0440 WTFCrash
2   0x10dfb1391 WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad(WebCore::PendingScript&)
3   0x10ded2776 WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&)
4   0x10ded27df non-virtual thunk to WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&)
5   0x10f18af5a WebCore::PendingScript::notifyClientFinished()
6   0x10f18af89 WebCore::PendingScript::notifyFinished(WebCore::LoadableScript&)
7   0x10ee50865 WebCore::LoadableScript::notifyClientFinished()
8   0x10eaed0bc WebCore::LoadableModuleScript::notifyFinished(WebCore::CachedModuleScript&)
9   0x10eaed0ff non-virtual thunk to WebCore::LoadableModuleScript::notifyFinished(WebCore::CachedModuleScript&)
10  0x10e0c7d73 WebCore::CachedModuleScript::notifyClientFinished()
11  0x10e0c7ccb WebCore::CachedModuleScript::notifyLoadCompleted(WTF::UniquedStringImpl&)
12  0x10f6b7cda WebCore::ScriptController::setupModuleScriptHandlers(WebCore::CachedModuleScript&, JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0::operator()(JSC::ExecState*) const
13  0x10f6b7c60 long long std::__1::__invoke_void_return_wrapper<long long>::__call<WebCore::ScriptController::setupModuleScriptHandlers(WebCore::CachedModuleScript&, JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0&, JSC::ExecState*>(WebCore::ScriptController::setupModuleScriptHandlers(WebCore::CachedModuleScript&, JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0&&&, JSC::ExecState*&&)
14  0x10f6b7a8c std::__1::__function::__func<WebCore::ScriptController::setupModuleScriptHandlers(WebCore::CachedModuleScript&, JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0, std::__1::allocator<WebCore::ScriptController::setupModuleScriptHandlers(WebCore::CachedModuleScript&, JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0>, long long (JSC::ExecState*)>::operator()(JSC::ExecState*&&)
15  0x10a655e7b std::__1::function<long long (JSC::ExecState*)>::operator()(JSC::ExecState*) const
16  0x10a655b45 JSC::runStdFunction(JSC::ExecState*)
17  0x3a8214001028
18  0x10a75a965 llint_entry
19  0x10a75322e vmEntryToJavaScript
20  0x10a521b0c JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
21  0x10a49bb7f JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
22  0x109c9726e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
23  0x109c974bb JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
24  0x10a627736 JSC::JSJobMicrotask::run(JSC::ExecState*)
25  0x10e4cbd57 WebCore::JSMainThreadExecState::runTask(JSC::ExecState*, JSC::Microtask&)
26  0x10e60d0ea WebCore::JSDOMWindowMicrotaskCallback::call()
27  0x10e5f1a9d WebCore::JSDOMWindowBase::queueTaskToEventLoop(JSC::JSGlobalObject const*, WTF::Ref<JSC::Microtask>&&)::$_0::operator()()
28  0x10e5f19ec WTF::Function<void ()>::CallableWrapper<WebCore::JSDOMWindowBase::queueTaskToEventLoop(JSC::JSGlobalObject const*, WTF::Ref<JSC::Microtask>&&)::$_0>::call()
29  0x10d2bb2e3 WTF::Function<void ()>::operator()() const
30  0x10d2bb14b WebCore::ActiveDOMCallbackMicrotask::run()
31  0x10f0543a6 WebCore::MicrotaskQueue::performMicrotaskCheckpoint()
LEAK: 279 WebCoreNode
LEAK: 18 CachedResource
Comment 1 Radar WebKit Bug Importer 2016-11-17 12:45:37 PST
<rdar://problem/29317243>
Comment 2 Ryan Haddad 2016-11-17 12:52:42 PST
LayoutTest js/dom/modules/module-will-fire-beforeload.html was added with https://trac.webkit.org/changeset/208788
Comment 3 Yusuke Suzuki 2016-11-17 13:09:14 PST
Look into it once returning my office
Comment 4 Yusuke Suzuki 2016-11-17 14:03:39 PST
Interesting. I found the obvious bug. HTMLDocumentParser uses watchForLoad for non m_pendingScript thing. This should cause this ASSERT crash.
I think this bug exists from when we introduce <script defer> first... Idk. I need to investigate more :)
Comment 5 Yusuke Suzuki 2016-11-17 14:49:49 PST
Ah, no. I think this is due to module's issue.
Comment 6 Yusuke Suzuki 2016-11-17 15:53:34 PST
watchForLoad invariant seems correct.
My guess is that, HTMLDocumentParser is detached.
Comment 7 Yusuke Suzuki 2016-11-17 16:09:25 PST
finishJSTest() will be called in beforeload timing.
At that time, we finally call some of stopParsing() or detach().
But we may still register a deferred script.
Comment 8 Yusuke Suzuki 2016-11-17 16:36:43 PST
(In reply to comment #0)
> Created attachment 295069 [details]
> Crash log
> 
> Seen here with js/dom/modules/module-will-fire-beforeload.html (crash was
> attributed to js/kde/Array.html during the test run)
> 
> https://build.webkit.org/results/
> Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r208850%20(9494)/results.html
> 
> ASSERTION FAILED: hasParserBlockingScript()
> /Volumes/Data/slave/elcapitan-debug/build/Source/WebCore/html/parser/
> HTMLScriptRunner.cpp(177) : void
> WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad(WebCore::
> PendingScript &)
> 1   0x10abe0440 WTFCrash
> 2   0x10dfb1391
> WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad(WebCore::
> PendingScript&)
> 3   0x10ded2776
> WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&)
> 4   0x10ded27df non-virtual thunk to
> WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&)
> 5   0x10f18af5a WebCore::PendingScript::notifyClientFinished()
> 6   0x10f18af89
> WebCore::PendingScript::notifyFinished(WebCore::LoadableScript&)
> 7   0x10ee50865 WebCore::LoadableScript::notifyClientFinished()
> 8   0x10eaed0bc
> WebCore::LoadableModuleScript::notifyFinished(WebCore::CachedModuleScript&)
> 9   0x10eaed0ff non-virtual thunk to
> WebCore::LoadableModuleScript::notifyFinished(WebCore::CachedModuleScript&)
> 10  0x10e0c7d73 WebCore::CachedModuleScript::notifyClientFinished()
> 11  0x10e0c7ccb
> WebCore::CachedModuleScript::notifyLoadCompleted(WTF::UniquedStringImpl&)
> 12  0x10f6b7cda
> WebCore::ScriptController::setupModuleScriptHandlers(WebCore::
> CachedModuleScript&, JSC::JSInternalPromise&,
> WebCore::DOMWrapperWorld&)::$_0::operator()(JSC::ExecState*) const
> 13  0x10f6b7c60 long long std::__1::__invoke_void_return_wrapper<long
> long>::__call<WebCore::ScriptController::setupModuleScriptHandlers(WebCore::
> CachedModuleScript&, JSC::JSInternalPromise&,
> WebCore::DOMWrapperWorld&)::$_0&,
> JSC::ExecState*>(WebCore::ScriptController::
> setupModuleScriptHandlers(WebCore::CachedModuleScript&,
> JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0&&&,
> JSC::ExecState*&&)
> 14  0x10f6b7a8c
> std::__1::__function::__func<WebCore::ScriptController::
> setupModuleScriptHandlers(WebCore::CachedModuleScript&,
> JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0,
> std::__1::allocator<WebCore::ScriptController::
> setupModuleScriptHandlers(WebCore::CachedModuleScript&,
> JSC::JSInternalPromise&, WebCore::DOMWrapperWorld&)::$_0>, long long
> (JSC::ExecState*)>::operator()(JSC::ExecState*&&)
> 15  0x10a655e7b std::__1::function<long long
> (JSC::ExecState*)>::operator()(JSC::ExecState*) const
> 16  0x10a655b45 JSC::runStdFunction(JSC::ExecState*)
> 17  0x3a8214001028
> 18  0x10a75a965 llint_entry
> 19  0x10a75322e vmEntryToJavaScript
> 20  0x10a521b0c JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
> 21  0x10a49bb7f JSC::Interpreter::executeCall(JSC::ExecState*,
> JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue,
> JSC::ArgList const&)
> 22  0x109c9726e JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType,
> JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
> 23  0x109c974bb JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason,
> JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue,
> JSC::ArgList const&)
> 24  0x10a627736 JSC::JSJobMicrotask::run(JSC::ExecState*)
> 25  0x10e4cbd57 WebCore::JSMainThreadExecState::runTask(JSC::ExecState*,
> JSC::Microtask&)
> 26  0x10e60d0ea WebCore::JSDOMWindowMicrotaskCallback::call()
> 27  0x10e5f1a9d
> WebCore::JSDOMWindowBase::queueTaskToEventLoop(JSC::JSGlobalObject const*,
> WTF::Ref<JSC::Microtask>&&)::$_0::operator()()
> 28  0x10e5f19ec WTF::Function<void
> ()>::CallableWrapper<WebCore::JSDOMWindowBase::queueTaskToEventLoop(JSC::
> JSGlobalObject const*, WTF::Ref<JSC::Microtask>&&)::$_0>::call()
> 29  0x10d2bb2e3 WTF::Function<void ()>::operator()() const
> 30  0x10d2bb14b WebCore::ActiveDOMCallbackMicrotask::run()
> 31  0x10f0543a6 WebCore::MicrotaskQueue::performMicrotaskCheckpoint()
> LEAK: 279 WebCoreNode
> LEAK: 18 CachedResource

notifyFinished just checks `if (isStopping())`.
So if the document is detached / stopped, deferred script will go to the completely wrong path.
Comment 9 Yusuke Suzuki 2016-11-17 16:39:16 PST
whatwg issue about deferred script and unloading.
https://github.com/whatwg/html/issues/2058
Comment 10 Ryan Haddad 2016-11-29 15:35:57 PST
Marked test as flaky on mac-wk2 debug in http://trac.webkit.org/projects/webkit/changeset/209097
Comment 11 Yusuke Suzuki 2016-12-10 22:20:09 PST
Created attachment 296854 [details]
Patch

WIP
Comment 12 Yusuke Suzuki 2016-12-10 22:21:10 PST
Comment on attachment 296854 [details]
Patch

WIP. I tested the crashed one in GTK build, but I cannot reproduce it. I'll test it in macOS. If I cannot reproduce it, I'll upload this patch as r?, this patch is "attempt to fix".
Comment 13 Yusuke Suzuki 2016-12-12 00:21:13 PST
Comment on attachment 296854 [details]
Patch

Let's attempt to fix.
Comment 14 Saam Barati 2016-12-12 17:06:15 PST
Comment on attachment 296854 [details]
Patch

LGTM, but I don't know this code very well. I'll allow someone else with more knowledge to r+
Comment 15 Yusuke Suzuki 2016-12-12 18:42:03 PST
Ryosuke, could you take a look?
I would like to watch the bot status (I think this will fix the issue) after landing this patch.
Comment 16 Yusuke Suzuki 2016-12-13 22:22:51 PST
Committed r209791: <http://trac.webkit.org/changeset/209791>