Skip to content

Commit

Permalink
Merge branch '2.13' into backport-10962-to-2.13
Browse files Browse the repository at this point in the history
  • Loading branch information
taegyunkim authored Oct 30, 2024
2 parents 7ef54e3 + 968d4dd commit 82cc668
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 8 deletions.
6 changes: 6 additions & 0 deletions ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,9 @@ if (LIB_INSTALL_DIR)
RUNTIME DESTINATION ${LIB_INSTALL_DIR}
)
endif()

if(BUILD_TESTING)
enable_testing()
add_subdirectory(test)
endif()

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <memory>
#include <mutex>
#include <optional>
#include <stdint.h>
#include <string>
#include <unordered_map>
Expand Down Expand Up @@ -36,8 +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);

static void postfork_child();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <iostream>
#include <mutex>
#include <optional>
#include <stdint.h>
#include <string>

Expand All @@ -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
Expand Down
25 changes: 25 additions & 0 deletions ddtrace/internal/datadog/profiling/stack_v2/test/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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();
}
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 82cc668

Please sign in to comment.