test(fetch): isolate REQUEST_HANDLES guard probe

[proggeramlug]

Summary

• serialize the fetch unit tests that use the process-global request registry on a dedicated test-only mutex
• keep the guard-release probe meaningful while preventing sibling tests from causing false failures
• distinguish a busy registry from a poisoned registry in the assertion diagnostics
• leave production code and version metadata unchanged

Fixes #9267.

Validation

• cargo fmt -p perry-ext-fetch -- --check
• cargo test -p perry-ext-fetch --lib (15 passed)
• cargo test --release -p perry-ext-fetch --lib (15 passed)
• 1,000 debug and 1,000 release runs of the full unit-test binary with --test-threads=64
• sabotage check: a deliberately forgotten REQUEST_HANDLES guard made request_reads_release_the_registry_guard fail at the first post-call probe; the sabotage was then reverted

Summary by CodeRabbit

• Bug Fixes

  • Improved test reliability for request handling by preventing interference between parallel test runs.
  • Enhanced diagnostics when request-registry lock states indicate contention or poisoning.

• Documentation

  • Added changelog documentation describing the improved request-registry test isolation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8beee86c-1ab8-4c8b-a726-76b67c05cc4a

📥 Commits

Reviewing files that changed from the base of the PR and between 6c880be and 1e259af.

📒 Files selected for processing (2)

• changelog.d/9280-fetch-request-registry-test-isolation.md
• crates/perry-ext-fetch/src/tests.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds a test-only mutex for shared REQUEST_HANDLES access. Request tests hold the mutex for their full lifetime. The guard-release probe now distinguishes lock contention from poisoning. A changelog entry documents the isolation.

Changes

Request registry test isolation

Layer / File(s)	Summary
Add test isolation guard crates/perry-ext-fetch/src/tests.rs	Adds a poison-tolerant process-global mutex and applies it to tests that access REQUEST_HANDLES.
Preserve and clarify guard-release checks crates/perry-ext-fetch/src/tests.rs, changelog.d/9280-fetch-request-registry-test-isolation.md	Keeps the guard-leak probe isolated from sibling tests, reports WouldBlock and poisoned states separately, and documents the change.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 1e259

This change serializes tests that share a request registry without altering shipped request handling or runtime behavior. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: isolating the REQUEST_HANDLES guard probe in fetch tests.
Description check	✅ Passed	The description provides a clear summary, concrete changes, related issue, and detailed validation commands and results. It does not reproduce every template heading, but it contains the required info…
Linked Issues check	✅ Passed	The changes satisfy issue #9267. They serialize tests that access REQUEST_HANDLES, preserve detection of genuine same-thread guard leaks, and distinguish busy and poisoned lock states in diagnostics…
Out of Scope Changes check	✅ Passed	The changes remain within the linked issue scope. The test-only mutex, probe diagnostics, and changelog entry support the stated fix, while production code and version metadata remain unchanged.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (1 skipped: 1 u…

Full details: Description check

Explanation

The description provides a clear summary, concrete changes, related issue, and detailed validation commands and results. It does not reproduce every template heading, but it contains the required information and is mostly complete.

Full details: Linked Issues check

Explanation

The changes satisfy issue #9267. They serialize tests that access REQUEST_HANDLES, preserve detection of genuine same-thread guard leaks, and distinguish busy and poisoned lock states in diagnostics.

Full details: Docstring Coverage

Explanation

Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Audited the diff — correct fix, and importantly it does not weaken the invariant. I'd merge.

The isolation mutex is the right shape: it serializes the tests that reach REQUEST_HANDLES while leaving production storage and locking untouched. With the guard held, a failing try_lock() can only mean this test's reader leaked — which is exactly the property #9267 was about, and exactly what the old probe could not distinguish.

Two details I checked specifically:

The assertion still fires on a real leak. Err(WouldBlock) still panics. The probe was not softened into a warning or a skip, which is the usual way a flake 'fix' quietly removes coverage.

The new Poisoned arm is an improvement, not a loophole. Previously try_lock().is_ok() collapsed would-block and poisoned into one failure that blamed a named reader for either. Now each reports its own cause. A poisoned mutex still panics, so nothing is swallowed.

unwrap_or_else(|poisoned| poisoned.into_inner()) on the test lock is also right: one panicking test should not cascade into unrelated failures in siblings, and the isolation lock carries no state worth protecting.

The comment requiring every js_request_* / request_body_* / store_request caller to hold the guard is worth keeping accurate as tests are added — it is the kind of convention that silently decays. Not a blocker.

For context on why this mattered beyond tidiness: cargo-test is in full-suite-gate's needs, so this flake could red a release run at random, and a full tier is ~3.5 h.

[proggeramlug]

Merged.

Validated on a shared branch with #9228, #9257, #9263, #9271, #9272, #9274, #9277, #9279 and #9280 — one build, one validation pass, then split back out and merged individually.

Results across the batch:

• perry-runtime 2885 passed / 0 failed at RUST_TEST_THREADS=1
• perry-codegen 30 suites green (the one failure was a doc-test reporting a missing libperry_codegen-*.rlib — an artifact of my own cleanup of stale build dirs, confirmed by a clean re-run at 31/31, not a code defect)
• all 60 lint gates plus the check_thread_locals / tls_budget checkers
• a nine-row differential probe byte-identical to node 26.5.1, covering every changed area: RegExp \w/\b/. ASCII and LineTerminator semantics, offset (a[i±1]) and length-bounded array reads, the assert RegExp matcher, and closure identity across a 40k-allocation GC churn
• seven earlier regression probes re-run at zero diff lines: tagged and scalar array stores, pointer↔scalar transition churn, growth-forwarding receivers, BigInt negation, iterator protocols, field shadowing

One probe (protorepl) moved from 2 to 4 diff lines and I ran it down rather than waving it through: both divergences are accepted trades already on main — the fresh-instance case (#9239) and #9247's deliberate change of a custom-chain miss from Some(undefined) to None, which it made because swallowing the miss left everything Perry synthesizes unreachable (Object(true).valueOf(), plain-function .prototype, iterator helpers). Neither is anything in this batch.