docs(gc-handoff): triage #7803 on a corpus that links again - #7989
Conversation
The zod dep-corpus links on main again (#7980), so #7803 is runnable for the first time since it was filed. Record what running it says: - #7803's own reproducer (seed 1, rate 1, quarantine off) no longer fails, but the class it reports does — 3 of 16 seeds fail, one with the same "Cannot read properties of undefined" shape. - The candidate cause on record (#7962/#7978, Object.defineProperti(es) rooting) is refuted by a sabotage A/B: reverting both fixes underneath current main does not bring the failure back. - Every failure is intermittent; a fixed seed does not replay. The reportable figure is the rate, not the seed. - Seed 15 is a separate, self-detecting bug: the #7645 pin latch aborts on a pinned young Map, and the FATAL's own suggested remediation (scripts/gc_pin_sites.py) reports OK. - Aside: PERRY_GC_DIAG=0 enables diagnostics (var_os(...).is_some()).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a changelog entry and a 263-line investigation note for zod corpus triage. It records reproduction results, intermittent scheduled-GC failures, diagnostic details, and unresolved rooting and pin-latch issues. No code changes were made. ChangesZod corpus triage
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
gc-handoff/ZOD-NOTES.md (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the reported Markdownlint findings.
Add language identifiers to the fenced blocks at Lines 13, 25, 36, 72, 104, 121, 171, and 192. Render
#7154as inline code or escape the hash at Line 110 so it is not parsed as a heading.Also applies to: 25-25, 36-36, 72-72, 104-104, 110-110, 121-121, 171-171, 192-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gc-handoff/ZOD-NOTES.md` at line 13, Update the fenced code blocks in ZOD-NOTES.md at the specified lines to include appropriate language identifiers, and format `#7154` as inline code or escape its hash so Markdownlint no longer interprets it as a heading.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gc-handoff/ZOD-NOTES.md`:
- Around line 116-132: The protected-arm discussion in the quarantine analysis
should not claim confirmation of the stale-pointer mechanism. In the section
describing PERRY_GC_PROTECT_FROMSPACE=1 and the shipped witness, replace
“confirmed” with “supports,” and characterize the result as mitigation evidence
while preserving that the rooting defect remains unlocalized.
- Around line 216-220: Update the rate discussion in the surrounding notes to
describe “3 of 16 runs failed” as an observed 18.75% sample rate, rather than
calling it durable. Clarify that the clean-sweep probability estimates assume
independent runs with a fixed underlying failure probability.
---
Nitpick comments:
In `@gc-handoff/ZOD-NOTES.md`:
- Line 13: Update the fenced code blocks in ZOD-NOTES.md at the specified lines
to include appropriate language identifiers, and format `#7154` as inline code or
escape its hash so Markdownlint no longer interprets it as a heading.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d9bf3281-d3bb-4977-8cd2-1b860cbb2582
📒 Files selected for processing (2)
changelog.d/7989-zod-corpus-triage.mdgc-handoff/ZOD-NOTES.md
| ### The quarantine still hides it, exactly as #7803 predicted | ||
|
|
||
| Same seed 4, quarantine ON at depth 800: **exit 0**, answer byte-exact, and the | ||
| instrument is saturated rather than absent — | ||
|
|
||
| ``` | ||
| [gc-fromspace-protect] mode=ProtectPages retired_set=#999 blocks=2 | ||
| sets_held=800/800 bytes_protected=2095054848 bytes_poisoned=0 blocks_recycled=398 | ||
| ``` | ||
|
|
||
| 6,888 retired sets, 2.09 GB held, and the run reaches 6,888 copying minors | ||
| instead of dying at 738. This is #7803's reading (1) confirmed on a second seed: | ||
| holding retired from-space pages out of Eden changes *which* addresses get | ||
| recycled and the vulnerable window stops lining up. The protected arm is | ||
| therefore not evidence of health, and the shipped witness | ||
| (`scripts/gc_dep_scale_witness.sh`, quarantine ON) cannot catch this class of | ||
| window on this workload. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not call the protected-arm result confirmation.
PERRY_GC_PROTECT_FROMSPACE=1 suppresses the observed failure and changes address reuse. This supports the quarantine hypothesis, but it does not confirm the stale-pointer mechanism. Section 7 states that the rooting defect remains unlocalized. Change “confirmed” to “supports” and describe this arm as mitigation evidence.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 121-121: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gc-handoff/ZOD-NOTES.md` around lines 116 - 132, The protected-arm discussion
in the quarantine analysis should not claim confirmation of the stale-pointer
mechanism. In the section describing PERRY_GC_PROTECT_FROMSPACE=1 and the
shipped witness, replace “confirmed” with “supports,” and characterize the
result as mitigation evidence while preserving that the rooting defect remains
unlocalized.
| one is the **rate**: at rate 1 with the quarantine off, **3 of 16 runs failed | ||
| (~19%)**. That is the number to A/B a candidate fix against, and 16 runs is a | ||
| thin sample for it — a fix claiming to close this needs a sweep wide enough that | ||
| 19%→0% is distinguishable from luck (at ~19%, a 16-run clean sweep is only | ||
| ~3% likely by chance; a 40-run clean sweep is ~0.02%). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Label 3/16 as an observed sample rate.
“The durable one is the rate” overstates what one 16-run sweep establishes. Use “3 of 16 runs failed (18.75% observed in this sweep)” instead. State that the clean-sweep probabilities assume independent runs with a fixed underlying failure probability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gc-handoff/ZOD-NOTES.md` around lines 216 - 220, Update the rate discussion
in the surrounding notes to describe “3 of 16 runs failed” as an observed 18.75%
sample rate, rather than calling it durable. Clarify that the clean-sweep
probability estimates assume independent runs with a fixed underlying failure
probability.
1 participant
Docs-only. The zod dependency corpus links again as of #7980, so #7803 was
runnable for the first time since it was filed. This records what running it
actually says, as
gc-handoff/ZOD-NOTES.md.Headlines:
test-files/gc-dep-corpus/main.tscompiles and links onbdfcba4a2(exit 0, 29.9 MB). No regeneration, no re-pin — the corpus and thezod@4.3.5pin are the ones already in the tree.at rate 1 with the quarantine off: 6/6 clean, answer byte-exact. A 16-seed
sweep in the same configuration: 3 failures, one of them
TypeError: Cannot read properties of undefined (reading 'issues')— the sameshape as the one filed.
ROOTVEC-NOTES.mdnamed fix(runtime): root JS values retained in Rust containers across allocations (#7949) #7962/fix(gc): root Object.defineProperty's receiver, key and descriptor fields across its own allocating calls #7978(
Object.defineProperties/Object.definePropertyrooting) as an unconfirmedcandidate. Reverting both underneath current
mainand rebuilding does notbring the failure back.
in the sweep and passes 2/2 on re-run; seed 15 aborts once and passes 3/3. The
reportable figure is the rate (3/16), not the seed.
Map, and the FATAL's own suggested remediation (scripts/gc_pin_sites.py)reports OK. Filed separately.
PERRY_GC_DIAG=0enables diagnostics (var_os(...).is_some()),which silently collapsed an A/B I was running.
No code change and deliberately no new gate: a gate for a ~19%-of-runs
intermittent failure would be red on a healthy tree often enough to train people
to ignore it.
No version bump (maintainer bumps at merge).
Summary by CodeRabbit