1718074 - Crash in [@ (anonymous namespace)::nsAvailableMemoryWatcher::LowMemoryCallback]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Sean Feng [:sefeng]]

Crash report: https://crash-stats.mozilla.org/report/index/77fd58f7-832c-40c7-be6b-fdcc20210624

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 8 frames of crashing thread:

0 xul.dll static `anonymous namespace'::nsAvailableMemoryWatcher::LowMemoryCallback xpcom/base/AvailableMemoryTracker.cpp:162
1 ntdll.dll RtlpTpWaitCallback 
2 ntdll.dll TppExecuteWaitCallback 
3 ntdll.dll TppWaitCompletion 
4 ntdll.dll TppWorkerThread 
5 kernel32.dll BaseThreadInitThunk 
6 ntdll.dll _RtlUserThreadStart 
7 ntdll.dll _RtlUserThreadStart 

Comment 1

[Toshihito Kikuchi [:toshi]]

I got what happened. This time nsAvailableMemoryWatcher was shut down while running ListenForLowMemory resulting in AV when dereferencing mTimer.

Comment 2

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1718074 - Early return from the callback when the object was shut down. r=gsvelto

nsAvailableMemoryWatcher::mTimer was nullptr when LowMemoryCallback() tried to
initialize a timer via OnLowMemory(). There are two possible reasons.

The first case is that NS_NewTimer() returned nullptr because the available memory was
already low when initializing nsAvailableMemoryWatcher. In this case, we should not
register the callback.

The second case is the low-memory callback was queued while shutting down
nsAvailableMemoryWatcher (just before we unregiter the callback). We do refcount
the object and use the mutex correctly, but we touch the nulled out member. We should
make an early return if the object was shut down.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13f1b6e39989
Early return from the callback when the object was shut down.  r=gsvelto

Comment 4

[Andrew McCreight [:mccr8]]

I see similar crashes on older branches.

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/13f1b6e39989

Comment 6

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

The patch landed in nightly and beta is affected.
:toshi, 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 7

[Toshihito Kikuchi [:toshi]]

Comment on attachment 9228824 [details]
Bug 1718074 - Early return from the callback when the object was shut down. r=gsvelto

Beta/Release Uplift Approval Request

• User impact if declined: Firefox may crash when the low-memory callback was triggered in a limited timing window during startup or shutdown.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The fix consists of two parts 1) not to register the callback when we failed to create a timer and 2) not to continue the callback function during shutdown, both of which are simple and easy to understand. The risk is low.
• String changes made/needed: None

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 9228824 [details]
Bug 1718074 - Early return from the callback when the object was shut down. r=gsvelto

approved for 90.0 rc1

Comment 9

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d61e348995cb