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

src: make workers messaging more resilient #38510

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
26 changes: 16 additions & 10 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,22 @@ 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`
<!--
added: REPLACEME
changes:
- version: 11.12.0
pr-url: https://github.com/nodejs/node/pull/26487
description: The error message was removed.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/38510
description: The error message was reintroduced.
-->

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 @@ -2460,16 +2476,6 @@ removed: v12.5.0
The value passed to `postMessage()` contained an object that is not supported
for transferring.

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

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

<a id="ERR_CRYPTO_HASH_DIGEST_NO_UTF16"></a>
### `ERR_CRYPTO_HASH_DIGEST_NO_UTF16`
<!-- YAML
Expand Down
2 changes: 2 additions & 0 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ void OnFatalError(const char* location, const char* message);
V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, Error) \
V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \
V(ERR_BUFFER_TOO_LARGE, Error) \
V(ERR_CLOSED_MESSAGE_PORT, Error) \
V(ERR_CONSTRUCT_CALL_REQUIRED, TypeError) \
V(ERR_CONSTRUCT_CALL_INVALID, TypeError) \
V(ERR_CRYPTO_INITIALIZATION_FAILED, Error) \
Expand Down Expand Up @@ -118,6 +119,7 @@ ERRORS_WITH_CODE(V)
#define PREDEFINED_ERROR_MESSAGES(V) \
V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, \
"Buffer is not available for the current Context") \
V(ERR_CLOSED_MESSAGE_PORT, "Cannot send data on closed MessagePort") \
V(ERR_CONSTRUCT_CALL_INVALID, "Constructor cannot be called") \
V(ERR_CONSTRUCT_CALL_REQUIRED, "Cannot call constructor without `new`") \
V(ERR_CRYPTO_INITIALIZATION_FAILED, "Initialization failed") \
Expand Down
6 changes: 5 additions & 1 deletion src/node_messaging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,11 @@ void MessagePort::MoveToContext(const FunctionCallbackInfo<Value>& args) {
"The \"port\" argument must be a MessagePort instance");
}
MessagePort* port = Unwrap<MessagePort>(args[0].As<Object>());
CHECK_NOT_NULL(port);
if (port == nullptr || port->IsHandleClosing()) {
Isolate* isolate = env->isolate();
THROW_ERR_CLOSED_MESSAGE_PORT(isolate);
return;
}

Local<Value> context_arg = args[1];
ContextifyContext* context_wrapper;
Expand Down
12 changes: 11 additions & 1 deletion test/parallel/test-worker-message-port-close.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
const common = require('../common');
const { MessageChannel } = require('worker_threads');
const assert = require('assert');
const { MessageChannel, moveMessagePortToContext } = require('worker_threads');

// Make sure that .start() and .stop() do not throw on closing/closed
// MessagePorts.
Expand Down Expand Up @@ -29,3 +30,12 @@ function dummy() {}
port1.off('message', dummy);
}));
}

{
const { port2 } = new MessageChannel();
port2.close();
assert.throws(() => moveMessagePortToContext(port2, {}), {
code: 'ERR_CLOSED_MESSAGE_PORT',
message: 'Cannot send data on closed MessagePort'
});
}