fix(profiling): crash with synchronized_sample_pool on uWSGI shutdown [backport 2.21] (#13401)

backport from #13370

(cherry picked from commit 9b8d0cb)

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Author: realFlowControl

ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt

@@ -49,7 +49,7 @@ add_library(
     src/receiver_interface.cpp
     src/sample.cpp
     src/sample_manager.cpp
-    src/synchronized_sample_pool.cpp
+    src/static_sample_pool.cpp
     src/uploader.cpp
     src/uploader_builder.cpp)
 

ddtrace/internal/datadog/profiling/dd_wrapper/include/constants.hpp

@@ -14,7 +14,7 @@ constexpr unsigned int g_backend_max_nframes = 512;
 // Maximum amount of time, in milliseconds, to wait for crashtracker signal handler
 constexpr uint64_t g_crashtracker_timeout_ms = 5000;
 
-// Default value for the max number of samples to keep in the SynchronizedSamplePool
+// Default value for the max number of samples to keep in the StaticSamplePool
 constexpr size_t g_default_sample_pool_capacity = 4;
 
 // Default name of the runtime.  This will almost certainly get overridden by the caller, but we set it here

ddtrace/internal/datadog/profiling/dd_wrapper/include/sample_manager.hpp

@@ -2,7 +2,6 @@
 
 #include "constants.hpp"
 #include "sample.hpp"
-#include "synchronized_sample_pool.hpp"
 #include "types.hpp"
 
 #include <array>
@@ -20,7 +19,6 @@ class SampleManager
     static inline unsigned int max_nframes{ g_default_max_nframes };
     static inline SampleType type_mask{ SampleType::All };
     static inline size_t sample_pool_capacity{ g_default_sample_pool_capacity };
-    static inline std::unique_ptr<SynchronizedSamplePool> sample_pool{ nullptr };
 
   public:
     // Configuration

ddtrace/internal/datadog/profiling/dd_wrapper/include/static_sample_pool.hpp

@@ -0,0 +1,24 @@
+#pragma once
+
+#include <cstddef>
+#include <optional>
+
+#include "sample.hpp"
+
+namespace Datadog {
+
+class StaticSamplePool
+{
+public:
+    static constexpr std::size_t CAPACITY = g_default_sample_pool_capacity;
+
+    static std::optional<Sample*> take_sample();
+    static std::optional<Sample*> return_sample(Sample* sample);
+
+private:
+    StaticSamplePool() = delete;
+    StaticSamplePool(const StaticSamplePool&) = delete;
+    StaticSamplePool& operator=(const StaticSamplePool&) = delete;
+};
+
+} // namespace Datadog

ddtrace/internal/datadog/profiling/dd_wrapper/include/synchronized_sample_pool.hpp

@@ -1,4 +1,5 @@
 #include "sample_manager.hpp"
+#include "static_sample_pool.hpp"
 #include "types.hpp"
 
 void
@@ -38,11 +39,7 @@ Datadog::SampleManager::set_sample_pool_capacity(size_t capacity)
 Datadog::Sample*
 Datadog::SampleManager::start_sample()
 {
-    if (sample_pool == nullptr) {
-        return new Datadog::Sample(type_mask, max_nframes); // NOLINT(cppcoreguidelines-owning-memory)
-    }
-
-    auto sample_opt = sample_pool->take_sample();
+    auto sample_opt = StaticSamplePool::take_sample();
     if (sample_opt.has_value()) {
         return sample_opt.value();
     }
@@ -56,14 +53,7 @@ Datadog::SampleManager::start_sample()
 void
 Datadog::SampleManager::drop_sample(Datadog::Sample* sample)
 {
-    if (sample_pool == nullptr) {
-        delete sample; // NOLINT(cppcoreguidelines-owning-memory)
-        return;
-    }
-
-    sample->clear_buffers();
-
-    std::optional<Sample*> result_opt = sample_pool->return_sample(sample);
+    std::optional<Sample*> result_opt = StaticSamplePool::return_sample(sample);
     // If the pool is full, the pool returns the pointer and we need to delete the sample.
     if (result_opt.has_value()) {
         delete result_opt.value(); // NOLINT(cppcoreguidelines-owning-memory)
@@ -74,19 +64,10 @@ void
 Datadog::SampleManager::postfork_child()
 {
     Datadog::Sample::postfork_child();
-    if (sample_pool != nullptr) {
-        // Clear the pool to make sure it's in a consistent state.
-        // Suppose there was a thread that was adding/removing sample from the pool
-        // and the fork happened in the middle of that operation.
-        sample_pool = std::make_unique<SynchronizedSamplePool>(sample_pool_capacity);
-    }
 }
 
 void
 Datadog::SampleManager::init()
 {
-    if (sample_pool == nullptr) {
-        sample_pool = std::make_unique<SynchronizedSamplePool>(sample_pool_capacity);
-    }
     Datadog::Sample::profile_state.one_time_init(type_mask, max_nframes);
 }

ddtrace/internal/datadog/profiling/dd_wrapper/src/sample_manager.cpp

@@ -0,0 +1,41 @@
+#include "static_sample_pool.hpp"
+#include "sample.hpp"
+#include <optional>
+#include <atomic>
+#include <cstddef>
+
+namespace Datadog {
+
+static constexpr std::size_t CAPACITY = StaticSamplePool::CAPACITY;
+static std::atomic<Sample*> pool[CAPACITY];
+
+struct StaticSamplePoolInitializer {
+    StaticSamplePoolInitializer() {
+        for (std::size_t i = 0; i < CAPACITY; ++i) {
+            pool[i].store(nullptr, std::memory_order_relaxed);
+        }
+    }
+} initializer;
+
+std::optional<Sample*> StaticSamplePool::take_sample() {
+    for (std::size_t i = 0; i < CAPACITY; ++i) {
+        Sample* s = pool[i].exchange(nullptr, std::memory_order_acq_rel);
+        if (s != nullptr) {
+            return s;
+        }
+    }
+    return std::nullopt;
+}
+
+std::optional<Sample*> StaticSamplePool::return_sample(Sample* sample) {
+    for (std::size_t i = 0; i < CAPACITY; ++i) {
+        Sample* expected = nullptr;
+        if (pool[i].compare_exchange_strong(expected, sample,
+                                            std::memory_order_acq_rel,
+                                            std::memory_order_relaxed)) {
+            return std::nullopt;
+        }
+    }
+    return sample;
+}
+} // namespace Datadog

ddtrace/internal/datadog/profiling/dd_wrapper/src/static_sample_pool.cpp

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    profiling: fixed an issue in the ``SynchronizedSamplePool`` where pool
+    could be null when calling into ``ddog_ArrayQueue_`` functions, leading to
+    segfaults in the uWSGI shutdown