1806064 - Crash in [@ nsQueryObject<T>::operator()] with string bundles

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Pascal Chevrel:pascalc]

Crash report: https://crash-stats.mozilla.org/report/index/7e9bbefb-973a-4503-8207-b62260221215

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0  xul.dll  nsQueryObject<nsIStringBundle>::operator const  xpcom/base/nsQueryObject.h:24
0  xul.dll  RefPtr<  xpcom/base/nsCOMPtr.h:1475
0  xul.dll  nsStringBundleService::RegisterContentBundle  intl/strres/nsStringBundle.cpp:835
1  xul.dll  mozilla::dom::ContentChild::RecvRegisterStringBundles  dom/ipc/ContentChild.cpp:2369
2  xul.dll  mozilla::dom::PContentChild::OnMessageReceived  ipc/ipdl/PContentChild.cpp:10988
3  xul.dll  mozilla::ipc::MessageChannel::DispatchAsyncMessage  ipc/glue/MessageChannel.cpp:1756
3  xul.dll  mozilla::ipc::MessageChannel::DispatchMessage  ipc/glue/MessageChannel.cpp:1681
3  xul.dll  mozilla::ipc::MessageChannel::RunMessage  ipc/glue/MessageChannel.cpp:1481
3  xul.dll  mozilla::ipc::MessageChannel::MessageTask::Run  ipc/glue/MessageChannel.cpp:1579
4  xul.dll  mozilla::RunnableTask::Run  xpcom/threads/TaskController.cpp:539

Comment 1

[Pascal Chevrel:pascalc]

Crashes started with Build 20221215092759
Changelog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7ff758e0d08b4bdf0ce3bd2abe84a1bff3be8ff4&tochange=061ba69417ebfdcb275f01049f09a893004c5587

Comment 2

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

use-after-free in the crash address: 0xe5e5e5e5e5e5e5e5

Comment 3

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

[@ mozilla::dom::WorkerRunnable::Run] variant: bp-48d4a7d3-ee36-4bcf-aecb-467250221216

Comment 4

[Yulia Startsev [:yulia]]

It looks like the issue might be that the StringBundle isn't initialized before an error occurs. But that only explains the second variant, not the first. I am not sure how the worker modules changes impacted the first, as that happens on the content process and not on the worker process or the main thread while doing worker work. In particular, it looks like an issue when opening PDF files -- which suggests that the pdf worker might be triggering it... but the error still comes from content. I'll ask around.

Comment 5

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

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

Comment 6

[Andrew McCreight [:mccr8]]

Here's an example of the string bundle crash on a poison value: bp-bbd28d65-3982-480a-981e-927520221216

Comment 7

[Andrew McCreight [:mccr8]]

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #3)

[@ mozilla::dom::WorkerRunnable::Run] variant: bp-48d4a7d3-ee36-4bcf-aecb-467250221216

Should there maybe be a new bug filed for this instead? It does seem like it probably has the same regressor, but the stack looks completely different.

Comment 8

[Andrew McCreight [:mccr8]]

Bug 1805387 in the regression range involves both PDFs and strings, which is suspicious, but the actual patch is a CSS tweak to avoid windows overlapping so I can't see how it could be related.

One comment says "Cannot open PDF files."

Comment 9

[Yulia Startsev [:yulia]]

The part of the worker module code that is causing this issue is most likely https://phabricator.services.mozilla.com/D163239 -- I added this because we cannot use the string bundle cross thread otherwise, and the module loader crashes without it if it fails on the worker side. However, I have little insight to how this works, and I don't know if :kmag is still on until the end of the year. I'll cc him here and maybe we can take a look together. If this is severe, i have no qualms about pulling the module loader back out if necessary. I've been keeping this rebased for a half year anyway.

Kmag, you helped yoshi with the stringbundle stuff related to worklets, ccing you here in case you have any idea. To me it looks like I am calling it in the wrong place, either too early or too late.

Comment 10

[Yulia Startsev [:yulia]]

(shouldn't have cleared the needinfo for me here, i might forget to come back to this otherwise)

Comment 11

[Andrew McCreight [:mccr8]]

Via the signature spikes report, here's what looks like another string bundle issue. As with the other one, a decent chunk are UAFs. bp-50a47787-801c-47ab-ab06-17de60221216

Comment 12

[Andrew McCreight [:mccr8]]

..and another, [@ nsContentUtils::FormatLocalizedString ]: bp-5e6f3da4-b23d-4c24-96ed-580c30221216

Comment 13

[Gabriele Svelto [:gsvelto]]

This one too: https://crash-stats.mozilla.org/report/index/e8213bc7-dc2e-4280-98c6-4ec930221217

Comment 14

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

The bug is linked to topcrash signatures, which match the following criteria:

• Top 10 desktop browser crashes on nightly
• Top 10 AArch64 and ARM crashes on nightly

For more information, please visit auto_nag documentation.

Comment 15

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

The bug is marked as tracked for firefox110 (nightly). However, the bug still isn't assigned.

:jstutte, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 16

[Jens Stutte [:jstutte]]

(In reply to Andrew McCreight [:mccr8] from comment #7)

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #3)

[@ mozilla::dom::WorkerRunnable::Run] variant: bp-48d4a7d3-ee36-4bcf-aecb-467250221216

Should there maybe be a new bug filed for this instead? It does seem like it probably has the same regressor, but the stack looks completely different.

Yeah, that seems to be a different issue. Cracking up https://crash-stats.mozilla.org/report/index/570e2142-c5cb-4712-9630-00c3e0221218 shows me that apparently we try to run an already freed WorkerRunnable while doing a sync loop and fail on reading its mWorkerPrivate member, which sounds weird.

Comment 17

[Jens Stutte [:jstutte]]

For now this is mitigated by backout as of bug 1247687 comment 77.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Yes, and bug 1806390 should fix the stringbundle crashes.

Comment 19

[Andrew McCreight [:mccr8]]

I confirmed that the crashes have gone away, so I'll mark this fixed.

Comment 21

[Yulia Startsev [:yulia]]

I will try to reland the changes now that emilios changes are in. I'll monitor the crashes over the next few days.