1610246 - Crash in [@ mozilla::ScriptPreloader::CacheWriteComplete]

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P1)

Description

[[:philipp]]

[Tracking Requested - why for this release]:
This bug is for crash report bp-70abda3a-9f3c-467d-8f22-ac0250200119.

Top 10 frames of crashing thread:

0 xul.dll void mozilla::ScriptPreloader::CacheWriteComplete js/xpconnect/loader/ScriptPreloader.cpp:750
1 xul.dll nsresult mozilla::detail::RunnableMethodImpl< xpcom/threads/nsThreadUtils.h:1176
2 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1250
3 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
4 xul.dll void mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:110
5 xul.dll void MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
6 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
7 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
8 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:406
9 xul.dll nsresult nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:276

this crash signature is spiking up since yesterday 2020-01-19 from users of zh-cn builds. it's happening across platforms and multiple release channels are affected.

Comment 1

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

kmag, does this ring any bells?
Is it possible that we somehow call InvalidateCache while invalidation is already in-progress and thus get
CacheWriteComplete call after another CacheWriteComplete?

Comment 2

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

We've had this crash for quite some time.

Comment 3

[Julien Cristau [:jcristau]]

There's a new spike (again on zh-cn) since yesterday.

Comment 4

[Hector Zhao [:hectorz]]

Sorry I didn't pay much attention last time.

I think these crashes come from extensions by Beijing office, several of which have code like 1 after they were removed in bug 1445739.

We released new versions of two or more such extensions on both Jan. 19th and yesterday. Our release of an update to a single extension on Feb. 25th doesn't seem to trigger this.

Would you please advise me on how best to fix this? Thanks!

Comment 5

[Hector Zhao [:hectorz]]

(In reply to Hector Zhao [:hectorz] from comment #4)

We released new versions of two or more such extensions on both Jan. 19th and yesterday. Our release of an update to a single extension on Feb. 25th doesn't seem to trigger this.

We shipped our Fx 76 compat extension fixes earlier today, another surge in crash volumes might be coming.

Comment 6

[[:philipp]]

indeed, there are already a thousand new reports today.

Comment 7

[Hector Zhao [:hectorz]]

(In reply to Hector Zhao [:hectorz] from comment #5)

We shipped our Fx 76 compat extension fixes earlier today, another surge in crash volumes might be coming.

And moments ago for Fx 77 compat fixes.

Comment 8

[Julien Cristau [:jcristau]]

Do we know why these cause firefox crashes? Can we do something to avoid that?

Comment 9

[Hector Zhao [:hectorz]]

(In reply to Olli Pettay [:smaug] from comment #1)

Is it possible that we somehow call InvalidateCache while invalidation is already in-progress and thus get
CacheWriteComplete call after another CacheWriteComplete?

IIUC, the above mentioned situation is happening, because more than one of our extensions call Services.obs.notifyObservers(null, "startupcache-invalidate"); at roughly the same time during their updates.

(In reply to Julien Cristau [:jcristau] from comment #8)

Do we know why these cause firefox crashes?

See above.

Can we do something to avoid that?

I'm not really sure. Maybe we could try to update only one of our extensions at a time, but that's clearly not a real fix.

Or maybe bug 1445739 could be partially reverted, so that the startup cache is still invalidated if any extension with priviledged signatures is being updated, so that I don't have to do it from individual extensions?

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

There is a large spike of crashes on 77.0.1 since yesterday for zh-CN users, already 2000 crash reports, Hector can this be acted upon rapidly? Thanks

Comment 11

[Hector Zhao [:hectorz]]

(In reply to Pascal Chevrel:pascalc from comment #10)

There is a large spike of crashes on 77.0.1 since yesterday for zh-CN users, already 2000 crash reports, Hector can this be acted upon rapidly? Thanks

It's somewhat expected with our Fx 78 compat updates, similar to comment 5 and comment 7. The spike will most likely be gone w/o any intervention, maybe tomorrow. I understand it's not ideal, I'll try to "update only one of our extensions at a time" next time, as mentioned in comment 9.

In the long term, we do want to restructure our extensions to move more logic into proper WebExt experiment API implementations, but it's unclear to me whether that will eliminate the need to call Services.obs.notifyObservers(null, "startupcache-invalidate"); on our own.

I don't really know but maybe it's best to make Fx not crash even when it's called repeatedly?

Comment 12

[Hector Zhao [:hectorz]]

(In reply to Hector Zhao [:hectorz] from comment #11)

... I understand it's not ideal, I'll try to "update only one of our extensions at a time" next time, as mentioned in comment 9.

FYI, we're shipping our Fx 79 compat updates, but started with just one of them. We'll enable updates for one another extension after 24 hrs, and so on. Hopefully this will prevent more spikes here.

Comment 13

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

We got another crash spike with this signature. Can and should anything be done here (e.g. roll out only one extension update)?

Comment 14

[Hector Zhao [:hectorz]]

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #13)

We got another crash spike with this signature. Can and should anything be done here (e.g. roll out only one extension update)?

Hi, we're already shipping our updates separately with an interval, and the spike is much smaller than the late June one.

I still think the best fix would be:

(In reply to Hector Zhao [:hectorz] from comment #11)

......

I don't really know but maybe it's best to make Fx not crash even when it's called repeatedly?

Comment 15

[Kris Maglione [:kmag]]

Attachment: Bug 1610246: Don't spawn a second save thread when one already exists. r=mccr8

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbcef6181e20
Don't spawn a second save thread when one already exists. r=mccr8

Comment 17

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/fbcef6181e20

Comment 18

[Hector Zhao [:hectorz]]

Thank you!