mirror of
https://github.com/zebrajr/node.git
synced 2026-01-15 12:15:26 +00:00
src: do not unnecessarily re-assign uv handle data
a555be2e45 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>
This commit is contained in:
@@ -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();
|
||||
|
||||
|
||||
@@ -488,10 +488,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))
|
||||
@@ -508,7 +507,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");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user