Skip to content

Commit

Permalink
Fix thread-safety violation in Allocations Profiler: Create a new per…
Browse files Browse the repository at this point in the history
…-thread backtrace buffer in ptls (#44116)

* Fix thread-safety violation in Allocations Profiler:

Re-use the shared `ptls->bt_data` buffer from the thread-local storage
for the buffer, to ensure that each thread has a separate buffer.

This buffer is shared with the exception throwing mechanism, but is safe
to share since julia exception throwing never interleaves with
allocations profiling.

* Approach two: Create a separate per-thread allocations backtrace buffer.

* Update src/gc-alloc-profiler.cpp

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Update src/gc-alloc-profiler.cpp

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* fix type error (#44235)

* Update src/gc-alloc-profiler.cpp

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Pete Vilter <7341+vilterp@users.noreply.github.com>
  • Loading branch information
3 people authored Feb 22, 2022
1 parent e492025 commit 4e57966
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
19 changes: 14 additions & 5 deletions src/gc-alloc-profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,24 @@ jl_combined_results g_combined_results; // Will live forever.
// === stack stuff ===

jl_raw_backtrace_t get_raw_backtrace() JL_NOTSAFEPOINT {
// A single large buffer to record backtraces onto
static jl_bt_element_t static_bt_data[JL_MAX_BT_SIZE];
// We first record the backtrace onto a MAX-sized buffer, so that we don't have to
// allocate the buffer until we know the size. To ensure thread-safety, we use a
// per-thread backtrace buffer.
jl_ptls_t ptls = jl_current_task->ptls;
jl_bt_element_t *shared_bt_data_buffer = ptls->profiling_bt_buffer;
if (shared_bt_data_buffer == NULL) {
size_t size = sizeof(jl_bt_element_t) * (JL_MAX_BT_SIZE + 1);
shared_bt_data_buffer = (jl_bt_element_t*) malloc_s(size);
ptls->profiling_bt_buffer = shared_bt_data_buffer;
}

size_t bt_size = rec_backtrace(static_bt_data, JL_MAX_BT_SIZE, 2);
size_t bt_size = rec_backtrace(shared_bt_data_buffer, JL_MAX_BT_SIZE, 2);

// Then we copy only the needed bytes out of the buffer into our profile.
size_t bt_bytes = bt_size * sizeof(jl_bt_element_t);
jl_bt_element_t *bt_data = (jl_bt_element_t*) malloc(bt_bytes);
memcpy(bt_data, static_bt_data, bt_bytes);
jl_bt_element_t *bt_data = (jl_bt_element_t*) malloc_s(bt_bytes);
memcpy(bt_data, shared_bt_data_buffer, bt_bytes);


return jl_raw_backtrace_t{
bt_data,
Expand Down
2 changes: 2 additions & 0 deletions src/julia_threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ typedef struct _jl_tls_states_t {
// Temporary backtrace buffer. Scanned for gc roots when bt_size > 0.
struct _jl_bt_element_t *bt_data; // JL_MAX_BT_SIZE + 1 elements long
size_t bt_size; // Size for backtrace in transit in bt_data
// Temporary backtrace buffer used only for allocations profiler.
struct _jl_bt_element_t *profiling_bt_buffer;
// Atomically set by the sender, reset by the handler.
volatile _Atomic(sig_atomic_t) signal_request; // TODO: no actual reason for this to be _Atomic
// Allow the sigint to be raised asynchronously
Expand Down

0 comments on commit 4e57966

Please sign in to comment.