messaging: fix edge cases with transferring ports

Currently, transferring the port on which postMessage is called causes a
segmentation fault, and transferring the target port causes a subsequent
port.onmessage setting to throw, or a deadlock if onmessage is set
before the postMessage. Fix both of these behaviors and align the
methods more closely with the normative definitions in the HTML
Standard.

Also, per spec postMessage must not throw just because the ports are
disentangled. Implement that behavior.

PR-URL: https://github.com/nodejs/node/pull/21540
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

1 files changed, 26 insertions, 5 deletions

diff --git a/src/node_messaging.h b/src/node_messaging.h
index 28122c526c..62ae633b9e 100644
--- a/src/node_messaging.h
+++ b/src/node_messaging.h
@@ -32,10 +32,14 @@ class Message {
   // Serialize a JS value, and optionally transfer objects, into this message.
   // The Message object retains ownership of all transferred objects until
   // deserialization.
+  // The source_port parameter, if provided, will make Serialize() throw a
+  // "DataCloneError" DOMException if source_port is found in transfer_list.
   v8::Maybe<bool> Serialize(Environment* env,
                             v8::Local<v8::Context> context,
                             v8::Local<v8::Value> input,
-                            v8::Local<v8::Value> transfer_list);
+                            v8::Local<v8::Value> transfer_list,
+                            v8::Local<v8::Object> source_port =
+                                v8::Local<v8::Object>());
 
   // Internal method of Message that is called when a new SharedArrayBuffer
   // object is encountered in the incoming value's structure.
@@ -44,6 +48,13 @@ class Message {
   // and that transfers ownership of `data` to this message.
   void AddMessagePort(std::unique_ptr<MessagePortData>&& data);
 
+  // The MessagePorts that will be transferred, as recorded by Serialize().
+  // Used for warning user about posting the target MessagePort to itself,
+  // which will as a side effect destroy the communication channel.
+  const std::vector<std::unique_ptr<MessagePortData>>& message_ports() const {
+    return message_ports_;
+  }
+
  private:
   MallocedBuffer<char> main_message_buf_;
   std::vector<MallocedBuffer<char>> array_buffer_contents_;
@@ -122,10 +133,11 @@ class MessagePort : public HandleWrap {
                           std::unique_ptr<MessagePortData> data = nullptr);
 
   // Send a message, i.e. deliver it into the sibling's incoming queue.
-  // If there is no sibling, i.e. this port is closed,
-  // this message is silently discarded.
-  void Send(Message&& message);
-  void Send(const v8::FunctionCallbackInfo<v8::Value>& args);
+  // If this port is closed, or if there is no sibling, this message is
+  // serialized with transfers, then silently discarded.
+  v8::Maybe<bool> PostMessage(Environment* env,
+                              v8::Local<v8::Value> message,
+                              v8::Local<v8::Value> transfer);
   // Deliver a single message into this port's incoming queue.
   void AddToIncomingQueue(Message&& message);
 
@@ -157,6 +169,15 @@ class MessagePort : public HandleWrap {
   void Close(
       v8::Local<v8::Value> close_callback = v8::Local<v8::Value>()) override;
 
+  // Returns true if either data_ has been freed, or if the handle is being
+  // closed. Equivalent to the [[Detached]] internal slot in the HTML Standard.
+  //
+  // If checking if a JavaScript MessagePort object is detached, this method
+  // alone is often not enough, since the backing C++ MessagePort object may
+  // have been deleted already. For all intents and purposes, an object with a
+  // NULL pointer to the C++ MessagePort object is also detached.
+  inline bool IsDetached() const;
+
   size_t self_size() const override;
 
  private: