Skip to content

fix(test-isolation): the stream constructor tests read a side table the GC guards clear - #7671

Merged
proggeramlug merged 2 commits into
mainfrom
fix/stream-static-methods-side-table-race
Aug 9, 2026
Merged

fix(test-isolation): the stream constructor tests read a side table the GC guards clear#7671
proggeramlug merged 2 commits into
mainfrom
fix/stream-static-methods-side-table-race

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

fix(test-isolation): the stream constructor tests read a side table the GC guards clear.

stream_constructors_expose_static_method_values asserts a static method value is not TAG_UNDEFINED, and intermittently it was — assertion left != right failed, left: 9222246136947933185, right: 9222246136947933185, i.e. both sides 0x7FFC_0000_0000_0001.

CLOSURE_PROPS is a process-global Mutex<HashMap<usize, HashMap<String, f64>>> keyed by closure address, and the GC test guards' state reset (test_clear_closure_side_tables) clears it from whatever thread runs them. gc/tests/support.rs documents this in its own comment, and three tests in closure/dynamic_props.rs already take crate::gc::global_side_table_test_lock() for exactly this reason. The two native_module_stream tests did not.

0/16 on main, 1/12 on #7664's branch — the third time in two days that a branch has exposed a pre-existing global-sink race by changing the parallel schedule rather than introducing one (#7665 fixed the other two: opt_report and ext_registry). Verified 0/20 after.

The class is wider than this fix. A survey of files that read closure dynamic props in tests:

guarded file
3 of 4 closure/dynamic_props.rs
2 of 9 object/global_this_webassembly.rs
0 array/tests.rs (65), node_stream_tests.rs (42), node_submodules/tests.rs (33), object/instanceof.rs (5), value/to_string.rs (4), object/native_module/constants.rs (4), and ~14 more

Blanket-locking several hundred tests would serialise a large part of the suite for a hazard that only bites a test reading a persisted prop across a window, so this change fixes the observed instance and the exposure is filed with the survey rather than guessed at.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an intermittent test race that could cause stream static method lookups to return undefined.
    • Improved test isolation when garbage-collection checks run concurrently.
  • Documentation

    • Added a changelog entry describing the resolved stream test race.
  • Chores

    • Updated the application version from 0.5.1380 to 0.5.1381.

Ralph Küpper added 2 commits August 9, 2026 02:31
…he GC side-table reset

CLOSURE_PROPS is process-global and test_clear_closure_side_tables clears it
from parallel test threads, so a static method value read here came back
TAG_UNDEFINED. 0/16 on main, 1/12 on #7664's branch, 0/20 after.

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
@proggeramlug
proggeramlug merged commit db4a2d0 into main Aug 9, 2026
25 of 47 checks passed
@proggeramlug
proggeramlug deleted the fix/stream-static-methods-side-table-race branch August 9, 2026 00:32
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3a6518c1-f2d0-4f1f-8f35-d50454ea7167

📥 Commits

Reviewing files that changed from the base of the PR and between 3beef03 and 3f52134.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • CLAUDE.md
  • Cargo.toml
  • changelog.d/7671-stream-static-methods-side-table-race.md
  • crates/perry-runtime/src/object/native_module_stream.rs

📝 Walkthrough

Walkthrough

The PR serializes closure side-table access in two stream tests, documents the test-isolation race, and updates the workspace and documented versions to 0.5.1381.

Changes

Stream test isolation and release version

Layer / File(s) Summary
Serialize stream side-table access
crates/perry-runtime/src/object/native_module_stream.rs, changelog.d/7671-stream-static-methods-side-table-race.md
Two stream tests now acquire the global side-table test lock before reading closure properties. The changelog documents the race and affected tests.
Update release version metadata
Cargo.toml, CLAUDE.md
The workspace package version and documented current version change from 0.5.1380 to 0.5.1381.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested labels: bug

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/stream-static-methods-side-table-race

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

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

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.

1 participant