Skip to content

script: Disable incremental garbage collection - #46316

Merged
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:disable-incremental-gc
Jul 6, 2026
Merged

script: Disable incremental garbage collection#46316
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:disable-incremental-gc

Conversation

@mrobinson

@mrobinson mrobinson commented Jul 6, 2026

Copy link
Copy Markdown
Member

The SpiderMonkey 137 upgrade inadvertently enabled incremental garbage
collection. Previously it was preffed on, but disabled via the
DisableIncrementalGC() API which unconditionally disabled it no matter what
JSGCParamKey::JSGC_INCREMENTAL_GC_ENABLED was set to. The upgrade replaced
DisableIncrementalGC() with setting JSGCParamKey::JSGC_INCREMENTAL_GC_ENABLED
to 0, but a later call set it to the value of the preference (true). This
change removes the duplicate call and sets the default value fo the preference
to false.

Testing: This leads to very timing specific memory corruption errors, so
making a reliable test for it is very difficult. Regardless, we should see
improvements in fuzzing results.

@mrobinson
mrobinson requested a review from gterzian as a code owner July 6, 2026 18:52
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 6, 2026
@mrobinson
mrobinson force-pushed the disable-incremental-gc branch 2 times, most recently from ce9da7e to d5940f7 Compare July 6, 2026 19:00
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jul 6, 2026

@simonwuelker simonwuelker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a silly bug.

Comment on lines +282 to +284
/// Whether or not incremental garbage collection is turned on. This is currently
/// turned off by default as pre-barriers are not implemented yet. If turned on, it
/// will likely lead to memory corruption.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a link to #7621.

The SpiderMonkey 137 inadvertently enabled incremental garbage
collection. Previously it was preffed on, but disabled via the
`DisableIncrementalGC(0` API which unconditionally disabled it no matter
what `JSGCParamKey::JSGC_INCREMENTAL_GC_ENABLED` was set to. The upgrade
replaced `DisableIncrementalGC()` with setting
`JSGCParamKey::JSGC_INCREMENTAL_GC_ENABLED` to 0, but a later call set
it to the value of the preference (`true`). This change removes the
duplicate call and sets the default value fo the preference to `false`.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
@mrobinson
mrobinson force-pushed the disable-incremental-gc branch from d5940f7 to cdf1cb3 Compare July 6, 2026 19:34
@mrobinson
mrobinson enabled auto-merge July 6, 2026 19:35
@mrobinson
mrobinson added this pull request to the merge queue Jul 6, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 6, 2026
@jdm

jdm commented Jul 6, 2026

Copy link
Copy Markdown
Member

Good lord.

Merged via the queue into servo:main with commit 2e3c896 Jul 6, 2026
34 checks passed
@mrobinson
mrobinson deleted the disable-incremental-gc branch July 6, 2026 21:25
@Narfinger

Copy link
Copy Markdown
Contributor

Is there an issue on what needs to be implemented for incremental garbage collection?

@jdm

jdm commented Jul 7, 2026

Copy link
Copy Markdown
Member

#7621

jschwe pushed a commit that referenced this pull request Jul 18, 2026
The SpiderMonkey 137 upgrade inadvertently enabled incremental garbage
collection. Previously it was preffed on, but disabled via the
`DisableIncrementalGC()` API which unconditionally disabled it no matter
what
`JSGCParamKey::JSGC_INCREMENTAL_GC_ENABLED` was set to. The upgrade
replaced
`DisableIncrementalGC()` with setting
`JSGCParamKey::JSGC_INCREMENTAL_GC_ENABLED`
to 0, but a later call set it to the value of the preference (`true`).
This
change removes the duplicate call and sets the default value fo the
preference
to `false`.

Testing: This leads to *very* timing specific memory corruption errors,
so
making a reliable test for it is very difficult. Regardless, we should
see
improvements in fuzzing results.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
(cherry picked from commit 2e3c896)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants