src: do not unnecessarily re-assign uv handle data

a555be2e45b283 re-assigned `async_.data` to indicate success
or failure of the constructor. As the `HandleWrap` implementation
uses that field to access the `HandleWrap` instance from the
libuv handle, this introduced two issues:

- It implicitly assumed that casting
  `MessagePort*` → `void*` → `HandleWrap*` would be valid.
- It made the `HandleWrap::OnClose()` function fail with a
  `nullptr` dereference if the constructor did fail.

In particular, the second issue made
test/parallel/test-worker-cleanexit-with-moduleload.js` crash at
least once in CI.

Since re-assigning `async_.data` isn’t actually necessary here
(only a leftover from earlier versions of that commit), fix this by
using a local variable instead, and add a `CHECK` that provides better
error messages for this type of issue in the future.

Refs: https://github.com/nodejs/node/pull/31605

PR-URL: https://github.com/nodejs/node/pull/31696
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

1 files changed, 4 insertions, 5 deletions

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
index f510706fa6e..608f62e7f0a 100644
--- a/src/node_messaging.cc
+++ b/src/node_messaging.cc
@@ -487,10 +487,9 @@ MessagePort::MessagePort(Environment* env,
   CHECK_EQ(uv_async_init(env->event_loop(),
                          &async_,
                          onmessage), 0);
-  async_.data = nullptr;  // Reset later to indicate success of the constructor.
-  auto cleanup = OnScopeLeave([&]() {
-    if (async_.data == nullptr) Close();
-  });
+  // Reset later to indicate success of the constructor.
+  bool succeeded = false;
+  auto cleanup = OnScopeLeave([&]() { if (!succeeded) Close(); });
 
   Local<Value> fn;
   if (!wrap->Get(context, env->oninit_symbol()).ToLocal(&fn))
@@ -507,7 +506,7 @@ MessagePort::MessagePort(Environment* env,
     return;
   emit_message_fn_.Reset(env->isolate(), emit_message_fn);
 
-  async_.data = static_cast<void*>(this);
+  succeeded = true;
   Debug(this, "Created message port");
 }