chore(profiling): make sanitizers happy

ddtrace/internal/datadog/profiling/cmake/FindLibdatadog.cmake

@@ -5,7 +5,7 @@ endif()
 
 include(ExternalProject)
 set(TAG_LIBDATADOG
-    "v14.1.0"
+    "v14.3.1"
     CACHE STRING "libdatadog github tag")
 
 set(Datadog_BUILD_DIR ${CMAKE_BINARY_DIR}/libdatadog)

ddtrace/internal/datadog/profiling/cmake/tools/libdatadog_checksums.txt

@@ -1,5 +1,5 @@
-fc6be3383d3a115804c43e2c66dd176c63f33b362d987d9b1211034e2b549c2d libdatadog-aarch64-alpine-linux-musl.tar.gz
-b9c972afea19696ee6a459d2fa65563b738baf77dcb12739c8e4ae44d1c975fb libdatadog-aarch64-apple-darwin.tar.gz
-1a9bc4d99d23f7baf403b6b7527f9b9d76bdb166dc34656150561dcb148cc90b libdatadog-aarch64-unknown-linux-gnu.tar.gz
-8244831681332dfa939eefe6923fe6a8beaffff48cb336f836b55a438078add1 libdatadog-x86_64-alpine-linux-musl.tar.gz
-76fcb3bfe3b3971d77f6dd4968ffe6bd5f6a1ada82e2e990a78919107dc2ee40 libdatadog-x86_64-unknown-linux-gnu.tar.gz
+57f83aff275628bb1af89c22bb4bd696726daf2a9e09b6cd0d966b29e65a7ad6 libdatadog-aarch64-alpine-linux-musl.tar.gz
+2be2efa98dfc32f109abdd79242a8e046a7a300c77634135eb293e000ecd4a4c libdatadog-aarch64-apple-darwin.tar.gz
+36db8d50ccabb71571158ea13835c0f1d05d30b32135385f97c16343cfb6ddd4 libdatadog-aarch64-unknown-linux-gnu.tar.gz
+2f61fd21cf2f8147743e414b4a8c77250a17be3aecc42a69ffe54f0a603d5c92 libdatadog-x86_64-alpine-linux-musl.tar.gz
+f01f05600591063eba4faf388f54c155ab4e6302e5776c7855e3734955f7daf7 libdatadog-x86_64-unknown-linux-gnu.tar.gz

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

@@ -19,7 +19,6 @@ class SampleManager
   private:
     static inline unsigned int max_nframes{ g_default_max_nframes };
     static inline SampleType type_mask{ SampleType::All };
-    static inline std::mutex init_mutex{};
     static inline size_t sample_pool_capacity{ g_default_sample_pool_capacity };
     static inline std::unique_ptr<SynchronizedSamplePool> sample_pool{ nullptr };
 

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

@@ -2,29 +2,28 @@
 
 #include "sample.hpp"
 
-extern "C"
-{
-#include "datadog/common.h"
-}
+#include "vendored/concurrentqueue.h"
+
 #include <memory>
 #include <optional>
 
 namespace Datadog {
 
-struct Deleter
-{
-    void operator()(ddog_ArrayQueue* object) { ddog_ArrayQueue_drop(object); }
-};
-
 class SynchronizedSamplePool
 {
   private:
-    std::unique_ptr<ddog_ArrayQueue, Deleter> pool;
+    moodycamel::ConcurrentQueue<std::unique_ptr<Sample>> pool;
+    std::atomic<size_t> capacity;
 
   public:
-    SynchronizedSamplePool(size_t capacity);
+    SynchronizedSamplePool(size_t _capacity)
+    {
+        capacity.store(_capacity);
+        pool = moodycamel::ConcurrentQueue<std::unique_ptr<Sample>>(_capacity);
+    }
 
     std::optional<Sample*> take_sample();
     std::optional<Sample*> return_sample(Sample* sample);
+    void postfork_child();
 };
 } // namespace Datadog

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

@@ -186,6 +186,9 @@ Datadog::Profile::collect(const ddog_prof_Sample& sample, int64_t endtime_ns)
 void
 Datadog::Profile::postfork_child()
 {
-    profile_mtx.unlock();
+    // DEV: Need to reset locks in child to avoid deadlock, and we recreate
+    // the data structures these lock protect.
+    profile_mtx.~mutex();
+    new (&profile_mtx) std::mutex();
     cycle_buffers();
 }

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

@@ -2,78 +2,35 @@
 
 #include "libdatadog_helpers.hpp"
 
