diff options
author | Anna Henningsen <anna@addaleax.net> | 2020-02-08 18:57:02 +0300 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2020-02-10 19:27:53 +0300 |
commit | f938cbd77d487559d273a975bb43ecf8439f9146 (patch) | |
tree | 61cc54b30f6b7d4ab1b11bdacea07e9b3900a068 /src/handle_wrap.cc | |
parent | 0ac04ecee20f00cf3daaf006496d461bb9fecbc2 (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/handle_wrap.cc')
-rw-r--r-- | src/handle_wrap.cc | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index f5d622fc255..4e039d5dbf2 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -115,6 +115,7 @@ HandleWrap::HandleWrap(Environment* env, void HandleWrap::OnClose(uv_handle_t* handle) { + CHECK_NOT_NULL(handle->data); BaseObjectPtr<HandleWrap> wrap { static_cast<HandleWrap*>(handle->data) }; wrap->Detach(); |