Skip to content

Commit a690e38

Browse files
NHDalyvtjnashvilterp
committed
Fix thread-safety violation in Allocations Profiler: Create a new per-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>
1 parent b5331da commit a690e38

File tree

2 files changed

+16
-5
lines changed

2 files changed

+16
-5
lines changed

src/gc-alloc-profiler.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,24 @@ jl_combined_results g_combined_results; // Will live forever.
4747
// === stack stuff ===
4848

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

53-
size_t bt_size = rec_backtrace(static_bt_data, JL_MAX_BT_SIZE, 2);
61+
size_t bt_size = rec_backtrace(shared_bt_data_buffer, JL_MAX_BT_SIZE, 2);
5462

5563
// Then we copy only the needed bytes out of the buffer into our profile.
5664
size_t bt_bytes = bt_size * sizeof(jl_bt_element_t);
57-
jl_bt_element_t *bt_data = (jl_bt_element_t*) malloc(bt_bytes);
58-
memcpy(bt_data, static_bt_data, bt_bytes);
65+
jl_bt_element_t *bt_data = (jl_bt_element_t*) malloc_s(bt_bytes);
66+
memcpy(bt_data, shared_bt_data_buffer, bt_bytes);
67+
5968

6069
return jl_raw_backtrace_t{
6170
bt_data,

src/julia_threads.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,8 @@ typedef struct _jl_tls_states_t {
238238
// Temporary backtrace buffer. Scanned for gc roots when bt_size > 0.
239239
struct _jl_bt_element_t *bt_data; // JL_MAX_BT_SIZE + 1 elements long
240240
size_t bt_size; // Size for backtrace in transit in bt_data
241+
// Temporary backtrace buffer used only for allocations profiler.
242+
struct _jl_bt_element_t *profiling_bt_buffer;
241243
// Atomically set by the sender, reset by the handler.
242244
volatile _Atomic(sig_atomic_t) signal_request; // TODO: no actual reason for this to be _Atomic
243245
// Allow the sigint to be raised asynchronously

0 commit comments

Comments
 (0)