Skip to content

GC stress mode#6681

Merged
erikcorry merged 5 commits into
mainfrom
erikcorry/gc-stress-mode
Apr 30, 2026
Merged

GC stress mode#6681
erikcorry merged 5 commits into
mainfrom
erikcorry/gc-stress-mode

Conversation

@erikcorry

Copy link
Copy Markdown
Contributor

The new mode calls GC just after microtasks and at awaitIO continuations

This catches some cases where an unfortunately timed GC leads
to intermittent issues. Running the just-recipe reveals current
failing tests, but the security issue that it unearthed has been
fixed. Some of the failures are just timing.

Not currently enabled in the CI.

@erikcorry erikcorry requested review from a team as code owners April 28, 2026 06:57

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm an AI code reviewer. Findings may contain errors; please verify before acting on them.

Summary: Adds a GC stress test mode that forces full V8 GC at microtask and awaitIo boundaries, with Bazel test variants and just-recipes.

Findings (1 item):

  1. [LOW] Freelist bypass in wrappable.c++ is not #ifndef NDEBUG-gated, unlike the GC injection points in io-context.c++.

Comment thread src/workerd/jsg/wrappable.c++
@ask-bonk

ask-bonk Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

I'm Bonk, and I've done a quick review of your PR.

Summary: Adds a GC stress test mode (--gc-stress / WORKERD_GC_STRESS=1) that forces full V8 GC after microtasks and at awaitIo continuations, with Bazel test variants and just-recipes.

One low-severity finding posted as an inline comment:

  1. [LOW] The freelist bypass in src/workerd/jsg/wrappable.c++:184 is not #ifndef NDEBUG-gated, while the GC injection points in io-context.c++ are. In release builds, this means the freelist skip has cost (extra cppgc allocations) but no benefit (GC isn't actually forced at continuations). Suggested wrapping it in #ifndef NDEBUG for consistency, or adding a comment explaining the intent.

Overall the implementation is clean and well-documented. The thread-scope flag follows the established predictableMode pattern, the Bazel variant generation is properly tagged off-by-default, and the static const env-var caching in isGcStressModeForTest() is a nice touch for subprocess use cases. No backward compatibility, security, or memory safety concerns.

github run

@github-actions

github-actions Bot commented Apr 28, 2026

Copy link
Copy Markdown

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@erikcorry erikcorry force-pushed the erikcorry/gc-stress-mode branch from 6e8df83 to f7fd973 Compare April 29, 2026 16:01
@erikcorry erikcorry force-pushed the erikcorry/gc-stress-mode branch from f7fd973 to 56f0376 Compare April 30, 2026 09:23
@erikcorry erikcorry merged commit 74ad743 into main Apr 30, 2026
22 of 23 checks passed
@erikcorry erikcorry deleted the erikcorry/gc-stress-mode branch April 30, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants