Skip to content

fix(gc): preserve request graphs across route imports - #8044

Open
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/8036-next-request-import-rooting
Open

fix(gc): preserve request graphs across route imports#8044
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/8036-next-request-import-rooting

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Summary

  • exclude a module own exported classes from the foreign polymorphic-class fallback, preserving native-backed derived constructor initialization
  • root imported exported-const closures while request arguments are evaluated
  • root the complete detached expired-timer batch across callback dispatch and microtask checkpoints

Regression coverage

  • add a generated-route-shaped re-export fixture with a NextRequest-shaped class, URL, readonly URLSearchParams subclass, Headers, and async body state
  • exercise sync and async handlers through allocation, dynamic import, promise and timer continuations, and explicit moving GC
  • mirror the production verifier with 20 concurrent GET requests plus one POST body request

Testing

  • cargo test -p perry-runtime test_timer_tick_roots_the_complete_detached_expired_batch -- --nocapture
  • cargo test -p perry-codegen --lib
  • cargo fmt --all -- --check
  • python3 scripts/check_test_registration.py
  • Node oracle vs Perry under normal GC: byte-identical, 22 output lines
  • Node oracle vs Perry under forced and verified evacuation: byte-identical; 6,601 live objects copied

Closes #8036

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 seconds

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d970290b-f150-4457-9902-66578360b97e

📥 Commits

Reviewing files that changed from the base of the PR and between 971d6ff and 42bccf6.

📒 Files selected for processing (12)
  • changelog.d/8044-next-request-import-rooting.md
  • crates/perry-codegen/src/lower_call/extern_func.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots/hook_dispatch_handles.rs
  • crates/perry-runtime/src/timer.rs
  • crates/perry/src/commands/compile/run_pipeline.rs
  • test-files/fixtures/gc_next_request_import/lazy_work.ts
  • test-files/fixtures/gc_next_request_import/request.ts
  • test-files/fixtures/gc_next_request_import/route.ts
  • test-files/fixtures/gc_next_request_import/route_impl.ts
  • test-files/test_gap_gc_next_request_import.ts
  • test-parity/gc_repsel_corpus.txt
✨ 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

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

@proggeramlug proggeramlug left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Audited exact head 42bccf62b05911446c614c6c72103d850cfcaac5.

The three source fixes look well-scoped: imported exported-const closures are rooted during argument evaluation, timer ticks root the complete detached expired batch across callbacks/microtasks, and a module's own exported classes are excluded from foreign polymorphic-class augmentation. The focused timer test and the new normal and forced/verified-evacuation parity fixture pass locally.

I am holding the merge because this head says Closes #8036 but does not satisfy that issue's required acceptance gate. #8036 requires the pinned #8034 production AppRouteRouteModule.handle integration, including 20 concurrent GETs and POSTs, normal and forced evacuation, and no direct-handler fallback. The added request-shaped compiler fixture is synthetic and does not run that production route-module path.

Please either add/run the exact production integration gate, or narrow this to a non-closing partial fix. I can re-audit the updated exact head.

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.

[Next.js/dylib] Preserve NextRequest and nextUrl.searchParams across App Route imports

1 participant