diff options
author | Anna Henningsen <anna@addaleax.net> | 2020-02-08 18:57:02 +0300 |
---|---|---|
committer | Shelley Vohr <shelley.vohr@gmail.com> | 2020-02-17 23:32:13 +0300 |
commit | 6ad8ca5ecf8bb8482f11f8a57a76cc95c1c5d077 (patch) | |
tree | 7d05e87a140a4bd0438c3b0cccac0fab90ac3308 /src/node_messaging.cc | |
parent | 2837788849fa4f103a54203fb15421663d4d1a31 (diff) |
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>
Diffstat (limited to 'src/node_messaging.cc')
-rw-r--r-- | src/node_messaging.cc | 9 |
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"); } |