fix(profiling): fix data race when accessing span for thread

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

@@ -117,3 +117,9 @@ if(LIB_INSTALL_DIR)
         ARCHIVE DESTINATION ${LIB_INSTALL_DIR}
         RUNTIME DESTINATION ${LIB_INSTALL_DIR})
 endif()
+
+if(BUILD_TESTING)
+    enable_testing()
+    add_subdirectory(test)
+endif()
+

ddtrace/internal/datadog/profiling/stack_v2/include/thread_span_links.hpp

@@ -2,6 +2,7 @@
 
 #include <memory>
 #include <mutex>
+#include <optional>
 #include <stdint.h>
 #include <string>
 #include <unordered_map>
@@ -36,7 +37,7 @@ class ThreadSpanLinks
     ThreadSpanLinks& operator=(ThreadSpanLinks const&) = delete;
 
     void link_span(uint64_t thread_id, uint64_t span_id, uint64_t local_root_span_id, std::string span_type);
-    const Span* get_active_span_from_thread_id(uint64_t thread_id);
+    const std::optional<Span> get_active_span_from_thread_id(uint64_t thread_id);
     void reset();
 
     static void postfork_child();

ddtrace/internal/datadog/profiling/stack_v2/src/stack_renderer.cpp

@@ -55,8 +55,8 @@ StackRenderer::render_thread_begin(PyThreadState* tstate,
     ddup_push_threadinfo(sample, static_cast<int64_t>(thread_id), static_cast<int64_t>(native_id), name);
     ddup_push_walltime(sample, thread_state.wall_time_ns, 1);
 
-    const Span* active_span = ThreadSpanLinks::get_instance().get_active_span_from_thread_id(thread_id);
-    if (active_span != nullptr) {
+    const std::optional<Span> active_span = ThreadSpanLinks::get_instance().get_active_span_from_thread_id(thread_id);
+    if (active_span) {
         ddup_push_span_id(sample, active_span->span_id);
         ddup_push_local_root_span_id(sample, active_span->local_root_span_id);
         ddup_push_trace_type(sample, std::string_view(active_span->span_type));

ddtrace/internal/datadog/profiling/stack_v2/src/thread_span_links.cpp

@@ -2,6 +2,7 @@
 
 #include <iostream>
 #include <mutex>
+#include <optional>
 #include <stdint.h>
 #include <string>
 
@@ -19,15 +20,17 @@ ThreadSpanLinks::link_span(uint64_t thread_id, uint64_t span_id, uint64_t local_
     thread_id_to_span[thread_id]->span_type = span_type;
 }
 
-const Span*
+const std::optional<Span>
 ThreadSpanLinks::get_active_span_from_thread_id(uint64_t thread_id)
 {
     std::lock_guard<std::mutex> lock(mtx);
 
-    if (thread_id_to_span.find(thread_id) == thread_id_to_span.end()) {
-        return nullptr;
+    std::optional<Span> span;
+    auto it = thread_id_to_span.find(thread_id);
+    if (it != thread_id_to_span.end()) {
+        span = *(it->second);
     }
-    return thread_id_to_span[thread_id].get();
+    return span;
 }
 
 void

ddtrace/internal/datadog/profiling/stack_v2/test/CMakeLists.txt

@@ -0,0 +1,25 @@
+FetchContent_Declare(
+    googletest
+    GIT_REPOSITORY https://github.com/google/googletest.git
+    GIT_TAG release-1.11.0)
+set(gtest_force_shared_crt
+    ON
+    CACHE BOOL "" FORCE)
+set(INSTALL_GTEST
+    OFF
+    CACHE BOOL "" FORCE)
+FetchContent_MakeAvailable(googletest)
+include(GoogleTest)
+include(AnalysisFunc)
+
+function(dd_wrapper_add_test name)
+    add_executable(${name} ${ARGN})
+    target_include_directories(${name} PRIVATE ../include)
+    target_link_libraries(${name} PRIVATE gmock gtest_main _stack_v2)
+    add_ddup_config(${name})
+
+    gtest_discover_tests(${name})
+endfunction()
+
+# Add the tests
+dd_wrapper_add_test(thread_span_links thread_span_links.cpp)

ddtrace/internal/datadog/profiling/stack_v2/test/thread_span_links.cpp

@@ -0,0 +1,48 @@
+#include <gtest/gtest.h>
+
+#include <string>
+#include <thread>
+
+#include "thread_span_links.hpp"
+
+static void
+get()
+{
+    for (int i = 0; i < 100; i++) {
+        std::string span_type;
+        for (int j = 0; j < i; j++) {
+            span_type.append("a");
+        }
+        Datadog::ThreadSpanLinks::get_instance().link_span(42, 1, 2, span_type);
+    }
+}
+
+static std::string
+set()
+{
+    std::string s;
+    for (int i = 0; i < 100; i++) {
+        auto thing = Datadog::ThreadSpanLinks::get_instance().get_active_span_from_thread_id(42);
+        if (!thing) {
+            continue;
+        }
+        s = thing->span_type;
+    }
+    return s;
+}
+
+TEST(ThreadSpanLinksConcurrency, GetSetRace)
+{
+    std::thread t1(get);
+    std::thread t2(set);
+    t1.join();
+    t2.join();
+}
+
+int
+main(int argc, char** argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    (void)(::testing::GTEST_FLAG(death_test_style) = "threadsafe");
+    return RUN_ALL_TESTS();
+}

releasenotes/notes/profiling-fix-data-race-segfault-b14f59c06c6bf9ed.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    profiling: fix a data race where span information associated with a thread
+    was read and updated concurrently, leading to segfaults