1854907 - Crash in [@ nsHtml5TreeOpExecutor::FlushDocumentWrite]

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/d7607403-020f-45c9-8504-407880230924

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(!mReadingFromStage) (Got doc write flush when reading from stage)

Top 10 frames of crashing thread:

0  xul.dll  nsHtml5TreeOpExecutor::FlushDocumentWrite  parser/html/nsHtml5TreeOpExecutor.cpp:786
1  xul.dll  nsHtml5Parser::Parse  parser/html/nsHtml5Parser.cpp:406
2  xul.dll  mozilla::dom::Document::WriteCommon  dom/base/Document.cpp:10023
3  xul.dll  mozilla::dom::Document::Write  dom/base/Document.cpp:10033
3  xul.dll  mozilla::dom::Document_Binding::write  dom/bindings/DocumentBinding.cpp:3715
4  xul.dll  mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>  dom/bindings/BindingUtils.cpp:3327
5  xul.dll  CallJSNative  js/src/vm/Interpreter.cpp:486
5  xul.dll  js::InternalCallOrConstruct  js/src/vm/Interpreter.cpp:580
5  xul.dll  InternalCall  js/src/vm/Interpreter.cpp:647
5  xul.dll  js::CallFromStack  js/src/vm/Interpreter.cpp:652

Not sure what to make of this, the stacks contain a lot of JavaScript so it's hard to tell what's going on, but we've got some volume coming into the nightly channel and that's an alarm bell. No comments on the crashes but five of them happened on this page: https://www.pokellector.com

Comment 1

[Henri Sivonen (:hsivonen)]

(In reply to Gabriele Svelto [:gsvelto] from comment #0)

Not sure what to make of this, the stacks contain a lot of JavaScript so it's hard to tell what's going on

This much seems clear:

1. The HTML parser is in the state where the main-thread part does not expect there to be any parser-blocking script (i.e. non-async, non-defer classic script) running.
2. The main-thread part of the HTML parser is expecting to receive data from the off-the-main-thread part without data accumulating in a speculation.
3. The above points should mean that the document object is in a state where a call to document.write never reaches the parser. That is, the document.write should either be ignored or should imply a call to document.open, which would blow away the document and its off-the-main-thread parser and would create a new main-thread-only parser.

The combination of 1 and 3 is a bug somewhere. Item 2 is merely an explanation of the assertion wording.

but we've got some volume coming into the nightly channel and that's an alarm bell.

The most likely cause is bug 1846178. Marking it as the regressor as an educated guess without strict proof.

No comments on the crashes but five of them happened on this page: https://www.pokellector.com

From the stack frame https://hg.mozilla.org/mozilla-central/file/7c6496f92bb0572d56513d645f3d6201f77a5653/dom/script/ScriptLoader.cpp#l2551 we see that we're not executing a module script, so the handling of classic scripts broke.

The stack frame https://hg.mozilla.org/mozilla-central/file/7c6496f92bb0572d56513d645f3d6201f77a5653/dom/script/ScriptLoader.cpp#l2102 strongly hints at the problem script being an async script.

My best current guess is that the movement of the line sele->SetCreatorParser(GetParser()); to happen before the branch into async and non-async is the cause and the effect is that the new found creator parser for async scripts causes the document object to be prepared to receive a normal document.write when it shouldn't.

Comment 2

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1854907 - Make document.write-related operations on the creator parser no-ops on scripts whose type does not allow document.write.

Comment 3

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&revision=f2199bcf9811c2a9eb2eb0b564016a9883c8c16f

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1846178

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f762ae010c6
Make document.write-related operations on the creator parser no-ops on scripts whose type does not allow document.write. r=smaug

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/7f762ae010c6

Comment 7

[Henri Sivonen (:hsivonen)]

Comment on attachment 9354855 [details]
Bug 1854907 - Make document.write-related operations on the creator parser no-ops on scripts whose type does not allow document.write.

Beta/Release Uplift Approval Request

• User impact if declined: Scripts that do a thing that they aren't supposed to (a script that isn't a classic non-async, non-defer script calls document.write()) cause a content process to terminate (safely). (The regressor already rode the trains to beta.)
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is low-risk, because the cause is understood and the fix is simple. The alternative of not uplifting this seems much riskier.
• String changes made/needed: None
• Is Android affected?: Yes

Comment 8

[Dianna Smith [:diannaS]]

Comment on attachment 9354855 [details]
Bug 1854907 - Make document.write-related operations on the creator parser no-ops on scripts whose type does not allow document.write.

Approved for 119.0b4

Comment 9

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/9e1350906cc1