1636925 - Crash in [@ shutdownhang | mozilla::SpinEventLoopUntil<T> | mozilla::Preferences::SavePrefFileBlocking]

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Pascal Chevrel:pascalc]

This bug is for crash report bp-16cc2eb5-a183-4505-852f-aea1c0200511.

Top 10 frames of crashing thread:

0 ntdll.dll NtWaitForAlertByThreadId 
1 ntdll.dll RtlSleepConditionVariableSRW 
2 kernelbase.dll SleepConditionVariableSRW 
3 mozglue.dll mozilla::detail::ConditionVariableImpl::wait mozglue/misc/ConditionVariable_windows.cpp:50
4 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1120
5 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:481
6 xul.dll mozilla::SpinEventLoopUntil<mozilla::ProcessFailureBehavior::ReportToCaller, `lambda at /builds/worker/checkouts/gecko/modules/libpref/Preferences.cpp:3055:33'> xpcom/threads/nsThreadUtils.h:342
7 xul.dll mozilla::Preferences::SavePrefFileBlocking modules/libpref/Preferences.cpp:3827
8 xul.dll mozilla::Preferences::Observe modules/libpref/Preferences.cpp:3715
9 xul.dll nsObserverList::NotifyObservers xpcom/ds/nsObserverList.cpp:65

This is a new crash signature spiking on Windows since we merged to 77beta

Comment 1

[Andrew McCreight [:mccr8]]

It looks like preferences code is spinning the event loop during shutdown. It looks like bug 1630655 changed how we spin the event loop in preferences code, so maybe that's what caused this? It also might be a signature change. In other words, we might have been hanging in a shutdown event loop somewhere else.

Comment 2

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

Oof. Bug 1630655 changed PreferencesWriter::Flush to be more robust. Previously it could happen (and I suspect it often would happen) that PreferencesWriter::Flush would return before we had actually written preferences to disk. If this were to happen during shutdown, and PreferencesWriter::Write were to hang for some reason, then on Release, we would see hangs under the xpcom-shutdown-threads notification. On Nightly, we currently skip that notification due to the toolkit.shutdown.fastShutdownStage pref, so you would just see the possibility of data loss.

I don't think there's anything immediately actionable here. Unless we have some reason to believe that PreferencesWriter::Write is taking longer than it ought to, or we can convince ourselves that we don't need to save prefs during shutdown.

Comment 3

[Pascal Chevrel:pascalc]

So is the bug wontfix and these crashes on beta unavoidable?

Comment 4

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

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

So is the bug wontfix and these crashes on beta unavoidable?

I can't find anything I've overlooked; so yes, I believe so.

Comment 5

[Jens Stutte [:jstutte]]

Hi Doug, sure that we should not do something here? See also bug 1658871.

Comment 6

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

Hey Jens,

Yeah, I still believe that this is fundamentally a problem with the constraint of the preferences system, that we occasionally want to "Flush" it and ensure that what is on disk matches what is in memory. I cannot speak to this constraint; my work in this area was only to ensure that the code was actually delivering on its promise.

To give a bit more justification why I don't think this is a problem with SpinEventLoopUntil specifically, in both this report and the one linked in your linked bug, and in every report of this type I've seen, there is a thread running inside PreferencesWriter::Write, which means that we are in fact waiting simply for the write to complete, and it's not some logic problem with the counters it uses. Additionally, PreferencesWriter::Write doesn't wait on any mutexes or anything, so I can see no indication that there's some problem with tasks being reordered due to the SpinEventLoopUntil call.

Comment 7

[Jens Stutte [:jstutte]]

Looking at this again (coming from another bug), I see that all recent reports I looked at are stuck within Preferences::Write while creating a file through nsAtomicFileOutputStream::DoOpen(). Could it be that there are some flags for CreateFileW that make it wait if the file already exists or similar? Unfortunately due to optimizations it is hard to figure out, what are the effective flags we are composing here.

Comment 8

[Jens Stutte [:jstutte]]

It seems that the only affected version for this signature is ESR 102 these days. We still see PreferencesWriter::Flush in bug 1710018, though.