worker: remove ERR_CLOSED_MESSAGE_PORT

This aligns `MessagePort`s more with the web API. Refs: #26463 PR-URL: #26487 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Mar 14, 2019 · 73370b4 · 73370b4
1 parent 9398d84
commit 73370b4
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 10 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -688,12 +688,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
 
@@ -1975,6 +1969,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

diff --git a/src/node_errors.h b/src/node_errors.h
@@ -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)                                         \
@@ -87,7 +86,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")               \

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -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();
@@ -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();

diff --git a/test/parallel/test-worker-message-port-close.js b/test/parallel/test-worker-message-port-close.js
@@ -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);
+  }));
+}