1823657 - Crash in [@ shutdownhang | mozilla::IdentityCredentialStorageService::WaitForInitialization | mozilla::IdentityCredentialStorageService::DeleteFromOriginAttributesPattern]

Status: RESOLVED FIXED

(Core :: DOM: Credential Management, defect)

Description

[Donal Meehan [:dmeehan]]

Crash report: https://crash-stats.mozilla.org/report/index/6e20bd7e-537e-4e48-81bf-8f1240230321

MOZ_CRASH Reason: Shutdown hanging at step AppShutdownTeardown. Something is blocking the main-thread.

Top 10 frames of crashing thread:

0  ntdll.dll  KiFastSystemCallRet  
1  ntdll.dll  NtWaitForKeyedEvent  
2  ntdll.dll  RtlSleepConditionVariableSRW  
3  kernel32.dll  SleepConditionVariableSRW  
4  mozglue.dll  mozilla::detail::ConditionVariableImpl::wait  mozglue/misc/ConditionVariable_windows.cpp:50
5  xul.dll  mozilla::OffTheBooksCondVar::Wait  xpcom/threads/CondVar.h:58
5  xul.dll  mozilla::Monitor::Wait  xpcom/threads/Monitor.h:37
5  xul.dll  mozilla::IdentityCredentialStorageService::WaitForInitialization  toolkit/components/credentialmanagement/IdentityCredentialStorageService.cpp:198
6  xul.dll  mozilla::IdentityCredentialStorageService::DeleteFromOriginAttributesPattern  toolkit/components/credentialmanagement/IdentityCredentialStorageService.cpp:719
7  xul.dll  NS_InvokeByIndex  

So far 333 crashes all on windows

Comment 1

[Donal Meehan [:dmeehan]]

See Bug 1823655 also, not sure if this both should be tracked under the same bug

Comment 2

[Donal Meehan [:dmeehan]]

Added a needinfo on Bug 1823655
https://bugzilla.mozilla.org/show_bug.cgi?id=1823655#c1

Comment 3

[Donal Meehan [:dmeehan]]

Settings 111 to wontfix, as 112 RC builds next week.

Comment 4

[Dianna Smith [:diannaS]]

marking as this disabled since the crashes should technically not occur in 112 with the landing of D174488 in Bug 1823655. The actual fix remains in 113 (D174212 in Bug 1823655)

Comment 5

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

The severity field is not set for this bug.
:bvandersloot, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

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

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 20 desktop browser crashes on release

:bvandersloot, could you consider increasing the severity of this top-crash bug?

For more information, please visit auto_nag documentation.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

We're still seeing crashes from Beta 113, i.e. https://crash-stats.mozilla.org/report/index/682a49af-6279-4090-8cba-d748b0230421

Comment 8

[Ryan VanderMeulen [:RyanVM]]

We landed the same disabling patch from bug 1823655 for 113 that landed back in comment 4 for 112. And if I understand correctly, we're hoping that bug 1825749 resolves these crashes for 114+. We can probably close this out next cycle if Beta114 is looking good.

Comment 9

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

Based on the topcrash criteria, the crash signature linked to this bug is not a topcrash signature anymore.

For more information, please visit BugBot documentation.

Comment 10

[Tim Huang[:timhuang]]

Bug 1825749 disables cleaning credential identity storage during shutdown if the feature is not enabled(The default setting). This will stop this crash, but we don't fix the root cause. So, the crash will happen again once we enable this feature. I suggest we keep this bug open for investigating and fixing the rool cause.

Comment 11

[Benjamin VanderSloot [:bvandersloot]]

I don't have a windows machine to repro this on, however I have a likely root cause and will validate it in a Treeherder run.

This service has some logic to prevent initialization during shutdown. This prevents deadlocks like this where we wait for initialization. This was using ShutdownPhase::AppShutdown. It looks like Windows is hitting this code in ShutdownPhase::AppShutdownTeardown, which is before ShutdownPhase::AppShutdown according to https://searchfox.org/mozilla-central/source/xpcom/base/ShutdownPhase.h.

The correct solution to fix that issue would be to use ShutdownPhase::AppShutdownConfirmed as the first phase in which we don't spin up this service, rather than ShutdownPhase::AppShutdown.

Comment 12

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1823657 - Include earlier stages of shutdown when preventing initialization during shutdown of IdentityCredentialStorageService, r=timhuang!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3cff6aaf6bd
Include earlier stages of shutdown when preventing initialization during shutdown of IdentityCredentialStorageService, r=timhuang

Comment 14

[Natalia Csoregi [:nataliaCs]]

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

Comment 15

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

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 16

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

The patch landed in nightly and beta is affected.
:bvandersloot, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox114 to wontfix.

For more information, please visit BugBot documentation.