-extern "C"
-{
-#include "datadog/common.h"
-}
+#include "vendored/concurrentqueue.h"
 
 namespace Datadog {
 
-void
-sample_delete_fn(void* sample)
-{
-    delete static_cast<Sample*>(sample); // NOLINT(cppcoreguidelines-owning-memory)
-}
-
-SynchronizedSamplePool::SynchronizedSamplePool(size_t capacity)
-{
-    ddog_ArrayQueue_NewResult array_queue_new_result = ddog_ArrayQueue_new(capacity, sample_delete_fn);
-    if (array_queue_new_result.tag == DDOG_ARRAY_QUEUE_NEW_RESULT_OK) {
-        pool = std::unique_ptr<ddog_ArrayQueue, Deleter>(array_queue_new_result.ok);
-    } else {
-        auto err = array_queue_new_result.err;
-        std::string errmsg = err_to_msg(&err, "Failed to create sample pool");
-        std::cerr << errmsg << std::endl;
-        ddog_Error_drop(&err);
-        pool = nullptr;
-    }
-}
-
 std::optional<Sample*>
 SynchronizedSamplePool::take_sample()
 {
-    std::optional<Sample*> result = std::nullopt;
+    std::unique_ptr<Sample> sample = nullptr;
 
-    // It's actually ok to call ddog_ArrayQueue_* methods with a nullptr,
-    // they will return an error result, but we already have printed out
-    // an error message in the constructor, so check for nullptr here to
-    // avoid spamming the error message.
-    if (pool != nullptr) {
-        ddog_ArrayQueue_PopResult pop_result = ddog_ArrayQueue_pop(pool.get());
+    pool.try_dequeue(sample);
 
-        if (pop_result.tag == DDOG_ARRAY_QUEUE_POP_RESULT_OK) {
-            result = static_cast<Sample*>(pop_result.ok);
-        } else if (pop_result.tag == DDOG_ARRAY_QUEUE_POP_RESULT_ERR) {
-            auto err = pop_result.err;
-            std::string errmsg = err_to_msg(&err, "Failed to get sample from pool");
-            std::cerr << errmsg << std::endl;
-            ddog_Error_drop(&err);
-        }
+    if (sample == nullptr) {
+        return std::nullopt;
     }
-
-    return result;
+    return sample.release();
 }
 
 std::optional<Sample*>
 SynchronizedSamplePool::return_sample(Sample* sample)
 {
-    std::optional<Sample*> result = std::nullopt;
-
-    if (pool != nullptr) {
-        ddog_ArrayQueue_PushResult push_result = ddog_ArrayQueue_push(pool.get(), sample);
-
-        if (push_result.tag == DDOG_ARRAY_QUEUE_PUSH_RESULT_OK) {
-            // The sample was successfully returned to the pool.
-        } else if (push_result.tag == DDOG_ARRAY_QUEUE_PUSH_RESULT_FULL) {
-            result = static_cast<Sample*>(push_result.full);
-        } else if (push_result.tag == DDOG_ARRAY_QUEUE_PUSH_RESULT_ERR) {
-            auto err = push_result.err;
-            std::string errmsg = err_to_msg(&err, "Failed to return sample to pool");
-            std::cerr << errmsg << std::endl;
-            ddog_Error_drop(&err);
-        }
+    // We don't want the pool to grow without a bound, so check the size and
+    // discard the sample if it's larger than capacity.
+    if (capacity.load() <= pool.size_approx()) {
+        return sample;
+    } else {
+        std::unique_ptr<Sample> ptr = nullptr;
+        ptr.reset(sample);
+        pool.enqueue(std::move(ptr));
+        return std::nullopt;
     }
-
-    return result;
 }
 } // namespace Datadog

releasenotes/notes/profiling-reset-lock-51a032479617f038.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes an issue where calling ``unlock()`` on unlocked mutex could result in
+    an undefined behavior.
+    Fixes an issue where the child process could deadlock after ``fork()`` as
+    it didn't reset the locks.
+

scripts/cformat.sh

@@ -27,12 +27,17 @@ then
     | grep -v '_taint_tracking/_vendor/' \
     | grep -v 'ddtrace/appsec/_iast/_taint_tracking/cmake-build-debug/' \
     | grep -v '^ddtrace/appsec/_iast/_taint_tracking/_vendor/' \
+    | grep -v '^ddtrace/internal/datadog/profiling/dd_wrapper/include/vendored/' \
     | while IFS= read -r file; do
   clang-format -i $file
   echo "Formatting $file"
 done
 else
-  git ls-files '*.c' '*.h' '*.cpp' '*.hpp' | grep -v '^ddtrace/vendor/' | grep -v '^ddtrace/appsec/_iast/_taint_tracking/_vendor/'  | while read filename
+  git ls-files '*.c' '*.h' '*.cpp' '*.hpp' \
+    | grep -v '^ddtrace/vendor/' \
+    | grep -v '^ddtrace/appsec/_iast/_taint_tracking/_vendor/' \
+    | grep -v '^ddtrace/internal/datadog/profiling/dd_wrapper/include/vendored/' \
+    | while read filename
   do
     CFORMAT_TMP=`mktemp`
     clang-format "$filename" > "$CFORMAT_TMP"