test(gap): #9552 fixture that fails unfixed; changelog fragment for #9565 - #9571
test(gap): #9552 fixture that fails unfixed; changelog fragment for #9565#9571proggeramlug wants to merge 1 commit into
Conversation
…9565 The #9552 fix landed via merge train #9569 with the first draft of its gap test. That draft — `await fetch()` in a plain async function plus `gc()` — passed 6/6 on the UNFIXED runtime: the collection it forces does not free the in-flight promise, so the gate could not fail. This is the fixture validated on both arms: three consumer shapes (`.then`, async arrow, async class method) start a request from a frame that has returned, `Symbol()` churn trips the malloc-count sweep, `RegExp` headers reuse freed 80-byte slots. Unfixed it hangs 4/4 (two of the three promises freed by malloc sweeps, then a stale resolve on a reused slot); fixed it prints node's `ok,ok,ok 6` 5/5. Also lands the changelog fragment for #9565, which the train did not carry. Claude-Session: https://claude.ai/code/session_01Bok4V8wzgNGmBeE4GPf7Up
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe changelog documents cross-thread promise pinning and provenance checks. The regression test keeps three fetch-based promise consumers in flight while allocation churn triggers garbage collection. ChangesCross-thread promise GC handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change strengthens regression coverage and documents an existing runtime fix without altering production behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description clearly explains the purpose, implementation, related issues, and validation results. It omits the template headings and explicit checklist commands, but it provides the required information in equivalent sections and is mostly complete. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Landed via merge train #9573 (rebase-merge, authorship preserved). |
1 participant
Follow-through for #9552 / #9565, which landed via merge train #9569 before this branch's last two commits.
Why
The gap test the train carried (
test_gap_9552_cross_thread_promise_survives_gc.ts, first draft) passes 6/6 on the unfixed runtime:await fetch()in a plain async function keeps the promise reachable from the awaiting frame, andgc()alone does not run the malloc sweep that freed the promise in the report. A gate that cannot fail is not a gate.What this lands
.then, async arrow, async class method) each start afetchagainst a local server that answers late, from a frame that has returned before any collection;Symbol()churn trips the malloc-count sweep (GC_NEXT_MALLOC_TRIGGER= 100k),RegExpheaders reuse freed 80-byte slots (the promise's size class). Unfixed: hangs 4/4 — an env-gated trace shows two of the three promisesFREEDby malloc sweeps atstate=0, thenSTALE js_promise_resolveon a slot now holding another object. Fixed:ok,ok,ok 65/5, byte-identical to node (gcis conditional).changelog.d/9565-cross-thread-promise-pin.md: the fragment for the fix itself; the train did not carry one.https://claude.ai/code/session_01Bok4V8wzgNGmBeE4GPf7Up
Summary by CodeRabbit
Bug Fixes
Tests