1864989 - Crash in [@ JS_SetGCParameter]

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P3)

Description

[BugBot [:suhaib / :marco]]

Crash report: https://crash-stats.mozilla.org/report/index/277275e6-f738-49fb-b6aa-2bb280231026

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (cx->runtime()->gc.setParameter(cx, key, value))

Top 10 frames of crashing thread:

0  libxul.so  JS_SetGCParameter  js/src/jsapi.cpp:1410
0  libxul.so  SetGCParameter  dom/base/nsJSEnvironment.cpp:1930
0  libxul.so  SetMemoryPrefChangedCallbackInt  dom/base/nsJSEnvironment.cpp:1968
1  libxul.so  NotifyCallbacks  modules/libpref/Preferences.cpp:1958
2  libxul.so  NotifyCallbacks  modules/libpref/Preferences.cpp:1713
2  libxul.so  pref_SetPref  modules/libpref/Preferences.cpp:1916
3  libxul.so  mozilla::Preferences::SetInt  modules/libpref/Preferences.cpp:5128
4  libxul.so  nsPrefBranch::SetIntPref  modules/libpref/Preferences.cpp:2477
5  libxul.so  NS_InvokeByIndex  
6  libxul.so  CallMethodHelper::Invoke  js/xpconnect/src/XPCWrappedNative.cpp:1627

By querying Nightly crashes reported within the last 2 months, here are some insights about the signature:

• First crash report: 2023-10-26
• Process type: Parent
• Is startup crash: No
• Has user comments: No
• Is null crash: Yes - 9 out of 11 crashes happened on null or near null memory address

Comment 1

[Gabriele Svelto [:gsvelto]]

This is a diagnostic assert, so it's naturally low-volume. Given it's happening within the GC code there's a strong chance that it's caused by bad hardware. However without knowing the values involved it's hard to say.

Comment 2

[Nicolas B. Pierron [:nbp] {away}]

My guess is that this could be some bad about:config user input, such as not allocating any helper thread for JS, or being out of range.

From what I understand, removing the diagnostic assertion might be the easiest way forward given that the parameters are not modified.
I do not think we have any mean to prevent bad perf values.

Also, it might be worth checking that we do not cause persistent start-up crashes after modifying the preference.
If so we should raise the priority & severity of this bug.

Comment 3

[Jon Coppeard (:jonco)]

The way way handle error checking for GC parameters is pretty flawed. I've previously filed bug 1854611 and bug 1742118. We should probably remove this assertion until we fix this.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1864989 - Remove diagnostic assertion when setting invalid GC parameter values r?sfink

Since MOZ_ALWAYS_TRUE uses MOZ_DIAGNOSTIC_ASSERT on failure (TIL) this causes
crashes in nightly builds when setting bad parameter values. We already have
bugs open for the general issue.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60c35d4b7fc7
Remove diagnostic assertion when setting invalid GC parameter values r=sfink

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/60c35d4b7fc7