1424220 - Crash in nsHtml5TreeOpExecutor::RunScript

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P2)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-32d12c86-4e43-4fc0-91d8-789160171208.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll nsHtml5TreeOpExecutor::RunScript parser/html/nsHtml5TreeOpExecutor.cpp:731
1 xul.dll nsHtml5TreeOperation::Perform parser/html/nsHtml5TreeOperation.cpp:986
2 xul.dll nsHtml5TreeOpExecutor::RunFlushLoop parser/html/nsHtml5TreeOpExecutor.cpp:492
3 xul.dll nsHtml5ExecutorReflusher::Run parser/html/nsHtml5TreeOpExecutor.cpp:56
4 xul.dll mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:396
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1037
6 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:97
7 xul.dll mozilla::ipc::MessagePumpForChildProcess::Run ipc/glue/MessagePump.cpp:301
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:319
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:299

=============================================================

this crash signature seems to be regressing across platforms in 58 with "MOZ_RELEASE_ASSERT(mFlushState == eNotFlushing) (Tried to run script while flushing.)"

Comment 1

[Henri Sivonen (:hsivonen)]

So the parser has decided that the script is either a defer script or an async script, but after we've created a DOM element for the script and QIed it to nsIScriptElement, both GetScriptDeferred() and GetScriptAsync() returned false.

It would be great to have steps to reproduce...

Comment 2

[Henri Sivonen (:hsivonen)]

Fortunately, the crash is very low volume. Still, it's odd that all the crashes are on Windows. One should expect this to be a cross-platform crash.

Comment 3

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #2)
> Fortunately, the crash is very low volume. Still, it's odd that all the
> crashes are on Windows. One should expect this to be a cross-platform crash.

Maybe the MOZ_CRASH signature is different elsewhere?

Comment 4

[Henri Sivonen (:hsivonen)]

Single Linux crash with the same assertion but lacking stack trace:
https://crash-stats.mozilla.com/report/index/9f485da0-11cd-4851-b6c6-282500171204

Comment 5

[Henri Sivonen (:hsivonen)]

Note to self:
mCurrentHtmlScriptIsAsyncOrDefer is not part of snapshotted tree builder state on the assumption that snapshots are taken always at the end of a non-defer, non-async script.

But should it always be set to false when restoring a snapshot?

Comment 6

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1424220 - Set mCurrentHtmlScriptIsAsyncOrDefer to false when restoring tree builder state.

Review commit: https://reviewboard.mozilla.org/r/212976/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212976/

Comment 7

[Henri Sivonen (:hsivonen)]

The deletions of blank lines in the patch are artifacts of clang-format.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8942710 [details]
Bug 1424220 - Set mCurrentHtmlScriptIsAsyncOrDefer to false when restoring tree builder state.

https://reviewboard.mozilla.org/r/212976/#review218798

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1565e46c8479
Set mCurrentHtmlScriptIsAsyncOrDefer to false when restoring tree builder state. r=smaug

Comment 10

[Henri Sivonen (:hsivonen)]

https://hg.mozilla.org/projects/htmlparser/rev/efc0c1019018bf7433d0e1553052c21cb471aafc
Mozilla bug 1424220 - Set mCurrentHtmlScriptIsAsyncOrDefer to false when restoring tree builder state. r=smaug.

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1565e46c8479

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Crash volume for 58 doesn't look high enough to warrant consideration as a dot release ride-along, so calling this wontfix for 58. Feel free to set it back to affected and nominate for mozilla-release approval if you feel strongly otherwise, however.