fix(gc-tests): suppress automatic GC triggers around raw-pointer forwarding setup - #7283
Merged
proggeramlug merged 1 commit intoAug 3, 2026
Conversation
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe changes add automatic-GC trigger suppression guards and pointer-liveness comments to runtime-root tests. The changelog documents the guarded allocation cases and the opaque-helper exception. ChangesGC test safety
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
…arding setup prototype_addr_cache.rs and side_table_scanners.rs hold raw GC pointers (nursery/old-gen addresses in native locals, no root registered) across further arena_alloc_gc / arena_alloc_gc_old calls while hand-staging forwarding chains. If one of those allocations lands on the block-full slow path, arena_cell_alloc's gc_check_trigger() can run a real collection and move/free the held object out from under the test. Apply the sibling pattern already used in callback_scanners.rs, hook_dispatch_handles.rs, and transient_handles.rs: GcTriggerThresholdTestGuard::suppress_automatic_triggers() at every exposed call site. Sites that are not exposed carry a comment explaining why, rather than a blanket guard.
jdalton
force-pushed
the
fix/gc-test-helper-trigger-suppression
branch
from
August 3, 2026 05:03
c109480 to
73d8d68
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
prototype_addr_cache.rsandside_table_scanners.rs(both undercrates/perry-runtime/src/gc/tests/runtime_roots/) hand-stage GC forwardingchains by allocating with
arena_alloc_gc/arena_alloc_gc_oldand holdingthe raw returned pointer/address in a native local — with no root
registered — while further allocations happen. Those further allocations can
land on the block-full slow path (
arena_cell_alloc), which callsgc_check_trigger(). Without suppression, an automatic collection there canrelocate or free the held object out from under the test before it finishes
staging the forwarding chain.
Neither file used the guard its siblings (
callback_scanners.rs,hook_dispatch_handles.rs,transient_handles.rs) already rely on for thisexact hazard:
GcTriggerThresholdTestGuard::suppress_automatic_triggers().This PR adds it at every exposed call site in both files.
Enumerated call sites
prototype_addr_cache.rsnursery_stand_in()—arena_alloc_gcevacuate()—arena_alloc_gc_oldfromforwarded_pair()— viaevacuate()from, live in this frame across the call intoevacuateevacuate's own guard)prototype_addr_reads_through_a_forwarding_stub(test)forwarded_pair()already returns plainusizes and carries its own guardprototype_addr_reads_through_a_multi_hop_forwarding_chain(test) — 2ndarena_alloc_gc, thenarena_alloc_gc_oldfirstlive across the 2ndarena_alloc_gc;first+secondlive acrossarena_alloc_gc_oldprototype_addr_cache_is_rewritten_by_the_collector(test) — 2ndnursery_stand_in(), then twoevacuate()callsarray_fromlive across the 2ndnursery_stand_in()callprototype_addr_cache_scanner_is_registered,prototype_addr_cache_scanner_leaves_the_unset_sentinel_alonearena_alloc_gc*callsside_table_scanners.rstest_implicit_this_root_scanner_marks_and_rewritesnursery_userlive acrossarena_alloc_gc_oldtest_class_side_table_scanner_marks_values_but_not_function_keysdynamic_value/prototype_value/cached_value/prototype_objectlive across botharena_alloc_gccalls (parent_closure,function_key);parent_closurelive acrossfunction_key's calltest_registered_class_side_table_scanner_rewrites_values_and_function_keysvalue_userlive acrosskey_user'sarena_alloc_gc; both (+value_old) live across the twoarena_alloc_gc_oldcallstest_symbol_side_table_scanner_marks_keys_and_values_without_marking_ownerjs_object_alloc,alloc_nursery_test_symbol,young_leaf) with no literalarena_alloc_gc*call written in this filetest_symbol_side_table_registered_scanner_rewrites_roots_and_metadataowner/sym_key/value/static_sym_key/static_value(+_oldaddresses) live across the threearena_alloc_gc_oldcallstest_runtime_root_visitor_rewrites_raw_pointer_slotsnursery_userlive acrossarena_alloc_gc_oldtest_class_inheritance_side_table_roots_mark_and_rewriteproto_user/decl_proto_user/closure_user(+_oldaddresses) live across the laterarena_alloc_gc/arena_alloc_gc_oldcallstest_runtime_root_visitor_rewrites_cell_and_atomic_slotsnursery_userlive acrossarena_alloc_gc_oldtest_runtime_root_visitor_rewrites_metadata_without_markingnursery_userlive acrossarena_alloc_gc_oldNote:
young_leaf,alloc_nursery_test_symbol,alloc_old_test_symbol, andjs_object_alloc(shared helpers, mostly insupport.rs) have the sameunderlying shape but are out of scope here — they're used far beyond these
two files, and this PR is scoped to the two files' own literal
arena_alloc_gc/arena_alloc_gc_oldcall sites per the task that opened it.Flagging for awareness, not fixing inline.
Test plan
cargo test -p perry-runtime runtime_roots— 77 passed, 0 failedcargo test -p perry-runtime --lib -- --test-threads=1on this branch: 1645 passed, 0 failed, 3 ignoredgc::tests::teardown,pty,object::prop_plan,object::global_this_webassembly), confirming they're pre-existing test-isolation flakiness unrelated to this change, not a regressioncargo fmt --check -p perry-runtimecleanSummary by CodeRabbit
Tests
Documentation