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: #13 Reviewed-by: Santiago Gimeno <[email protected]>
nodesource · Dec 7, 2023 · f18c40b · f18c40b
1 parent 2744a99
commit f18c40b
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 5 deletions.
diff --git 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
@@ -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;
 }