From 051741cebbb066bcb02d0dccf4c08ca4852738f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Mon, 3 May 2021 13:12:39 -0500 Subject: [PATCH 1/2] Revert "worker: remove `ERR_CLOSED_MESSAGE_PORT`" This reverts commit 73370b45844fd64e9ca8f9270220e425f9ef8ab6. The unit test is preserved to make sure it does not break https://github.com/nodejs/node/issues/26463 again. --- doc/api/errors.md | 6 ++++++ src/node_errors.h | 2 ++ src/node_messaging.cc | 6 +++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index e8b416949da7c8..b8dfeb4e53d573 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -714,6 +714,12 @@ 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. + +### ERR_CLOSED_MESSAGE_PORT + +There was an attempt to use a `MessagePort` instance in a closed +state, usually after `.close()` has been called. + ### `ERR_CONSOLE_WRITABLE_STREAM` diff --git a/src/node_errors.h b/src/node_errors.h index 291365fa3b4dc9..96659f3a400826 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -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) \ @@ -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") \ diff --git a/src/node_messaging.cc b/src/node_messaging.cc index fbe6c407a6cea2..ac4abc3af01568 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1065,7 +1065,11 @@ void MessagePort::MoveToContext(const FunctionCallbackInfo& args) { "The \"port\" argument must be a MessagePort instance"); } MessagePort* port = Unwrap(args[0].As()); - CHECK_NOT_NULL(port); + if (port == nullptr || port->IsHandleClosing()) { + Isolate* isolate = env->isolate(); + THROW_ERR_CLOSED_MESSAGE_PORT(isolate); + return; + } Local context_arg = args[1]; ContextifyContext* context_wrapper; From f7269dcb473c5f36d90e1cb8268f1faa0e4a6208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Sun, 2 May 2021 22:43:08 -0500 Subject: [PATCH 2/2] src: make workers messaging more resilient --- doc/api/errors.md | 22 +++++++++---------- .../test-worker-message-port-close.js | 12 +++++++++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index b8dfeb4e53d573..3ff3c5c1797332 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -715,7 +715,17 @@ 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. -### ERR_CLOSED_MESSAGE_PORT +### `ERR_CLOSED_MESSAGE_PORT` + There was an attempt to use a `MessagePort` instance in a closed state, usually after `.close()` has been called. @@ -2466,16 +2476,6 @@ removed: v12.5.0 The value passed to `postMessage()` contained an object that is not supported for transferring. - -### `ERR_CLOSED_MESSAGE_PORT` - - -There was an attempt to use a `MessagePort` instance in a closed -state, usually after `.close()` has been called. - ### `ERR_CRYPTO_HASH_DIGEST_NO_UTF16`