Skip to content

MessagePort: close a port whose peer closed, so it is rejected as a transferable - #38066

Open
robobun wants to merge 6 commits into
mainfrom
farm/23737826/messageport-peer-close-detaches
Open

MessagePort: close a port whose peer closed, so it is rejected as a transferable#38066
robobun wants to merge 6 commits into
mainfrom
farm/23737826/messageport-peer-close-detaches

MessagePort: track whether the port is receiving, re-report a gone pe…

d2851de
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 13, 2026 in 36m 18s

Code review found 3 important issues

Found 1 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 3
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/jsc/bindings/webcore/MessagePort.cpp:174-175 start() after listener removal does not restore m_receiving; peerClosed() defers forever and a ref()'d port hangs

Annotations

Check failure on line 175 in src/jsc/bindings/webcore/MessagePort.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

start() after listener removal does not restore m_receiving; peerClosed() defers forever and a ref()'d port hangs

`m_receiving = true` is only reached on the *first* `start()` — subsequent calls early-return on `if (m_started || !isEntangled()) return;` without touching it, whereas node's `MessagePort::Start()` sets `receiving_messages_ = true` unconditionally. So `port1.ref(); port1.on('message', h); port1.off('message', h); port1.start(); port2.postMessage('m'); port2.close();` leaves `m_receiving == false` (cleared by `removeEventListener` at :568), `peerClosed()` hits `!m_receiving && queuedCount > 0` a