diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index 7585fb1f..caeb485c 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -58,6 +58,8 @@ using namespace v8; namespace dd { +using ContextPtr = std::shared_ptr>; + // Maximum number of rounds in the GetV8ToEpochOffset static constexpr int MAX_EPOCH_OFFSET_ATTEMPTS = 20; @@ -492,7 +494,6 @@ WallProfiler::WallProfiler(std::chrono::microseconds samplingPeriod, contexts_.reserve(duration * 2 / samplingPeriod); } - curContext_.store(&context1_, std::memory_order_relaxed); collectionMode_.store(CollectionMode::kNoCollect, std::memory_order_relaxed); auto isolate = v8::Isolate::GetCurrent(); @@ -952,28 +953,26 @@ v8::CpuProfiler* WallProfiler::CreateV8CpuProfiler() { } v8::Local WallProfiler::GetContext(Isolate* isolate) { - auto context = *curContext_.load(std::memory_order_relaxed); + auto context = GetContextPtr(); if (!context) return v8::Undefined(isolate); return context->Get(isolate); } void WallProfiler::SetContext(Isolate* isolate, Local value) { - // Need to be careful here, because we might be interrupted by a - // signal handler that will make use of curContext_. - // Update of shared_ptr is not atomic, so instead we use a pointer - // (curContext_) that points on two shared_ptr (context1_ and context2_), - // update the shared_ptr that is not currently in use and then atomically - // update curContext_. - auto newCurContext = curContext_.load(std::memory_order_relaxed) == &context1_ - ? &context2_ - : &context1_; - if (!value->IsNullOrUndefined()) { - *newCurContext = std::make_shared>(isolate, value); - } else { - newCurContext->reset(); - } std::atomic_signal_fence(std::memory_order_release); - curContext_.store(newCurContext, std::memory_order_relaxed); + std::atomic_store_explicit( + &curContext_, + value->IsNullOrUndefined() + ? std::shared_ptr>() + : std::make_shared>(isolate, value), + std::memory_order_relaxed); +} + +ContextPtr WallProfiler::GetContextPtr() { + auto contextPtr = + atomic_load_explicit(&curContext_, std::memory_order_relaxed); + std::atomic_signal_fence(std::memory_order_acquire); + return contextPtr; } NAN_GETTER(WallProfiler::GetContext) { @@ -1007,10 +1006,8 @@ void WallProfiler::PushContext(int64_t time_from, // Be careful this is called in a signal handler context therefore all // operations must be async signal safe (in particular no allocations). // Our ring buffer avoids allocations. - auto context = curContext_.load(std::memory_order_relaxed); - std::atomic_signal_fence(std::memory_order_acquire); if (contexts_.size() < contexts_.capacity()) { - contexts_.push_back({*context, time_from, time_to, cpu_time}); + contexts_.push_back({GetContextPtr(), time_from, time_to, cpu_time}); std::atomic_fetch_add_explicit( reinterpret_cast*>(&fields_[kSampleCount]), 1U, diff --git a/bindings/profilers/wall.hh b/bindings/profilers/wall.hh index 91871b0f..1832dfbe 100644 --- a/bindings/profilers/wall.hh +++ b/bindings/profilers/wall.hh @@ -51,11 +51,7 @@ class WallProfiler : public Nan::ObjectWrap { // avoid heap allocation. Need to figure out the right move/copy semantics in // and out of the ring buffer. - // We're using a pair of shared pointers and an atomic pointer-to-current as - // a way to ensure signal safety on update. - ContextPtr context1_; - ContextPtr context2_; - std::atomic curContext_; + ContextPtr curContext_; std::atomic collectionMode_; std::atomic noCollectCallCount_; @@ -99,6 +95,7 @@ class WallProfiler : public Nan::ObjectWrap { int64_t startCpuTime); bool waitForSignal(uint64_t targetCallCount = 0); + ContextPtr GetContextPtr(); public: /**