diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp index 62ee6aad853..7077096c744 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp @@ -43,6 +43,8 @@ class UploaderBuilder static void set_output_filename(std::string_view _output_filename); static std::variant build(); + + static void postfork_child(); }; } // namespace Datadog diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp index 0a4a49a4ce5..f3147cd2034 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp @@ -14,9 +14,8 @@ namespace Datadog { void Datadog::CodeProvenance::postfork_child() { - get_instance().mtx.~mutex(); // Destroy the mutex + // NB placement-new to re-init and leak the mutex because doing anything else is UB new (&get_instance().mtx) std::mutex(); // Recreate the mutex - get_instance().reset(); // Reset the state } void diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp index baee51a7eda..e34bb7e8695 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp @@ -24,6 +24,7 @@ ddup_postfork_child() Datadog::Uploader::postfork_child(); Datadog::SampleManager::postfork_child(); Datadog::CodeProvenance::postfork_child(); + Datadog::UploaderBuilder::postfork_child(); } void diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp index 6f986f4896b..083c383fec0 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp @@ -201,6 +201,6 @@ Datadog::Profile::collect(const ddog_prof_Sample& sample, int64_t endtime_ns) void Datadog::Profile::postfork_child() { - profile_mtx.unlock(); + new (&profile_mtx) std::mutex(); cycle_buffers(); } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp index 3e63b97eb07..e85322ed34c 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp @@ -176,5 +176,6 @@ Datadog::Uploader::postfork_parent() void Datadog::Uploader::postfork_child() { - unlock(); + // NB placement-new to re-init and leak the mutex because doing anything else is UB + new (&upload_lock) std::mutex(); } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp index 0661b7f217f..8ff5d45e7c2 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp @@ -186,3 +186,10 @@ Datadog::UploaderBuilder::build() return Datadog::Uploader{ output_filename, ddog_exporter }; } + +void +Datadog::UploaderBuilder::postfork_child() +{ + // NB placement-new to re-init and leak the mutex because doing anything else is UB + new (&tag_mutex) std::mutex(); +} diff --git a/ddtrace/internal/datadog/profiling/stack_v2/src/sampler.cpp b/ddtrace/internal/datadog/profiling/stack_v2/src/sampler.cpp index c05ae45477e..7ad9ad692b2 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/src/sampler.cpp +++ b/ddtrace/internal/datadog/profiling/stack_v2/src/sampler.cpp @@ -67,6 +67,11 @@ _stack_v2_atfork_child() // so we don't even reveal this function to the user _set_pid(getpid()); ThreadSpanLinks::postfork_child(); + + // `thread_info_map_lock` and `task_link_map_lock` are global locks held in echion + // NB placement-new to re-init and leak the mutex because doing anything else is UB + new (&thread_info_map_lock) std::mutex; + new (&task_link_map_lock) std::mutex; } __attribute__((constructor)) void diff --git a/ddtrace/internal/datadog/profiling/stack_v2/src/thread_span_links.cpp b/ddtrace/internal/datadog/profiling/stack_v2/src/thread_span_links.cpp index c777ff8a510..6be43a04a42 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/src/thread_span_links.cpp +++ b/ddtrace/internal/datadog/profiling/stack_v2/src/thread_span_links.cpp @@ -53,10 +53,8 @@ ThreadSpanLinks::reset() void ThreadSpanLinks::postfork_child() { - // Explicitly destroy and reconstruct the mutex to avoid undefined behavior - get_instance().mtx.~mutex(); + // NB placement-new to re-init and leak the mutex because doing anything else is UB new (&get_instance().mtx) std::mutex(); - get_instance().reset(); } diff --git a/releasenotes/notes/fix-profiling-native-mutices-62440b5a3d9d6c4b.yaml b/releasenotes/notes/fix-profiling-native-mutices-62440b5a3d9d6c4b.yaml new file mode 100644 index 00000000000..40167a974c3 --- /dev/null +++ b/releasenotes/notes/fix-profiling-native-mutices-62440b5a3d9d6c4b.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + profiling: Fixes a bug where profiling mutexes were not cleared on fork in the child process. This could + cause deadlocks in certain configurations.