Remove AtomicContextPtr in favor of setInProgress_

Author: szegedi

bindings/profilers/wall.cc

@@ -58,6 +58,20 @@ using namespace v8;
 
 namespace dd {
 
+class SignalMutex {
+  std::atomic<bool>& mutex_;
+
+  inline void store(bool value) {
+    std::atomic_signal_fence(std::memory_order_release);
+    mutex_.store(value, std::memory_order_relaxed);
+  }
+
+ public:
+  inline SignalMutex(std::atomic<bool>& mutex) : mutex_(mutex) { store(true); }
+
+  inline ~SignalMutex() { store(false); }
+};
+
 // Maximum number of rounds in the GetV8ToEpochOffset
 static constexpr int MAX_EPOCH_OFFSET_ATTEMPTS = 20;
 
@@ -620,7 +634,18 @@ void WallProfiler::Dispose(Isolate* isolate, bool removeFromMap) {
   }
 }
 
-class PersistentContextPtr : AtomicContextPtr {
+void SetContextPtr(ContextPtr& contextPtr,
+                   Isolate* isolate,
+                   Local<Value> value) {
+  if (contextPtr) {
+    contextPtr = std::make_shared<Global<Value>>(isolate, value);
+  } else {
+    contextPtr.reset();
+  }
+}
+
+class PersistentContextPtr {
+  ContextPtr context;
   std::vector<PersistentContextPtr*>* dead;
   Persistent<Object> per;
 
@@ -648,6 +673,14 @@ class PersistentContextPtr : AtomicContextPtr {
         WeakCallbackType::kParameter);
   }
 
+  void Set(Isolate* isolate, const Local<Value>& value) {
+    SetContextPtr(context, isolate, value);
+  }
+
+  ContextPtr Get() const {
+    return context;
+  }
+
   friend class WallProfiler;
 };
 
@@ -1073,10 +1106,15 @@ Local<Value> WallProfiler::GetContext(Isolate* isolate) {
   return Undefined(isolate);
 }
 
+void WallProfiler::SetCurrentContextPtr(Isolate* isolate, Local<Value> value) {
+  SignalMutex m(setInProgress_);
+  SetContextPtr(curContext_, isolate, value);
+}
+
 void WallProfiler::SetContext(Isolate* isolate, Local<Value> value) {
 #if NODE_MAJOR_VERSION >= 23
   if (!useCPED_) {
-    curContext_.Set(isolate, value);
+    SetCurrentContextPtr(isolate, value);
     return;
   }
 
@@ -1102,15 +1140,12 @@ void WallProfiler::SetContext(Isolate* isolate, Local<Value> value) {
 
   PersistentContextPtr* contextPtr = nullptr;
   auto profData = maybeProfData.ToLocalChecked();
+  SignalMutex m(setInProgress_);
   if (profData->IsUndefined()) {
     contextPtr = new PersistentContextPtr(&deadContextPtrs_);
 
     auto external = External::New(isolate, contextPtr);
-    setInProgress_.store(true, std::memory_order_relaxed);
-    std::atomic_signal_fence(std::memory_order_release);
     auto maybeSetResult = cpedObj->SetPrivate(v8Ctx, localSymbol, external);
-    std::atomic_signal_fence(std::memory_order_release);
-    setInProgress_.store(false, std::memory_order_relaxed);
     if (maybeSetResult.IsNothing()) {
       delete contextPtr;
       return;
@@ -1124,7 +1159,7 @@ void WallProfiler::SetContext(Isolate* isolate, Local<Value> value) {
 
   contextPtr->Set(isolate, value);
 #else
-  curContext_.Set(isolate, value);
+  SetCurrentContextPtr(isolate, value);
 #endif
 }
 
@@ -1151,7 +1186,7 @@ ContextPtr WallProfiler::GetContextPtrSignalSafe(Isolate* isolate) {
 ContextPtr WallProfiler::GetContextPtr(Isolate* isolate) {
 #if NODE_MAJOR_VERSION >= 23
   if (!useCPED_) {
-    return curContext_.Get();
+    return curContext_;
   }
 
   if (!isolate->IsInUse()) {
@@ -1179,7 +1214,7 @@ ContextPtr WallProfiler::GetContextPtr(Isolate* isolate) {
   }
   return ContextPtr();
 #else
-  return curContext_.Get();
+  return curContext_;
 #endif
 }
 

bindings/profilers/wall.hh

@@ -38,39 +38,6 @@ struct Result {
 
 using ContextPtr = std::shared_ptr<v8::Global<v8::Value>>;
 
-/**
- * Class that allows atomic updates to a ContextPtr. Since update of shared_ptr
- * is not atomic, we use a pointer that alternates between pointing to one of
- * two shared_ptrs instead, and we use atomic operations to update the pointer.
- */
-class AtomicContextPtr {
-  ContextPtr ptr1;
-  ContextPtr ptr2;
-  std::atomic<ContextPtr*> currentPtr = &ptr1;
-
-  void Set(v8::Isolate* isolate, v8::Local<v8::Value> value) {
-    auto oldPtr = currentPtr.load(std::memory_order_relaxed);
-    std::atomic_signal_fence(std::memory_order_acquire);
-    auto newPtr = oldPtr == &ptr1 ? &ptr2 : &ptr1;
-    if (!value->IsNullOrUndefined()) {
-      *newPtr = std::make_shared<v8::Global<v8::Value>>(isolate, value);
-    } else {
-      newPtr->reset();
-    }
-    std::atomic_signal_fence(std::memory_order_release);
-    currentPtr.store(newPtr, std::memory_order_relaxed);
-    std::atomic_signal_fence(std::memory_order_release);
-  }
-
-  ContextPtr Get() {
-    auto ptr = currentPtr.load(std::memory_order_relaxed);
-    std::atomic_signal_fence(std::memory_order_acquire);
-    return ptr ? *ptr : ContextPtr();
-  }
-
-  friend class WallProfiler;
-};
-
 class PersistentContextPtr;
 
 class WallProfiler : public Nan::ObjectWrap {
@@ -85,7 +52,7 @@ class WallProfiler : public Nan::ObjectWrap {
 
   bool useCPED_ = false;
   // If we aren't using the CPED, we use a single context ptr stored here.
-  AtomicContextPtr curContext_;
+  ContextPtr curContext_;
   // Otherwise we'll use a private symbol to store the context in CPED objects.
   v8::Global<v8::Private> cpedSymbol_;
   // We track live context pointers in a set to avoid memory leaks. They will
@@ -151,6 +118,8 @@ class WallProfiler : public Nan::ObjectWrap {
   ContextPtr GetContextPtr(v8::Isolate* isolate);
   ContextPtr GetContextPtrSignalSafe(v8::Isolate* isolate);
 
+  void SetCurrentContextPtr(v8::Isolate* isolate, v8::Local<v8::Value> context);
+
  public:
   /**
    * @param samplingPeriodMicros sampling interval, in microseconds