Skip to content
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.

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

worker: align MessagePort message handling more with Web #26487

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 10 additions & 6 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -685,12 +685,6 @@ Used when a child process is being forked without specifying an IPC channel.
Used when the main process is trying to read data from the child process's
STDERR/STDOUT, and the data's length is longer than the `maxBuffer` option.

<a id="ERR_CLOSED_MESSAGE_PORT"></a>
### ERR_CLOSED_MESSAGE_PORT

There was an attempt to use a `MessagePort` instance in a closed
state, usually after `.close()` has been called.

<a id="ERR_CONSOLE_WRITABLE_STREAM"></a>
### ERR_CONSOLE_WRITABLE_STREAM

Expand Down Expand Up @@ -1983,6 +1977,16 @@ A module file could not be resolved while attempting a [`require()`][] or
> Stability: 0 - Deprecated. These error codes are either inconsistent, or have
> been removed.

<a id="ERR_CLOSED_MESSAGE_PORT"></a>
### ERR_CLOSED_MESSAGE_PORT
<!-- YAML
added: v10.5.0
removed: REPLACEME
-->

There was an attempt to use a `MessagePort` instance in a closed
state, usually after `.close()` has been called.

<a id="ERR_HTTP2_FRAME_ERROR"></a>
### ERR_HTTP2_FRAME_ERROR
<!-- YAML
Expand Down
3 changes: 3 additions & 0 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,9 @@ listeners are attached.

This method exists for parity with the Web `MessagePort` API. In Node.js,
it is only useful for ignoring messages when no event listener is present.
Node.js also diverges in its handling of `.onmessage`. Setting it will
automatically call `.start()`, but unsetting it will let messages queue up
until a new handler is set or the port is discarded.

### port.unref()
<!-- YAML
Expand Down
2 changes: 0 additions & 2 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ void FatalException(const v8::FunctionCallbackInfo<v8::Value>& args);
V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \
V(ERR_BUFFER_TOO_LARGE, Error) \
V(ERR_CANNOT_TRANSFER_OBJECT, TypeError) \
V(ERR_CLOSED_MESSAGE_PORT, Error) \
V(ERR_CONSTRUCT_CALL_REQUIRED, Error) \
V(ERR_INVALID_ARG_VALUE, TypeError) \
V(ERR_INVALID_ARG_TYPE, TypeError) \
Expand Down Expand Up @@ -88,7 +87,6 @@ void FatalException(const v8::FunctionCallbackInfo<v8::Value>& args);
V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, \
"Buffer is not available for the current Context") \
V(ERR_CANNOT_TRANSFER_OBJECT, "Cannot transfer object of unsupported type")\
V(ERR_CLOSED_MESSAGE_PORT, "Cannot send data on closed MessagePort") \
V(ERR_CONSTRUCT_CALL_REQUIRED, "Cannot call constructor without `new`") \
V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList") \
V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") \
Expand Down
2 changes: 0 additions & 2 deletions src/node_messaging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,6 @@ void MessagePort::Start(const FunctionCallbackInfo<Value>& args) {
MessagePort* port;
ASSIGN_OR_RETURN_UNWRAP(&port, args.This());
if (!port->data_) {
THROW_ERR_CLOSED_MESSAGE_PORT(env);
return;
}
port->Start();
Expand All @@ -741,7 +740,6 @@ void MessagePort::Stop(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsObject());
ASSIGN_OR_RETURN_UNWRAP(&port, args[0].As<Object>());
if (!port->data_) {
THROW_ERR_CLOSED_MESSAGE_PORT(env);
return;
}
port->Stop();
Expand Down
31 changes: 31 additions & 0 deletions test/parallel/test-worker-message-port-close.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';
const common = require('../common');
const { MessageChannel } = require('worker_threads');

// Make sure that .start() and .stop() do not throw on closing/closed
// MessagePorts.
// Refs: https://github.com/nodejs/node/issues/26463

function dummy() {}

{
const { port1, port2 } = new MessageChannel();
port1.close(common.mustCall(() => {
port1.on('message', dummy);
port1.off('message', dummy);
port2.on('message', dummy);
port2.off('message', dummy);
}));
port1.on('message', dummy);
port1.off('message', dummy);
port2.on('message', dummy);
port2.off('message', dummy);
}

{
const { port1 } = new MessageChannel();
port1.on('message', dummy);
port1.close(common.mustCall(() => {
port1.off('message', dummy);
}));
}