MessagePort alignment with Nodejs

[39ali]

What does this PR do?

fixed 19863 and 19862

• Documentation or TypeScript types (it's okay to leave the rest blank in this case)
• Code changes

How did you verify your code works?

[Jarred-Sumner]

If the RefPtr causes it to be destructed, it would be really easy to lead to a use-after-free.

Instead, we should increment/decrement the reference count while messages are in-flight. Enqueuing a message should increment the reference count of the MessagePort and processing that message should decrement it. To do that, we can use a protectedThis = Ref{ *this } in the lambda capture. That is the typical pattern WebKit uses for this sort of thing.

[39ali]

@Jarred-Sumner that won't work because we need to keep MessagePort alive unitl we call MessagePort::close() for example :

import { MessageChannel } from "node:worker_threads";

function main (){
    const { port1, port2 } = new MessageChannel();

port1.on("message", (msg) =>
  console.log("message received on port 1 via on:", msg),
);
 port2.postMessage("hey");

}

main()

this should never close or get GCed even if we got no messages left but it does in bun unless we keep Ref for MessagePort

another solution could be to keep std::optional<Ref<MessagePort>> and optional::reset() on close()

[39ali]

@Jarred-Sumner any update on this ?

[graphite-app]

The dispatchCloseEvent() call during transfer appears to be inconsistent with the MessagePort specification. According to the standard, MessagePorts should only dispatch 'close' events when explicitly closed via the close() method, not when transferred between contexts. Transferring a port detaches it from its original context but doesn't conceptually "close" it - it continues to exist in the target context. This behavior difference could cause compatibility issues with Node.js and other standard implementations.

Suggested change

	port->dispatchCloseEvent();
	port->detachWithoutClosing();

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

[nazarhussain]

Please see my comment on actual issue. #19863 (comment)

I will emphasis that we should make WebCore implementation to align withe the JS specs and have that NodeJS compatibility layer abstracted away.

And until then just having documentation for such points would be suffice.

[robobun]

Thanks for working on this. Both issues this PR targeted have since been fixed on main by other changes:

• MessagePort does not transmit unless started explicitly #19863 (messages not delivered until start()): fixed by Fix MessagePort communication after transfer to Worker #22638.
• Unexpected MessagePort+Worker behavior on Bun vs NodeJS #19862 (close events, process lifetime): node:worker_threads: +48 Node.js tests passing — MessagePort, stdio, SHARE_ENV, exit codes, transfer semantics, postMessageToThread + inspector #31216 and Worker / worker_threads: WebCore-shaped lifetimes, joined threads, one ordered VM teardown #37075 dispatch the close event natively to both ends of a channel, make a message listener hold the event loop ref, and deliver queued messages before close() completes.

Verified on current main (f426a8e): the #19863 script prints the output of both handlers without an explicit start(), and the #19862 worker script exchanges all four messages, emits close on port2 after terminate(), and exits on its own.

The files this PR touches have also moved since (src/bun.js/bindings/webcore is now src/jsc/bindings/webcore), so closing this rather than asking for a rebase. The one part of this change that is not on main is the close event on the port object that was transferred away (Node emits it); that is being tracked as its own bug.