1656974 - Crash in [@ shutdownhang | RtlAcquireSRWLockExclusive | mozilla::scache::StartupCache::GetBuffer]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[[:philipp]]

[Tracking Requested - why for this release]:

This bug is for crash report bp-60f1d762-b1ef-4b9e-bacf-ca9510200803.

Top 10 frames of crashing thread:

0 ntdll.dll NtWaitForAlertByThreadId 
1 ntdll.dll RtlAcquireSRWLockExclusive 
2 xul.dll mozilla::scache::StartupCache::GetBuffer startupcache/StartupCache.cpp:840
3 xul.dll ReadCachedScript js/xpconnect/loader/mozJSLoaderUtils.cpp:25
4 xul.dll mozJSComponentLoader::ObjectForLocation js/xpconnect/loader/mozJSComponentLoader.cpp:758
5 xul.dll mozJSComponentLoader::Import js/xpconnect/loader/mozJSComponentLoader.cpp:1276
6 xul.dll mozilla::dom::module_getter::ModuleGetter dom/base/ChromeUtils.cpp:576
7 xul.dll InternalCall js/src/vm/Interpreter.cpp:640
8 xul.dll js::NativeGetExistingProperty js/src/vm/NativeObject.cpp:2325
9 xul.dll Interpret js/src/vm/Interpreter.cpp:3471

the volume of these shutdownhangs is jumping up noticably in firefox 80.0b - acoounting for ~2.75% of all browser crashes. this first started on nightly build 20200708094217 and therefore looks tied to the changes in bug 1627075.

Comment 1

[Julien Cristau [:jcristau]]

Doug can you take a look?

Comment 2

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1656974 - Turn off StartupCache when startup is finished r?froydnj

I don't think this will fully resolve the shutdown hangs associated
with this bug. The underlying problem is that writing the startup
cache during shutdown can take a long time. This seems like A Bad Thing,
and I have a mild leaning toward the thought that we shouldn't write the
startup cache at all during shutdown. However, I do think that trading
shutdown time regressions for startup time wins is probably good, so
I don't feel super comfortable making that change.

However, this change at least fixes the problem where we A) block the main
thread waiting for the write to finish, and B) are more likely to hit a
shutdown hang, since we concentrate all of the hang time in one phase,
rather than it being spread across the shutdown timeline. This is likely
the major change that the regressing bug introduced.

The unfortunate consequence is that if we try to GetBuffer during shutdown,
we will now read out of the omnijar. However, it should overall be better
since we'll be waiting on a (hopefully) small number of small reads rather
than a large write.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25e500b64866
Turn off StartupCache when startup is finished r=froydnj

Comment 4

[Alexandru Michis [:malexandru]]

Backed out changeset 25e500b64866 for causing failures in TestStartupCache

Backout link: https://hg.mozilla.org/integration/autoland/rev/12ca712fdf780170f813353b4074d227f1b82ed5

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=android%2C7.0%2Cx86-64%2Copt%2Ctest-android-em-7.0-x86_64%2Fopt-geckoview-gtest-1proc%2Cgtest&fromchange=b8dfd9c7a388a2a21a2d1652d6547b940a46f68d&tochange=d51942b1e2d80cb9bfb1e5e5f21ba141e38fe065&selectedTaskRun=a5P9nG3KQjOBLxPgmMltqw.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=312322271&repo=autoland&lineNumber=24240

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e15bd19f021
Turn off StartupCache when startup is finished r=froydnj

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5e15bd19f021

Comment 7

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

The patch landed in nightly and beta is affected.
:dthayer, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 8

[Alex Thayer [:alexical] (she/her)]

Trying to sort out what's going on in bug 1658732 before requesting uplift. That's a fairly strange regression to see from this change.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Bug 1627075 was backed out for 80.0rc2, so we'll hopefully see this go down in volume on Beta once that build starts rolling out.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)

Bug 1627075 was backed out for 80.0rc2, so we'll hopefully see this go down in volume on Beta once that build starts rolling out.

No reports from 80.0rc2 - looks like the backout worked. Calling this fixed for Fx80 by bug 1656261.