diff --git a/src/node_worker.cc b/src/node_worker.cc index acc1d56d6ad329..701d90cfeeca2b 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -63,18 +63,18 @@ Worker::Worker(Environment* env, thread_id_.id); // Set up everything that needs to be set up in the parent environment. - parent_port_ = MessagePort::New(env, env->context()); - if (parent_port_ == nullptr) { + MessagePort* parent_port = MessagePort::New(env, env->context()); + if (parent_port == nullptr) { // This can happen e.g. because execution is terminating. return; } child_port_data_ = std::make_unique(nullptr); - MessagePort::Entangle(parent_port_, child_port_data_.get()); + MessagePort::Entangle(parent_port, child_port_data_.get()); - object()->Set(env->context(), - env->message_port_string(), - parent_port_->object()).Check(); + object() + ->Set(env->context(), env->message_port_string(), parent_port->object()) + .Check(); object()->Set(env->context(), env->thread_id_string(), @@ -734,10 +734,6 @@ void Worker::Exit(int code, const char* error_code, const char* error_message) { } } -void Worker::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("parent_port", parent_port_); -} - bool Worker::IsNotIndicativeOfMemoryLeakAtExit() const { // Worker objects always stay alive as long as the child thread, regardless // of whether they are being referenced in the parent thread. diff --git a/src/node_worker.h b/src/node_worker.h index faf28b04d334d1..b3814066287d4b 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -51,7 +51,7 @@ class Worker : public AsyncWrap { template inline bool RequestInterrupt(Fn&& cb); - void MemoryInfo(MemoryTracker* tracker) const override; + SET_NO_MEMORY_INFO() SET_MEMORY_INFO_NAME(Worker) SET_SELF_SIZE(Worker) bool IsNotIndicativeOfMemoryLeakAtExit() const override; @@ -111,10 +111,6 @@ class Worker : public AsyncWrap { std::unique_ptr child_port_data_; std::shared_ptr env_vars_; - // This is always kept alive because the JS object associated with the Worker - // instance refers to it via its [kPort] property. - MessagePort* parent_port_ = nullptr; - // A raw flag that is used by creator and worker threads to // sync up on pre-mature termination of worker - while in the // warmup phase. Once the worker is fully warmed up, use the diff --git a/test/parallel/test-worker-exit-heapsnapshot.js b/test/parallel/test-worker-exit-heapsnapshot.js new file mode 100644 index 00000000000000..a7b7b26ecaefd7 --- /dev/null +++ b/test/parallel/test-worker-exit-heapsnapshot.js @@ -0,0 +1,17 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { getHeapSnapshot } = require('v8'); +const { isMainThread, Worker } = require('worker_threads'); + +// Checks taking heap snapshot at the exit event listener of Worker doesn't +// crash the process. +// Regression for https://github.com/nodejs/node/issues/43122. +if (isMainThread) { + const worker = new Worker(__filename); + + worker.once('exit', common.mustCall((code) => { + assert.strictEqual(code, 0); + getHeapSnapshot().pipe(process.stdout); + })); +} diff --git a/test/pummel/test-heapdump-worker.js b/test/pummel/test-heapdump-worker.js index 0e8322affb5d64..d494de50922155 100644 --- a/test/pummel/test-heapdump-worker.js +++ b/test/pummel/test-heapdump-worker.js @@ -6,14 +6,6 @@ const { Worker } = require('worker_threads'); validateSnapshotNodes('Node / Worker', []); const worker = new Worker('setInterval(() => {}, 100);', { eval: true }); -validateSnapshotNodes('Node / Worker', [ - { - children: [ - { node_name: 'Node / MessagePort', edge_name: 'parent_port' }, - { node_name: 'Worker', edge_name: 'wrapped' }, - ] - }, -]); validateSnapshotNodes('Node / MessagePort', [ { children: [