Skip to content

Commit

Permalink
fix(profiling): reset all profiling c++ mutexes on fork
Browse files Browse the repository at this point in the history
Manual backport of #11768 to 2.17

---------

Co-authored-by: Taegyun Kim <[email protected]>
(cherry picked from commit d855c4a)
  • Loading branch information
sanchda authored and taegyunkim committed Jan 16, 2025
1 parent a20b4c2 commit 52a4383
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ class UploaderBuilder
static void set_output_filename(std::string_view _output_filename);

static std::variant<Uploader, std::string> build();

static void postfork_child();
};

} // namespace Datadog
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ ddup_postfork_child()
Datadog::Uploader::postfork_child();
Datadog::SampleManager::postfork_child();
Datadog::CodeProvenance::postfork_child();
Datadog::UploaderBuilder::postfork_child();
}

void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
5 changes: 5 additions & 0 deletions ddtrace/internal/datadog/profiling/stack_v2/src/sampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 52a4383

Please sign in to comment.