diff options
author | Chengzhong Wu <chengzhong.wcz@alibaba-inc.com> | 2022-05-17 06:40:40 +0300 |
---|---|---|
committer | Bryan English <bryan@bryanenglish.com> | 2022-05-30 19:33:53 +0300 |
commit | d91b489784df7fe591525d04cafb8ad175fc8d2a (patch) | |
tree | c18af1396c685ab16d9bd30cff03bbf8b3a2469b /src | |
parent | 11c783fa63733a8cff9d983b233c04fac88359b3 (diff) |
worker: fix heap snapshot crash on exit
Worker.parent_port_ can be released before the exit event of Worker. The
parent_port_ is not owned by `node::worker::Worker`, thus the reference
to it is not always valid, and accessing it at exit crashes the process.
As the Worker.parent_port_ is only used in the memory info tracking, it
can be safely removed.
PR-URL: https://github.com/nodejs/node/pull/43123
Fixes: https://github.com/nodejs/node/issues/43122
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Diffstat (limited to 'src')
-rw-r--r-- | src/node_worker.cc | 16 | ||||
-rw-r--r-- | src/node_worker.h | 6 |
2 files changed, 7 insertions, 15 deletions
diff --git a/src/node_worker.cc b/src/node_worker.cc index 1009c0788cc..8385bc96231 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<MessagePortData>(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 faf28b04d33..b3814066287 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -51,7 +51,7 @@ class Worker : public AsyncWrap { template <typename Fn> 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<MessagePortData> child_port_data_; std::shared_ptr<KVStore> 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 |