fix(runtime): deliver MessageChannel/MessagePort messages on the same thread#5530
Conversation
Previously MessagePort.postMessage was a no-op and onmessage was a plain
null field, so messages were silently dropped. A program that uses a
MessageChannel for async scheduling (port1.onmessage = cb;
port2.postMessage(0)) would idle forever because the scheduled callback
never ran.
Entangle the port1/port2 pair on channel creation and track per-port
delivery state (entangled link, FIFO incoming queue, started/closed
flags, onmessage handler, addEventListener listeners) in a process-global
side table keyed by the port object pointer, GC-rooted so held JS values
survive across event-loop turns.
postMessage structured-clones the value (pass-through; tolerates a
transfer-array arg), enqueues it on the entangled port, and — once that
port is started — schedules a macrotask via the existing setImmediate
callback-timer queue that dispatches a MessageEvent-shaped object
({ data, type: 'message' }) to the onmessage handler and any
addEventListener('message', ...) listeners. Delivery is FIFO, one
macrotask per message. A port starts implicitly on onmessage assignment
(now a getter/setter), .start(), or addEventListener('message'); messages
posted before start queue and flush in order once started. .close() stops
delivery.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughImplements same-thread ChangesMessageChannel/MessagePort same-thread delivery
Sequence Diagram(s)sequenceDiagram
participant TS as TypeScript Program
participant port_post_message
participant PORT_STATES as PORT_STATES (PortState queue)
participant setImmediate as setImmediate scheduler
participant deliver_one_message
participant onmessage as onmessage / listeners
TS->>port_post_message: port2.postMessage(data)
port_post_message->>PORT_STATES: structured_clone(data), enqueue to port1.queue
alt port1 is started and not closed
port_post_message->>setImmediate: schedule_delivery(port1)
end
setImmediate-->>deliver_one_message: fire macrotask turn
deliver_one_message->>PORT_STATES: dequeue one MessageEvent
deliver_one_message->>onmessage: call onmessage(event) + call each listener(event)
alt more messages in queue
deliver_one_message->>setImmediate: schedule_delivery(port1) again
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/messaging.rs`:
- Around line 360-371: The issue is that in the port_close function, when a port
is closed, the PortState entry remains in the PORT_STATES map indefinitely with
only the closed flag set to true and the queue cleared. Instead of just marking
the state as closed, the entry should be removed from the map when the port
closes. Modify the logic within the with_port_states closure to call
map.remove(&self_ptr) after setting state.closed = true, or implement logic to
only remove the entry when both the port and its entangled partner are closed to
prevent unbounded memory growth in long-running applications.
- Around line 138-181: The `entangled` field in the `PortState` struct is stored
as a raw `usize` pointer but is not visited by the GC scanner in the
`port_states_root_scanner_mut` function, creating a use-after-free risk if the
entangled port is collected while still referenced. Convert the `entangled`
field from `usize` to `f64` to use NaN-boxing like the other pointer fields
(`onmessage` and `listeners`), then add a visitor call for this field in
`port_states_root_scanner_mut` using `visitor.visit_nanbox_f64_slot()` to ensure
the entangled port is properly rooted by the garbage collector. Update any code
that reads or writes to the `entangled` field to work with the f64 NaN-boxed
representation instead of raw pointer arithmetic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fb0e9754-5b5f-4ec4-b790-626daaca05f6
📒 Files selected for processing (2)
crates/perry-runtime/src/messaging.rscrates/perry/tests/issue_message_channel_same_thread_delivery.rs
Address CodeRabbit review on the same-thread MessageChannel delivery: - Root the entangled-partner address in the GC scanner (visit_usize_slot) so the paired port survives event-loop turns and its address is rewritten on evacuation. Also rewrite the PORT_STATES keys — they are port heap addresses, so a moved port would otherwise leave a stale key and silently drop later messages. - Reclaim PORT_STATES entries on close() once the channel is dead (no partner, or partner already closed/gone) instead of leaving the entry in the map forever, so long-running apps that churn through short-lived channels don't accumulate port state without bound. perry-runtime lib tests: 1067 passed (--test-threads=1). The 4 e2e cases in issue_message_channel_same_thread_delivery still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed both CodeRabbit findings in b0a4774: 1. Entangled port not rooted by the GC — the scanner now visits While there I also fixed a related latent issue: 2. Closed ports never removed — Validation: |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
MessagePort.postMessagewas a no-op stub (messaging.rsinstallednoop1/noop2,onmessage = null), so messages were silently dropped:This breaks the extremely common MessageChannel-as-macrotask-scheduler pattern (
port1.onmessage = cb; port2.postMessage(0)to schedule a task — used by React's scheduler,setImmediatepolyfills, async yielders). The scheduled callback never runs, so a program that drives its startup through such a scheduler hangs forever with the event loop idle on the channel.Fix
Implement same-thread
MessageChannel/MessagePortdelivery (HTML/Node semantics):new MessageChannel()links port1 ↔ port2.VecDeque,started/closedflags, theonmessagevalue, andaddEventListener("message")listeners.postMessage(data): enqueues the value on the entangled port and, if it's started, schedules a task (macrotask) — reusing the existingsetImmediatecallback-timer queue (crate::timer::js_set_immediate_callback_args), which the event loop already pumps — that dispatches amessageevent ({ data, type: "message" }) to that port, invokingonmessageand all listeners withthis= the port.onmessageis assigned (getter/setter) or.start()/addEventListeneris called; queued messages then flush in FIFO order..close()stops delivery.Structured clone is a pass-through for now (a transfer-list argument is accepted without crashing); deep clone / transfer semantics are follow-ups.
Verification
Standalone: basic delivery, bidirectional, FIFO order,
addEventListeneralongsideonmessage, message posted before the handler is assigned, and the scheduler-chaining pattern (a handler that re-posts) all run to completion.cargo test -p perry-runtime --lib --test-threads=1: 1067 passed. New e2eissue_message_channel_same_thread_delivery.rs(4 cases) passes; the pre-existing worker_threadsMessagePortdelegation test still passes (factory path unaffected).Summary by CodeRabbit
New Features
MessageChannel/MessagePortdelivery with entangled ports.onmessageand"message"event handling viaaddEventListener/removeEventListener, including properstart/closelifecycle behavior.Tests
onmessagevsaddEventListenercallbacks, including cases where handlers trigger follow-up sends.