From f18c40ba0ec8aaaf402a6ded08304952ccc881a4 Mon Sep 17 00:00:00 2001
From: Trevor Norris <trev.norris@gmail.com>
Date: Mon, 30 Oct 2023 11:59:32 -0600
Subject: [PATCH] src: don't allow parallel calls to Update()

ThreadMetrics::Update() is supposed to be thread-safe, so we need to
guard against calls being made in parallel. Return UV_EBUSY if an
asynchronous Update() is already being processed.

PR-URL: https://github.com/nodesource/nsolid/pull/13
Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
---
 src/nsolid.cc |  5 +++++
 src/nsolid.h  | 22 +++++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/nsolid.cc b/src/nsolid.cc
index 289888b66d..1ad216b8ef 100644
--- a/src/nsolid.cc
+++ b/src/nsolid.cc
@@ -326,6 +326,11 @@ int ThreadMetrics::Update(v8::Isolate* isolate) {
   if (envinst == nullptr || envinst->thread_id() != thread_id_) {
     return UV_ESRCH;
   }
+  // An async update request is currently in process. Let that complete before
+  // running Update() again.
+  if (update_running_) {
+    return UV_EBUSY;
+  }
 
   uv_mutex_lock(&stor_lock_);
   envinst->GetThreadMetrics(&stor_);
diff --git a/src/nsolid.h b/src/nsolid.h
index 6cadb05eb8..914269d7cf 100644
--- a/src/nsolid.h
+++ b/src/nsolid.h
@@ -693,6 +693,7 @@ class NODE_EXTERN ThreadMetrics {
   void* user_data_ = nullptr;
   thread_metrics_proxy_sig proxy_;
 
+  std::atomic<bool> update_running_ = {false};
   uv_mutex_t stor_lock_;
   MetricsStor stor_;
 };
@@ -976,22 +977,32 @@ class NODE_EXTERN Snapshot {
 /** @cond DONT_DOCUMENT */
 template <typename Cb, typename... Data>
 int ThreadMetrics::Update(Cb&& cb, Data&&... data) {
+  bool expected = false;
   // NOLINTNEXTLINE(build/namespaces)
   using namespace std::placeholders;
   using UserData = decltype(std::bind(
         std::forward<Cb>(cb), _1, std::forward<Data>(data)...));
 
+  update_running_.compare_exchange_strong(expected, true);
+  if (expected) {
+    return UV_EBUSY;
+  }
+
   // _1 - ThreadMetrics*
-  std::unique_ptr<UserData> user_data = std::make_unique<UserData>(std::bind(
-        std::forward<Cb>(cb), _1, std::forward<Data>(data)...));
+  UserData* user_data = new UserData(
+      std::bind(std::forward<Cb>(cb), _1, std::forward<Data>(data)...));
 
-  user_data_ = static_cast<void*>(user_data.get());
+  user_data_ = user_data;
   proxy_ = thread_metrics_proxy_<UserData>;
   stor_.thread_id = thread_id_;
 
   int er = get_thread_metrics_();
-  if (!er)
-    user_data.release();
+  if (er) {
+    user_data_ = nullptr;
+    proxy_ = nullptr;
+    delete user_data;
+    update_running_ = false;
+  }
   return er;
 }
 
@@ -1001,6 +1012,7 @@ void ThreadMetrics::thread_metrics_proxy_(ThreadMetrics* tm) {
   G* g = static_cast<G*>(tm->user_data_);
   tm->user_data_ = nullptr;
   tm->proxy_ = nullptr;
+  tm->update_running_ = false;
   (*g)(tm);
   delete g;
 }