Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 6 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6e339fb
MessagePort: close the port when its peer closes
robobun Aug 13, 2026
fd863bc
test: prove the peer was collected in the collected-peer MessagePort …
robobun Aug 13, 2026
bc7a4e0
MessagePort: trim the peer-close comments
robobun Aug 13, 2026
f0af4bc
MessagePort: only defer a peer close behind the queue for an explicit…
robobun Aug 13, 2026
d2851de
MessagePort: track whether the port is receiving, re-report a gone pe…
robobun Aug 13, 2026
5db54a9
MessagePort: start() resumes a paused port
robobun Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 79 additions & 54 deletions src/jsc/bindings/webcore/MessagePort.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
{
auto messagePort = adoptRef(*new MessagePort(context, WTF::move(pipe), side));
messagePort->suspendIfNeeded();
// So the peer's close() reaches this port (peerClosed()) even if it never gets a listener.
messagePort->m_pipe->registerCloseContext(side, context.identifier(), ThreadSafeWeakPtr<MessagePort> { messagePort.get() });
return messagePort;
}

Expand Down Expand Up @@ -105,14 +107,11 @@
}
RETURN_IF_EXCEPTION(warnScope, {});

if (!isEntangled())
return {};

Vector<TransferredMessagePort> transferredPorts;
// Posting a port's own entangled peer targets the message at itself.
// (The source port itself was rejected before serialization above.)
Comment thread
robobun marked this conversation as resolved.
bool targetsEntangledPeer = false;
if (!ports.isEmpty()) {
// Posting a port's own entangled peer targets the message at itself.
// (The source port itself was rejected before serialization above.)
bool targetsEntangledPeer = false;
for (auto& port : ports) {
if (port->pipe() == m_pipe.ptr()) {
targetsEntangledPeer = true;
Expand All @@ -125,20 +124,25 @@
if (disentangled.hasException())
return disentangled.releaseException();
transferredPorts = disentangled.releaseReturnValue();
}

if (targetsEntangledPeer) {
// Posting the port's own entangled peer: node warns and loses the channel
// rather than throwing. Transferables were already detached above; drop the
// message and close so the dead channel stops reffing the loop.
Bun__Process__emitWarning(defaultGlobalObject(&state),
JSC::JSValue::encode(JSC::jsString(vm, String("The target port was posted to itself, and the communication channel was lost"_s))),
JSC::JSValue::encode(JSC::jsString(vm, String("Warning"_s))),
JSC::JSValue::encode(JSC::jsUndefined()),
JSC::JSValue::encode(JSC::jsUndefined()));
CLEAR_IF_EXCEPTION(warnScope);
close();
return {};
}
// A closed port drops the message; the ports taken out of the transfer list go down with it
// (~TransferredMessagePort closes their sides, so their peers see a close), as in node.
Comment thread
robobun marked this conversation as resolved.
if (!isEntangled())
return {};

if (targetsEntangledPeer) {
// Posting the port's own entangled peer: node warns and loses the channel
// rather than throwing. Transferables were already detached above; drop the
// message and close so the dead channel stops reffing the loop.
Comment thread
robobun marked this conversation as resolved.
Bun__Process__emitWarning(defaultGlobalObject(&state),
JSC::JSValue::encode(JSC::jsString(vm, String("The target port was posted to itself, and the communication channel was lost"_s))),
JSC::JSValue::encode(JSC::jsString(vm, String("Warning"_s))),
JSC::JSValue::encode(JSC::jsUndefined()),
JSC::JSValue::encode(JSC::jsUndefined()));
CLEAR_IF_EXCEPTION(warnScope);
close();
return {};
}

m_pipe->send(m_side, MessageWithMessagePorts { messageData.releaseReturnValue(), WTF::move(transferredPorts) });
Expand All @@ -154,20 +158,25 @@

void MessagePort::start()
{
if (m_started || !isEntangled())
if (!isEntangled())
return;
// A node worker's parentPort delivers nothing until the entry module has evaluated; keep the
// request and let entrySettled() perform it. Messages stay buffered in the pipe meanwhile.
if (auto* context = scriptExecutionContext()) {
if (auto* jsGlobal = context->globalObject()) {
auto* globalObject = defaultGlobalObject(jsGlobal);
if (globalObject->nodeParentPort() == this && !globalObject->nodeWorkerEntrySettled()) {
m_startDeferredUntilEntrySettled = true;
return;
if (!m_started) {
// A node worker's parentPort delivers nothing until the entry module has evaluated; keep the
// request and let entrySettled() perform it. Messages stay buffered in the pipe meanwhile.
Comment thread
robobun marked this conversation as resolved.
if (auto* context = scriptExecutionContext()) {
if (auto* jsGlobal = context->globalObject()) {
auto* globalObject = defaultGlobalObject(jsGlobal);
if (globalObject->nodeParentPort() == this && !globalObject->nodeWorkerEntrySettled()) {
m_startDeferredUntilEntrySettled = true;
return;
}
}
}
m_started = true;
}
m_started = true;
// Every call turns delivery (back) on, as node's Start() does for a port whose last 'message'
// listener was removed. attach() re-schedules the drain and re-reports a peer that has closed.
Comment thread
robobun marked this conversation as resolved.
m_receiving = true;

auto* context = scriptExecutionContext();
ASSERT(context);
Expand All @@ -191,10 +200,11 @@
// re-injecting into this closing port (via its entangled peer) can't starve the loop.
size_t limit = std::max<size_t>(MessagePortPipe::queuedCount(m_pipe->state(m_side)), 1000);
for (size_t i = 0; i < limit; ++i) {
// A handler (or a microtask it queued) may have transferred this port; the
// remaining inbox now belongs to the new owner. drainAndDispatch()'s
// per-iteration ctxId/port re-check guards the same case.
if (m_isDetached)
// A handler (or a microtask it queued) may have transferred this port (the remaining
// inbox now belongs to the new owner) or removed the last listener, e.g. once(): like
// drainAndDispatch(), stop and leave the rest queued. peerClosed() then keeps the port
// open for a later listener; close() drops it.
Comment thread
robobun marked this conversation as resolved.
if (m_isDetached || !hasMessageEventListener())
break;
auto message = m_pipe->takeOne(m_side);
if (!message)
Expand Down Expand Up @@ -271,24 +281,35 @@

void MessagePort::peerClosed()
{
if (m_isDetached)
if (m_isDetached || m_isClosing)
return;
auto* context = scriptExecutionContext();
if (!context || !context->globalObject())
return;
Ref protectedThis { *this };
// Deliver whatever the peer sent before it closed, then fire 'close'. Node orders
// them that way, and registerCloseContext()'s retroactive notify can land before any
// drain is scheduled -- e.g. on('close') registered before on('message').
// Node delivers what the peer sent before closing first; this task can run before the drain.
if (m_started && hasMessageEventListener())
flushQueuedMessagesBeforeClose();
// Fire 'close' (guarded against a double dispatch) and release this side's loop refs
// so the loop can idle, matching node.
dispatchCloseEvent();
// jsUnref() clears both the listener loop-ref (m_isRefd) and the onmessage/ref()
// keepalive (m_hasRef), so a listening transferred port stops pinning the loop.
auto* globalObject = defaultGlobalObject(context->globalObject());
jsUnref(globalObject);
// A handler in that flush closed or transferred this port.
if (m_isDetached || m_isClosing)
return;
if (!m_pipe->isOtherSideClosedByRequest(m_side)) {
// Peer collected, not closed: node never closes a channel over that (see jsRef()), so
// only release the loop refs of a port that is started or listening for 'close'. An
// idle port keeps its one 'close' event for its own close(cb); it is notified again
// if it ever gets a 'close' listener (addEventListener) or starts (attach()).
Comment thread
robobun marked this conversation as resolved.
if (m_started || m_hasCloseEventListener.load(std::memory_order_relaxed)) {
dispatchCloseEvent();
jsUnref(defaultGlobalObject(context->globalObject()));
}
return;
}
// Not receiving with messages queued: node's close notification waits behind them. The
// next 'message' listener (attach()) or the receiveMessageOnPort() that empties the queue
// (tryTakeMessage()) completes the close.
Comment thread
robobun marked this conversation as resolved.
if (!m_receiving && MessagePortPipe::queuedCount(m_pipe->state(m_side)) > 0)
return;

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

View check run for this annotation

Claude / Claude Code Review

ref()'d port paused by listener removal hangs after peer closes with a message queued 

The `!m_receiving && queuedCount > 0` deferral models node's `receiving_messages_`, but node's same `removeListener` hook that clears it (`stopMessagePort`) also calls `port.unref()` — so a not-receiving port never holds a loop ref there. Bun's `removeEventListener` (:562-568) clears `m_receiving` but leaves `m_hasRef` untouched, so `port1.ref(); port1.on('message',h); port1.off('message',h); port2.postMessage('m'); port2.close();` (or `.once()` + two peer messages) defers here forever with `ref
Comment on lines +307 to +311

@claude claude Bot Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 The !m_receiving && queuedCount > 0 deferral models node's receiving_messages_, but node's same removeListener hook that clears it (stopMessagePort) also calls port.unref() — so a not-receiving port never holds a loop ref there. Bun's removeEventListener (:562-568) clears m_receiving but leaves m_hasRef untouched, so port1.ref(); port1.on('message',h); port1.off('message',h); port2.postMessage('m'); port2.close(); (or .once() + two peer messages) defers here forever with refKeepAlive(+1) never balanced — process hangs. Pre-PR peerClosed() unconditionally jsUnref()'d and it exited; node exits because the pause unref'd the port. Fix: mirror node — release the loop ref when the last 'message' listener is removed (or jsUnref() here even when deferring, so the port stays open/transferable but stops pinning the loop).

Extended reasoning...

What the bug is

peerClosed() at MessagePort.cpp:310-311 defers closing when !m_receiving && queuedCount > 0, keeping the port open until a later 'message' listener (via attach()) or receiveMessageOnPort() (via tryTakeMessage()) drains the queue. m_receiving is modeled on node's receiving_messages_, which node's stopMessagePort clears when the last 'message' listener is removed. But node's setupPortReferencing removeListener hook does two things at that point: it calls stopMessagePort and port.unref(). Bun's removeEventListener at :562-568 replicates only the first — it clears m_receiving but leaves m_hasRef (the explicit .ref() keepalive) alone. So a .ref()'d port paused by removing its last listener still holds Bun__eventLoop__refKeepAlive(+1), and if the peer closes with a message left queued, peerClosed() defers here forever without ever reaching close() — the process hangs.

Step-by-step proof

const { port1, port2 } = new (require('worker_threads').MessageChannel)();
port1.ref();
const h = () => {};
port1.on('message', h);
port1.off('message', h);
port2.postMessage('queued');
port2.close();
setTimeout(() => { console.log('hung, hasRef=' + port1.hasRef()); process.exit(1); }, 3000).unref();
  1. port1.ref() → jsRef(): isEntangled() true, isOtherSideClosedByRequest() false → m_hasRef=true, self-ref(), Bun__eventLoop__refKeepAlive(vm, +1).
  2. port1.on('message', h) → addEventListener calls start() (sets m_started=true, m_receiving=true, attach()); onDidChangeListenerImpl bumps m_messageEventCount to 1 → m_listenerLoopRefActive=true.
  3. port1.off('message', h) → removeEventListener at :559-568: no more 'message' listeners → m_hasMessageEventListener=false, m_receiving=false. onDidChangeListenerImpl drops count to 0 → m_listenerLoopRefActive=false. m_hasRef stays true — nothing in this path touches it.
  4. port2.postMessage('queued') → send() appends to side 0's inbox (queued=1), sets DrainScheduled, posts a drain task.
  5. port2.close() → pipe->close(1, Explicit) sets Closed | ClosedByRequest on side 1, notifyPeerClosed(0) posts a task.
  6. Drain task runs → drainAndDispatch hits !port->hasMessageEventListener() (MessagePortPipe.cpp:127-131), clears DrainScheduled, returns without consuming. queued stays 1.
  7. peerClosed() task runs → m_started && hasMessageEventListener() is (true && false) → skip flush. isOtherSideClosedByRequest() is true → skip collected-peer branch. !m_receiving && queuedCount > 0 is (true && 1>0) → return at :311. close() at :312 (which would have released m_hasRef) is never reached.

Result: m_hasRef's Bun__eventLoop__refKeepAlive(+1) is never balanced → process hangs.

The .once() variant is arguably more realistic and hits the identical path: port1.ref(); port1.once('message', ()=>{}); then the peer sends two messages and closes — the once() consumes the first (its self-removal clears m_receiving), the second is left queued, peerClosed() defers forever.

Why nothing recovers it

  • attach() re-notifies on a Closed peer, but is only reached from start() or a new 'message' listener — the user never adds one and never calls start().
  • tryTakeMessage() requires a user receiveMessageOnPort() call; there isn't one.
  • registerCloseContext() (from a late 'close' listener) would just re-post peerClosed(), which hits the same deferral.
  • virtualHasPendingActivity() returns false (no listeners, DrainScheduled clear, peer closed, but queued>0 → actually true on the last line, but irrelevant): even if the wrapper were collected, m_hasRef's self-ref() keeps the C++ object alive and ~MessagePort() doesn't release the keepalive.

Why this is a regression

  • Pre-PR: peerClosed() unconditionally called jsUnref(globalObject), which cleared m_hasRef and Bun__eventLoop__refKeepAlive(-1) → process exited.
  • Node: setupPortReferencing's removeListener handler calls port.unref() alongside stopMessagePort. libuv ref is a single bit, so this undoes the earlier explicit .ref() — the port stays open (deferred close, matching this PR's intent) but its handle is unref'd → process exits.
  • The onmessage = fn; onmessage = null variant doesn't hit this because setJSMessagePort_onmessageSetter calls jsUnref() on the null assignment; only .ref() + .on/.off (or a fired .once) reaches it.

This is the remaining gap in the m_receiving fix from the previous review rounds: the earlier hangs (start()-only, and pause+start()) were fixed by making start() set m_receiving on every call, but a paused port the user never resumes still holds m_hasRef forever. The three "paused by …" tests don't .ref(), and the two "start() on a paused port" tests .ref() but explicitly resume via start(), so no test in the PR covers .ref() + paused + queued message + peer close + never resumed.

How to fix it

Two options that both match node's observable state (port stays open/transferable, but does not pin the loop):

(a) Mirror node — call jsUnref() (or at least release m_hasRef) in removeEventListener when the last 'message' listener goes, alongside clearing m_receiving. This is exactly what makes node's deferral safe: a not-receiving port never holds a loop ref there. addEventListener's start() path would need to reacquire it (or the user calls .ref() again), which is also what node does.

(b) Release the loop refs when deferring — in peerClosed(), jsUnref() even when taking the !m_receiving && queuedCount > 0 early return. The port stays open/transferable (the close is still deferred), but stops pinning the loop.

Either way, add a spawned-process test alongside the existing "a ref()'d port with no listeners…" one: port1.ref(); port1.once('message', ()=>{}); port2.postMessage('a'); port2.postMessage('b'); port2.close(); and assert the process exits.

close();
}
Comment thread
robobun marked this conversation as resolved.

TransferredMessagePort MessagePort::disentangle()
Expand Down Expand Up @@ -381,8 +402,14 @@
return jsUndefined();

auto message = m_pipe->takeOne(m_side);
if (!message)
if (!message) {
// This read reached the peer's close: node closes the port here (see peerClosed()).
// Peer state before our queue, as in virtualHasPendingActivity(): a message the peer
// sent between takeOne() and its close() is then visible and must not be dropped.
Comment thread
robobun marked this conversation as resolved.
if (m_pipe->isOtherSideClosedByRequest(m_side) && !MessagePortPipe::queuedCount(m_pipe->state(m_side)))
close();
return jsUndefined();
Comment thread
claude[bot] marked this conversation as resolved.
}

hadMessage = true;
auto ports = MessagePort::entanglePorts(*context, WTF::move(message->transferredPorts));
Expand Down Expand Up @@ -515,19 +542,13 @@
bool MessagePort::addEventListener(const AtomString& eventType, Ref<EventListener>&& listener, const AddEventListenerOptions& options)
{
if (eventType == eventNames().messageEvent) {
// Also resumes a port paused by removing its listeners (see start()).
start();
m_hasMessageEventListener = true;
// start() no-ops after the first call; re-attach so a listener re-added after a
// pause re-schedules the drain for messages buffered meanwhile.
if (m_started && isEntangled()) {
if (auto* context = scriptExecutionContext())
m_pipe->attach(m_side, context->identifier(), ThreadSafeWeakPtr<MessagePort> { *this });
}
} else if (eventType == eventNames().closeEvent) {
m_hasCloseEventListener.store(true, std::memory_order_release);
// Reports a peer that went away while this port had no listener (see peerClosed()).
if (isEntangled()) {
// Record our context with the pipe so the peer's close() can deliver a
// 'close' event even if we never started (no 'message' listener).
if (auto* context = scriptExecutionContext())
m_pipe->registerCloseContext(m_side, context->identifier(), ThreadSafeWeakPtr<MessagePort> { *this });
}
Expand All @@ -537,9 +558,13 @@

bool MessagePort::removeEventListener(const AtomString& eventType, EventListener& listener, const EventListenerOptions& options)
{
auto result = EventTarget::removeEventListener(eventType, listener, options);
if (!hasEventListeners(eventNames().messageEvent))
if (!hasEventListeners(eventNames().messageEvent)) {
m_hasMessageEventListener = false;
// Node stops the port when its last 'message' listener goes.
if (eventType == eventNames().messageEvent)
m_receiving = false;
}

Check warning on line 567 in src/jsc/bindings/webcore/MessagePort.cpp

View check run for this annotation

Claude / Claude Code Review

removeEventListener('message', notFound) clears m_receiving even when nothing was removed 

`m_receiving = false` is set whenever `!hasEventListeners(messageEvent)` after the base call, without checking `result` — so `port.start(); port.removeEventListener('message', neverAddedFn)` spuriously flips `m_receiving` to false even though nothing was removed (node's `[kRemoveListener]` → `stopMessagePort` only fires when the listener was actually found). A subsequent `port2.postMessage('m'); port2.close()` then defers at the `!m_receiving && queuedCount > 0` check in `peerClosed()` and the p
Comment on lines 561 to +567

@claude claude Bot Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 m_receiving = false is set whenever !hasEventListeners(messageEvent) after the base call, without checking result — so port.start(); port.removeEventListener('message', neverAddedFn) spuriously flips m_receiving to false even though nothing was removed (node's [kRemoveListener] → stopMessagePort only fires when the listener was actually found). A subsequent port2.postMessage('m'); port2.close() then defers at the !m_receiving && queuedCount > 0 check in peerClosed() and the port stays open/transferable where node closes it. Gate the assignment on result: if (result && eventType == eventNames().messageEvent) m_receiving = false;.

Extended reasoning...

What the bug is

MessagePort::removeEventListener at MessagePort.cpp:561-567 does:

auto result = EventTarget::removeEventListener(eventType, listener, options);
if (!hasEventListeners(eventNames().messageEvent)) {
    m_hasMessageEventListener = false;
    if (eventType == eventNames().messageEvent)
        m_receiving = false;   // ← not gated on `result`
}

The m_receiving = false assignment models node's stopMessagePort (which clears receiving_messages_), but node only calls that from the [kRemoveListener] hook, and lib/internal/event_target.js's removeEventListener only invokes this[kRemoveListener] inside the matched-handler branch of its linked-list walk — a not-found removal returns without calling it. So in node, port.removeEventListener('message', fnThatWasNeverAdded) leaves receiving_messages_ untouched; here it flips m_receiving to false whenever there happen to be no 'message' listeners left afterwards (which is trivially true if there never were any).

Step-by-step proof

const { port1, port2 } = new MessageChannel();
port1.start();                                   // m_receiving = true (set in start())
port1.removeEventListener('message', () => {});  // not found: result = false
port2.postMessage('m');
port2.close();
  1. port1.start() → m_started = true, m_receiving = true, attach().
  2. port1.removeEventListener('message', neverAdded) → EventTarget::removeEventListener finds nothing and returns false. !hasEventListeners(messageEvent) is true (there never were any) and eventType == messageEvent → m_receiving = false.
  3. port2.postMessage('m'); port2.close() → drain task runs, drainAndDispatch bails on !port->hasMessageEventListener() without consuming (queuedCount stays 1). peerClosed() runs: m_started && hasMessageEventListener() is (true && false) → skip flush; isOtherSideClosedByRequest() → true; !m_receiving && queuedCount > 0 at :310 is (true && true) → return. close() is never reached.

Node's behavior for the same sequence: receiving_messages_ stays true after step 2 (not-found removal is a no-op on the hook), so the started port drops 'm' into the void and processes the close — port is closed and rejected as a transferable. This is exactly the case the PR's own "a start()ed port without a 'message' listener is closed even with a message queued" test exercises; that test would fail if a bogus .removeEventListener('message', () => {}) were inserted after port1.start().

Why existing code doesn't prevent it

The .off() fallback at src/js/node/worker_threads.ts:183-184 does const wrapper = byListener?.get(listener) ?? listener; this.removeEventListener(event, wrapper), so .off('message', neverAdded) reaches this path with the raw (never-registered) listener. Nothing in removeEventListener distinguishes "the last listener was removed" from "there were never any listeners and this call removed nothing" — both satisfy !hasEventListeners(messageEvent).

Impact

  • Without .ref(): the port stays open and transferable after its peer closed, where node closes and rejects it — a node-compat divergence in the exact semantics this PR is implementing (m_receiving was added specifically to model node's receiving_messages_).
  • With .ref(): the deferral never completes and the process hangs — but that hang is the same mechanism as the separate "deferral doesn't release m_hasRef" issue; once that is addressed the hang here goes away, leaving only the transferable-vs-rejected divergence.

The trigger (calling removeEventListener/.off() with a function that was never added, on a start()ed port that has no 'message' listeners) is unusual — it's a user-side no-op call — so this is filed as a nit rather than a blocker.

How to fix

Gate the m_receiving = false assignment on result, matching node's [kRemoveListener] semantics:

if (result && eventType == eventNames().messageEvent)
    m_receiving = false;

(Or equivalently, move the whole m_receiving = false line under an if (result) — the m_hasMessageEventListener = false line above it is idempotent and safe to leave un-gated, since it just re-states !hasEventListeners(messageEvent).)

if (!hasEventListeners(eventNames().closeEvent))
m_hasCloseEventListener.store(false, std::memory_order_release);
return result;
Expand Down
7 changes: 5 additions & 2 deletions src/jsc/bindings/webcore/MessagePort.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ class MessagePort final : public ActiveDOMObject, public EventTarget, public Thr
// The worker's entry module finished evaluating: a start() requested before that takes effect now.
void entrySettled();
void close();
// Called on the entangled peer when this side closes: dispatches a
// 'close' event and releases the event-loop ref so the loop can idle.
// Posted by the pipe to the peer of a side that closed: closes this port too once its queue is drained.
void peerClosed();
void dispatchCloseEvent();

Expand Down Expand Up @@ -147,6 +146,10 @@ class MessagePort final : public ActiveDOMObject, public EventTarget, public Thr
const uint8_t m_side { 0 };

bool m_started { false };
// Node's receiving_messages_: set by start() (explicitly or through a 'message' listener),
// cleared when the last 'message' listener is removed. While clear, a peer's close waits
// behind the queued messages (peerClosed()); m_started itself is never cleared.
Comment thread
robobun marked this conversation as resolved.
bool m_receiving { false };
bool m_isDetached { false };
bool m_isClosing { false };
// True while a 'message' handler is on the stack. close() called from inside one
Expand Down
21 changes: 11 additions & 10 deletions src/jsc/bindings/webcore/MessagePortPipe.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,8 @@ void MessagePortPipe::attach(uint8_t side, ScriptExecutionContextIdentifier ctxI
}
if (wakeCtx)
scheduleDrain(side, wakeCtx);
// Peer already closed while this side was in transit (detach() cleared
// ContextKnown so notifyPeerClosed() early-returned): re-deliver to the new
// owner, or the receiving context's listener loop-ref is never released.
// Peer already closed: notify again now that this side is receiving. The task runs after the
// drain scheduled above, so a close that peerClosed() deferred behind queued messages completes.
Comment thread
robobun marked this conversation as resolved.
if (m_sides[1 - side].state.load(std::memory_order_acquire) & Closed)
notifyPeerClosed(side);
}
Expand All @@ -248,15 +247,17 @@ void MessagePortPipe::registerCloseContext(uint8_t side, ScriptExecutionContextI
{
Locker locker { s.lock };
uint64_t st = s.state.load(std::memory_order_relaxed);
// Already closed, or context already known (started or previously registered).
if ((st & Closed) || (st & (Attached | ContextKnown)))
if (st & Closed)
return;
s.ctxId = ctxId;
s.port = WTF::move(port);
s.state.store(st | ContextKnown, std::memory_order_release);
// Already known (started, or registered at creation): keep that, only re-check the peer.
if (!(st & (Attached | ContextKnown))) {
s.ctxId = ctxId;
s.port = WTF::move(port);
s.state.store(st | ContextKnown, std::memory_order_release);
}
}
// See attach(): re-deliver a peer-close that fired while this side had no
// context (in transit or never registered).
// See attach(). Also how a port that ignored a collected peer while it had no listener
// (MessagePort::peerClosed) hears about it again once it gets a 'close' listener.
Comment thread
robobun marked this conversation as resolved.
if (m_sides[1 - side].state.load(std::memory_order_acquire) & Closed)
notifyPeerClosed(side);
}
Expand Down
6 changes: 3 additions & 3 deletions src/jsc/bindings/webcore/MessagePortPipe.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ class MessagePortPipe final : public ThreadSafeRefCounted<MessagePortPipe> {
// already queued (e.g. after transfer). Passing a null port is allowed and
// means "just buffer, don't dispatch" (used before start()).
void attach(uint8_t side, ScriptExecutionContextIdentifier, ThreadSafeWeakPtr<MessagePort>);
// Like attach() but only records ctxId/port so the peer's close() can deliver
// a 'close' event to a port that never started (no 'message' listener). Does
// NOT enable drains. No-op if already attached/registered/closed.
// Records ctxId/port (unless already attached/registered) so the peer's close() reaches a port
// that never started, without enabling drains, and reports a peer that is already closed. Called
// when a MessagePort is created for this side and when it gains a 'close' listener.
Comment thread
robobun marked this conversation as resolved.
void registerCloseContext(uint8_t side, ScriptExecutionContextIdentifier, ThreadSafeWeakPtr<MessagePort>);
void detach(uint8_t side);
// Explicit == a real, permanent close: close(), context teardown, or an orphaned
Expand Down
Loading