fix(compile): initialize the cycle partner of a dynamically imported module - #10279
proggeramlug wants to merge 1 commit into
Conversation
…module A module reached only through a dynamic `import()` that takes part in an import cycle never initialized its partner. The partner body did not run, so every export it assigns at run time stayed undefined and the first call through such a binding threw `TypeError: value is not a function`. `module_init_deps` drops init-call back-edges (PerryTS#6463) so an `__init` wrapper does not re-enter a cycle member the entry has already run. That is sound only because the entry emits an eager init call for every Eager module in topological order. Deferred modules are filtered out of that loop, so nothing else runs them: dropping a wrapper edge to a Deferred dep left it with no caller at all. Apply the positional drop to Eager deps only. This cannot perturb the ordering PerryTS#6463 fixed. A module statically imported by an Eager module is itself statically reachable from the entry and therefore Eager, so the new arm never fires for it. Inside a deferred cycle the existing `__perry_init_done_*` guard keeps the extra call idempotent and reproduces ESM order: the partner body runs first and the re-entrant call returns. The regression test pairs the defect with its control, the same cycle entered statically, which pins the PerryTS#6463 ordering the fix must not disturb. Fixes PerryTS#10278. Refs PerryTS#10107.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe compiler now retains initialization edges to Deferred modules when positional filtering would remove a cycle back-edge. A regression test verifies deferred cycle initialization, runtime-assigned exports, and existing Eager cycle ordering. The changelog documents the fix. ChangesDeferred cycle initialization
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant DynamicImport
participant ModuleInitWrapper
participant DeferredPartner
participant RuntimeExports
DynamicImport->>ModuleInitWrapper: start deferred module initialization
ModuleInitWrapper->>DeferredPartner: retain and call cycle back-edge
DeferredPartner->>RuntimeExports: assign exports during module body execution
RuntimeExports-->>DynamicImport: expose initialized exports
Merge Risk: ⚪ Minimal · up to Deferred cycle partners initialize correctly on dynamic import while existing static cycle ordering remains covered. No actionable merge-blocking risk is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
Landed via merge train #10284 (v0.5.1572). All source commits preserve authorship; merged main matches the validated train exactly. |
1 participant
Fixes #10278. Refs #10107 (OpenCode v1.18.30 native bring-up).
The defect
A module reached only through a dynamic
import()— perry'sModuleInitKind::Deferred— that takes part in an import cycle never initialized its cycle partner. The partner's body never ran, so every export the body assigns at run time stayedundefined, and the first call through such a binding threwTypeError: value is not a function.Entering the same cycle through a static import is correct, which is what makes this easy to miss.
The full repro is in the issue; the regression test in this PR is the same shape.
Cause
module_init_depsinrun_pipeline.rsdrops init-call back-edges (#6463):That is sound only because the entry's
mainemits an eager__initcall for every Eager module ininit_posorder, so a dep positioned earlier has already initialized and re-entering it would be the ordering bug #6463 fixed.Deferred modules are filtered out of that loop in
codegen/entry.rs, so nothing runs them but a dynamic-import dispatch site or another wrapper. Dropping a wrapper's edge to a Deferred dep therefore leaves it with no caller at all. In the repro the topological sort placesbbeforea, sob's edge toais a back-edge, it is dropped, andais stranded.The fix
Apply the positional drop to Eager deps only:
Two properties make this safe rather than a re-opening of #6463:
__perry_init_done_*guard sets the flag before running deps, so the re-entrant call returns immediately and the partner's body completes first — the same order node and bun produce, from either member as the entry point.Tests
crates/perry/tests/issue_10278_dynamic_import_cycle_init.rsis a deliberate pair:dynamic_import_into_cycle_initializes_the_partneris the defect. On the pre-fix compiler it fails withassigned-UNDEFINED; it asserts byte-for-byte equality with node/bun output.static_import_into_cycle_keeps_the_6463_orderis the control that pins the ordering the fix must not disturb.The cycle member assigns its exports at run time on purpose. A plain function declaration would not witness the bug, since those resolve without the body ever running.
cargo fmt --all -- --checkandscripts/check_file_size.shpass.Why it was found
OpenCode v1.18.30 defers every heavy subsystem behind
await import(...)inside its command handlers, so its TUI,serveandrunpaths are Deferred subgraphs, and those subgraphs (solid-js,@opentui/*, effect) contain import cycles. Reproduced on linux-x64 and darwin-arm64.Summary by CodeRabbit
Bug Fixes
Tests
Documentation