fix(cluster): match Node 26 semantics - #7105
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
📝 WalkthroughWalkthroughThis change completes ChangesNode cluster parity
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Primary
participant ClusterWorker
participant ChildIpc
participant TcpServer
Primary->>ClusterWorker: fork worker with advanced serialization
ClusterWorker->>ChildIpc: send listening address and port
ChildIpc->>Primary: deliver advanced IPC frame
Primary->>TcpServer: connect to worker address
TcpServer->>Primary: return TCP response
Primary->>ClusterWorker: disconnect worker
ClusterWorker->>Primary: emit exit status
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
89944e7 to
140253d
Compare
140253d to
aed4fed
Compare
Align cluster exports, worker lifecycle, validation, events, networking, and IPC with Node.js 26.5.0. Complete the maintainer audit for EventEmitter semantics, worker identity and lifecycle, advanced IPC framing, scheduling, and parity coverage. Closes #6772
aed4fed to
76fa7b4
Compare
`every_dispatch_entry_has_manifest_counterpart` listed 14 missing rows at
origin/main; this branch as authored fixed 3 of them (dgram::sendto,
sqlite::serialize/deserialize). The other 11 landed on main *after* the
branch point, so merging main re-reds the very gate the branch exists to
turn green:
inspector::log/info/debug/warn/error, inspector/promises::SessionCall
— PerryTS#7090 (47040d5) inspector Node 26.5 parity
cluster::listeners/rawListeners/setMaxListeners/getMaxListeners
— PerryTS#7105 (76fa7b4) cluster Node 26 semantics
lru-cache::peek
— PerryTS#7136 (5f5006a) lru-cache faithful JS-value keys/values
Rows mirror how each surface is already modelled:
- the five `inspector.console.*` forwarders carry the dispatch table's
`class_filter: Some("console")`, so they hang off the existing
`property("inspector", "console")` instead of becoming named exports;
- `inspector/promises::SessionCall` is the synthetic row for `Session(...)`
called without `new` (js_node_inspector_session_call_without_new), so it
is `internal_method` — the .d.ts keeps advertising `Session`;
- the four `cluster` EventEmitter methods join their eleven siblings as
`internal_method` + paired `internal_property`, matching the PerryTS#3687 block
directly above them.
All rows use the plain `method`/`internal_method` helpers (params: &[],
returns: Any), which `manifest_param_counts_match_dispatch_table`
deliberately skips as the "no signature data" fallback — so no arity claim
is invented for them.
manifest_consistency: 5 passed / 0 failed (was 2 passed / 3 failed at
origin/main, 4 passed / 1 failed with only the original branch fix).
docs/src/api/reference.md regenerated: +6 rendered entries (the internal_*
rows stay out of the public surface, as cluster's existing ones already do);
docs/api/perry.d.ts is byte-unchanged, since nothing added is a named export.
…ble (#7159) * fix(api-manifest): reconcile API_MANIFEST with the native dispatch table The manifest_consistency drift guards in perry-codegen were red on main: - every_native_module_has_at_least_one_manifest_entry / #513: iovalkey (a NATIVE_MODULES member and well_known_bindings entry that normalizes to ioredis in dispatch) had zero API_MANIFEST rows, so strict mode never engaged for it. Add the Redis class + createClient rows, mirroring the redis alias. - every_dispatch_entry_has_manifest_counterpart: dgram Socket.sendto and sqlite DatabaseSync.serialize/deserialize existed in NATIVE_MODULE_TABLE with no manifest counterpart. Add the matching receiver-method rows. - every_well_known_binding_has_manifest_entry: the test's inline TOML parser misread the new [bindings.*.upstream] provenance sub-tables as binding names (dotenv.upstream, iovalkey.upstream, ...). Teach it to skip nested sub-tables so it only checks real routed module names. Regenerate docs/api/perry.d.ts and docs/src/api/reference.md from the manifest (api-docs-drift gate); this also folds in the previously uncommitted node-forge and tls.convertALPNProtocols entries. * chore(changelog): key the fragment to PR 7159 changelog.d fragments are named <PR>-<slug>.md (changelog.d/README.md), so in-flight PRs never collide. This one was keyed to the issue it fixes (0513) rather than the PR. Also confirms the merge resolution: regenerating both API-docs artifacts from the merged manifest reproduces the committed files byte-for-byte, and their only delta against main is the "Coverage:"/"Total:" header count — main's #7118 regenerated the artifacts without adding manifest rows, so nothing from main is dropped. * fix(api-manifest): cover the dispatch rows main added after this branch `every_dispatch_entry_has_manifest_counterpart` listed 14 missing rows at origin/main; this branch as authored fixed 3 of them (dgram::sendto, sqlite::serialize/deserialize). The other 11 landed on main *after* the branch point, so merging main re-reds the very gate the branch exists to turn green: inspector::log/info/debug/warn/error, inspector/promises::SessionCall — #7090 (47040d5) inspector Node 26.5 parity cluster::listeners/rawListeners/setMaxListeners/getMaxListeners — #7105 (76fa7b4) cluster Node 26 semantics lru-cache::peek — #7136 (5f5006a) lru-cache faithful JS-value keys/values Rows mirror how each surface is already modelled: - the five `inspector.console.*` forwarders carry the dispatch table's `class_filter: Some("console")`, so they hang off the existing `property("inspector", "console")` instead of becoming named exports; - `inspector/promises::SessionCall` is the synthetic row for `Session(...)` called without `new` (js_node_inspector_session_call_without_new), so it is `internal_method` — the .d.ts keeps advertising `Session`; - the four `cluster` EventEmitter methods join their eleven siblings as `internal_method` + paired `internal_property`, matching the #3687 block directly above them. All rows use the plain `method`/`internal_method` helpers (params: &[], returns: Any), which `manifest_param_counts_match_dispatch_table` deliberately skips as the "no signature data" fallback — so no arity claim is invented for them. manifest_consistency: 5 passed / 0 failed (was 2 passed / 3 failed at origin/main, 4 passed / 1 failed with only the original branch fix). docs/src/api/reference.md regenerated: +6 rendered entries (the internal_* rows stay out of the public surface, as cluster's existing ones already do); docs/api/perry.d.ts is byte-unchanged, since nothing added is a named export. * docs(changelog): note the post-branch dispatch rows in the 7159 fragment --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
`emit_preallocate_boxes` allocated a heap box — and, decisively, a `ctx.locals` slot — for every id in a `Stmt::PreallocateBoxes` directive, including ids that `codegen/module_globals_emit.rs` had already promoted to `@perry_global_<mod>__<id>`. Every other read/write site in codegen already encodes the rule that the module global wins over the box (`ctx.boxed_vars.contains(id) && !ctx.module_globals.contains_key(id)`). This one did not, and the stale `ctx.locals` entry it left behind is consulted BEFORE `ctx.module_globals` on the `Stmt::Let` reuse path (`let_stmt.rs`) and on the `LocalGet`/`LocalSet` store paths (`expr/literals_vars.rs`). The declaration therefore wrote its value into the local box-pointer slot; the module global was never stored; and every closure that reads the binding through the global saw the `undefined` it was defined with. Since #7105 added `lower_strict_block_fn_decls`, a bare block at the top level of any ES module emits `PreallocateBoxes` for the block's `let`/`const` bindings, so this hit ordinary code: { const events = []; function t() { events.push("x") } t(); events.length } `t()` ran, `events.length` was 0, and nothing threw. Objects, numbers and strings lost the same way (`acc` read as `undefined` inside `t`). Fix: skip promoted ids in `emit_preallocate_boxes`. The global is statically initialized to `TAG_UNDEFINED`, which is exactly what a non-TDZ prealloc box seeds. The TDZ variant is skipped too — module-global reads are raw `load double @g` with no `js_box_get_bits` choke point, so seeding `TAG_TDZ` there would leak the sentinel into arithmetic rather than throwing. Closes #7521 Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
Summary
node:clusterexports, worker lifecycle, validation, events, networking, and IPC with Node.js 26.5.0.Changes
Workersurface, setup semantics, scheduling validation, and lifecycle ordering.Related issue
Closes #6772
Stacked on #7089; base:
fix/child-process-node26-parity.Test plan
cargo build -p perry --releasecargo test -p perry-codegen type_analysis --lib(12 passed)./run_parity_tests.sh --suite node-suite --module cluster(41 passed; 0 failures, compile failures, crashes, or skips)Checklist
fix:formatSummary by CodeRabbit
New Features
node:clustercompatibility with improved worker lifecycle events, listener management, worker APIs, networking, setup validation, and advanced IPC serialization.ArrayBuffervalues.Bug Fixes
Tests
Documentation