Skip to content

Commit 730d1ae

Browse files
committed
Try passing writer directly
1 parent 3ee9ed8 commit 730d1ae

File tree

2 files changed

+71
-77
lines changed

2 files changed

+71
-77
lines changed

datadog-crashtracker-ffi/src/runtime_callback.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub use datadog_crashtracker::RuntimeType as ddog_RuntimeType;
1717

1818
#[cfg(test)]
1919
use datadog_crashtracker::clear_runtime_callback;
20-
use std::ffi::c_char;
20+
use std::ffi::{c_char, c_void};
2121

2222
/// Result type for runtime callback registration
2323
#[repr(C)]
@@ -165,8 +165,9 @@ mod tests {
165165
static TEST_MUTEX: Mutex<()> = Mutex::new(());
166166

167167
unsafe extern "C" fn test_runtime_callback(
168-
emit_frame: unsafe extern "C" fn(*const RuntimeStackFrame),
169-
_emit_stacktrace_string: unsafe extern "C" fn(*const c_char),
168+
emit_frame: unsafe extern "C" fn(*mut c_void, *const RuntimeStackFrame),
169+
_emit_stacktrace_string: unsafe extern "C" fn(*mut c_void, *const c_char),
170+
writer_ctx: *mut c_void,
170171
) {
171172
let function_name = CString::new("test_function").unwrap();
172173
let file_name = CString::new("test.rb").unwrap();
@@ -183,7 +184,7 @@ mod tests {
183184
module_name: ptr::null(),
184185
};
185186

186-
emit_frame(&frame);
187+
emit_frame(writer_ctx, &frame);
187188
}
188189

189190
#[test]

datadog-crashtracker/src/runtime_callback.rs

Lines changed: 66 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,6 @@ impl std::str::FromStr for CallbackType {
114114
static RUNTIME_CALLBACK: AtomicPtr<(RuntimeStackCallback, RuntimeType, CallbackType)> =
115115
AtomicPtr::new(ptr::null_mut());
116116

117-
thread_local! {
118-
static FRAME_WRITER: std::cell::RefCell<Option<*mut dyn std::io::Write>> = const { std::cell::RefCell::new(None) };
119-
static FRAME_COUNT: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
120-
}
121-
122117
#[repr(C)]
123118
#[derive(Debug, Clone)]
124119
pub struct RuntimeStackFrame {
@@ -145,17 +140,20 @@ pub struct RuntimeStackFrame {
145140
/// - Only async-signal-safe functions
146141
///
147142
/// # Parameters
148-
/// - `emit_frame`: Function to call for each runtime frame (marked unsafe because it takes raw pointers)
149-
/// - `emit_stacktrace_string`: Function to call for complete stacktrace string (marked unsafe because it takes raw C string pointers)
143+
/// - `emit_frame`: Function to call for each runtime frame (takes writer context and frame pointer)
144+
/// - `emit_stacktrace_string`: Function to call for complete stacktrace string (takes writer context and C string)
145+
/// - `writer_ctx`: Opaque pointer to writer context that should be passed to emit functions
150146
///
151147
/// # Safety
152148
/// The callback function is marked unsafe because:
153149
/// - It receives function pointers that take raw pointers as parameters
154150
/// - The callback must ensure any pointers it passes to these functions are valid
155151
/// - All C strings passed must be null-terminated and remain valid for the call duration
152+
/// - The writer_ctx must be passed unchanged to the emit functions
156153
pub type RuntimeStackCallback = unsafe extern "C" fn(
157-
emit_frame: unsafe extern "C" fn(*const RuntimeStackFrame),
158-
emit_stacktrace_string: unsafe extern "C" fn(*const c_char),
154+
emit_frame: unsafe extern "C" fn(*mut std::ffi::c_void, *const RuntimeStackFrame),
155+
emit_stacktrace_string: unsafe extern "C" fn(*mut std::ffi::c_void, *const c_char),
156+
writer_ctx: *mut std::ffi::c_void,
159157
);
160158

161159
/// Runtime stack representation for JSON serialization
@@ -367,78 +365,69 @@ pub(crate) unsafe fn invoke_runtime_callback_with_writer<W: std::io::Write>(
367365
// to a properly aligned, initialized tuple.
368366
let (callback_fn, _, _) = &*callback_ptr;
369367

370-
// Reset frame counter
371-
FRAME_COUNT.with(|count| count.set(0));
372-
373-
// Define the emit_frame function that writes directly to the pipe
374-
// Safety: This function is called by the user's runtime callback. It's marked unsafe
375-
// because it dereferences raw pointers, but the safety is guaranteed by the contract
376-
// with the runtime callback system.
377-
unsafe extern "C" fn emit_frame_collector(frame: *const RuntimeStackFrame) {
378-
// We need access to the writer, so we'll use thread-local storage for it
379-
FRAME_WRITER.with(|writer_cell| {
380-
if let Some(writer_ptr) = *writer_cell.borrow() {
381-
// Safety: writer_ptr was stored in thread-local storage before
382-
// calling the user callback, so it's guaranteed to be valid and point
383-
// to the same writer object we passed in.
384-
let writer = &mut *writer_ptr;
385-
386-
FRAME_COUNT.with(|count| {
387-
let current_count = count.get();
388-
389-
// Add comma separator for frames after the first
390-
if current_count > 0 {
391-
let _ = write!(writer, ", ");
392-
}
393-
394-
// Write the frame as JSON
395-
// Safety: frame pointer is passed from the runtime callback, writer is valid
396-
let _ = emit_frame_as_json(writer, frame);
397-
let _ = writer.flush();
398-
399-
count.set(current_count + 1);
400-
});
401-
}
402-
});
368+
let mut frame_count = 0usize;
369+
370+
// Define the emit_frame function that writes directly to the writer
371+
// Safety: This function receives writer context and frame pointer from the runtime callback.
372+
// The writer context is guaranteed to be valid for the duration of the callback.
373+
unsafe extern "C" fn emit_frame_collector(
374+
writer_ctx: *mut std::ffi::c_void,
375+
frame: *const RuntimeStackFrame,
376+
) {
377+
if writer_ctx.is_null() || frame.is_null() {
378+
return;
379+
}
380+
381+
// Safety: writer_ctx was created from a valid writer reference and frame_count pointer
382+
let (writer, frame_count) =
383+
&mut *(writer_ctx as *mut (&mut dyn std::io::Write, &mut usize));
384+
385+
// Add comma separator for frames after the first
386+
if **frame_count > 0 {
387+
let _ = write!(writer, ", ");
388+
}
389+
390+
// Write the frame as JSON
391+
// Safety: frame pointer is passed from the runtime callback
392+
let _ = emit_frame_as_json(*writer, frame);
393+
let _ = writer.flush();
394+
395+
**frame_count += 1;
403396
}
404397

405-
// Safety: This function is called by the user's runtime callback with a C string pointer.
406-
// The safety requirements are documented in the RuntimeStackCallback type.
407-
unsafe extern "C" fn emit_stacktrace_string_collector(stacktrace_string: *const c_char) {
408-
if stacktrace_string.is_null() {
398+
// Safety: This function receives writer context and C string pointer from the runtime callback.
399+
unsafe extern "C" fn emit_stacktrace_string_collector(
400+
writer_ctx: *mut std::ffi::c_void,
401+
stacktrace_string: *const c_char,
402+
) {
403+
if writer_ctx.is_null() || stacktrace_string.is_null() {
409404
return;
410405
}
411406

407+
// Safety: writer_ctx was created from a valid writer reference
408+
let (writer, _) = &mut *(writer_ctx as *mut (&mut dyn std::io::Write, &mut usize));
409+
412410
// Safety: stacktrace_string is guaranteed by the runtime callback contract to be
413411
// a valid, null-terminated C string that remains valid for the duration of this call.
414412
let cstr = std::ffi::CStr::from_ptr(stacktrace_string);
415413
let bytes = cstr.to_bytes();
416414

417-
FRAME_WRITER.with(|writer_cell| {
418-
if let Some(writer_ptr) = *writer_cell.borrow() {
419-
// Safety: writer_ptr was stored in thread-local storage just before
420-
// calling the user callback, so it's guaranteed to be valid.
421-
let writer = &mut *writer_ptr;
422-
let _ = writer.write_all(bytes);
423-
let _ = writer.flush();
424-
}
425-
});
415+
let _ = writer.write_all(bytes);
416+
let _ = writer.flush();
426417
}
427418

428-
// Store the writer in thread-local storage so the emit functions can access it
429-
FRAME_WRITER.with(|writer_cell| {
430-
*writer_cell.borrow_mut() = Some(writer as *mut W as *mut dyn std::io::Write);
431-
});
419+
// Create writer context that bundles writer and frame counter
420+
let mut writer_context = (writer as &mut dyn std::io::Write, &mut frame_count);
421+
let writer_ctx = &mut writer_context as *mut _ as *mut std::ffi::c_void;
432422

433-
// Invoke the user callback
423+
// Invoke the user callback with the writer context
434424
// Safety: callback_fn was verified to be non-null during registration, and the
435-
// emit functions we're passing are compatible with the expected function signatures.
436-
callback_fn(emit_frame_collector, emit_stacktrace_string_collector);
437-
438-
// Clear the writer reference
439-
FRAME_WRITER.with(|writer_cell| {
440-
*writer_cell.borrow_mut() = None;
441-
});
425+
// emit functions and writer context are valid for the duration of this call.
426+
callback_fn(
427+
emit_frame_collector,
428+
emit_stacktrace_string_collector,
429+
writer_ctx,
430+
);
442431

443432
Ok(())
444433
}
@@ -555,8 +544,9 @@ mod tests {
555544
static TEST_MUTEX: Mutex<()> = Mutex::new(());
556545

557546
unsafe extern "C" fn test_emit_frame_callback(
558-
emit_frame: unsafe extern "C" fn(*const RuntimeStackFrame),
559-
_emit_stacktrace_string: unsafe extern "C" fn(*const c_char),
547+
emit_frame: unsafe extern "C" fn(*mut std::ffi::c_void, *const RuntimeStackFrame),
548+
_emit_stacktrace_string: unsafe extern "C" fn(*mut std::ffi::c_void, *const c_char),
549+
writer_ctx: *mut std::ffi::c_void,
560550
) {
561551
let function_name = CString::new("test_function").unwrap();
562552
let file_name = CString::new("test.rb").unwrap();
@@ -571,16 +561,19 @@ mod tests {
571561
module_name: ptr::null(),
572562
};
573563

574-
emit_frame(&frame);
564+
// Safety: frame is a valid RuntimeStackFrame with valid C string pointers
565+
emit_frame(writer_ctx, &frame);
575566
}
576567

577568
unsafe extern "C" fn test_emit_stacktrace_string_callback(
578-
_emit_frame: unsafe extern "C" fn(*const RuntimeStackFrame),
579-
emit_stacktrace_string: unsafe extern "C" fn(*const c_char),
569+
_emit_frame: unsafe extern "C" fn(*mut std::ffi::c_void, *const RuntimeStackFrame),
570+
emit_stacktrace_string: unsafe extern "C" fn(*mut std::ffi::c_void, *const c_char),
571+
writer_ctx: *mut std::ffi::c_void,
580572
) {
581573
let stacktrace_string = CString::new("test_stacktrace_string").unwrap();
582574

583-
emit_stacktrace_string(stacktrace_string.as_ptr());
575+
// Safety: stacktrace_string.as_ptr() returns a valid null-terminated C string
576+
emit_stacktrace_string(writer_ctx, stacktrace_string.as_ptr());
584577
}
585578

586579
fn ensure_callback_cleared() {

0 commit comments

Comments
 (0)