1745139 - Crash in [@ nsHtml5TreeOpExecutor::CommitToInternalEncoding]

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

4 crashes from 4 different Nightly installations, starting with 97.0a1 20211208214306.

Crash report: https://crash-stats.mozilla.org/report/index/824789e2-8312-4dbb-b5c5-cd9230211209

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll nsHtml5TreeOpExecutor::CommitToInternalEncoding parser/html/nsHtml5TreeOpExecutor.cpp:836
1 xul.dll nsHtml5EncodingCommitter::Run parser/html/nsHtml5StreamParser.cpp:205
2 xul.dll mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:144
3 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:771
4 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1183
5 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:85
6 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:324
7 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:306
8 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
9 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:603

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1745139 - Check for termination even after the first flush loop in CommitToInternalEncoding.

Comment 2

[Henri Sivonen (:hsivonen)]

The crash reports show lots of extensions. I expect this to be an extension interaction that simply doesn't show up in CI of with my set of extensions.

Comment 3

[Rob Wu [:robwu]]

(In reply to Henri Sivonen (:hsivonen) from comment #2)

The crash reports show lots of extensions. I expect this to be an extension interaction that simply doesn't show up in CI of with my set of extensions.

Are you referring to https://crash-stats.mozilla.org/report/index/824789e2-8312-4dbb-b5c5-cd9230211209#tab-extensions as "lots of extensions"?
That's not significant. Most of it are search engines or built-in extensions. Only AdBlock Plus and DuckDuckGo Privacy Essentials are third-party (and also "Firefox Voice Beta", which is no longer supported by the way).

In the (existing) code around the area touched in your patch I see references about the parser having been destroyed by an extension via a http observer. Extensions cannot synchronously run code any more, so that's probably not really relevant.

In relation to the parser, the extension framework could block the parser at
https://searchfox.org/mozilla-central/rev/4ca2f73ae9346709d39de2c8fe33874e4203b9e6/toolkit/components/extensions/ExtensionContent.jsm#536-550
triggered via ExtensionPolicyService::CheckDocument at initial-document-element-inserted, and http-on-opening-request and document-on-opening-request. At initial-document-element-inserted, it is possible for the parser to briefly be blocked if the content script is still loading, e.g. in about:blank.

Comment 4

[Henri Sivonen (:hsivonen)]

(In reply to Rob Wu [:robwu] from comment #3)

(In reply to Henri Sivonen (:hsivonen) from comment #2)

The crash reports show lots of extensions. I expect this to be an extension interaction that simply doesn't show up in CI of with my set of extensions.

Are you referring to https://crash-stats.mozilla.org/report/index/824789e2-8312-4dbb-b5c5-cd9230211209#tab-extensions as "lots of extensions"?

Yes, that has multiple extensions, but there were other reports with even more.

In the (existing) code around the area touched in your patch I see references about the parser having been destroyed by an extension via a http observer. Extensions cannot synchronously run code any more, so that's probably not really relevant.

That comment was copied from code added in the era of old-style extensions.

In relation to the parser, the extension framework could block the parser at
https://searchfox.org/mozilla-central/rev/4ca2f73ae9346709d39de2c8fe33874e4203b9e6/toolkit/components/extensions/ExtensionContent.jsm#536-550
triggered via ExtensionPolicyService::CheckDocument at initial-document-element-inserted, and http-on-opening-request and document-on-opening-request. At initial-document-element-inserted, it is possible for the parser to briefly be blocked if the content script is still loading, e.g. in about:blank.

It sure looks like something that the installations submitting these crashes have but my setup doesn't terminates the parser from within the first flush loop in nsHtml5TreeOpExecutor::CommitToInternalEncoding, which wasn't supposed to happen.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b6d857e2b7a
Check for termination even after the first flush loop in CommitToInternalEncoding. r=smaug

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4b6d857e2b7a