Skip to content

Commit db4585a

Browse files
committed
use Once instead of Mutex to manage capture resolution
This allows us to return borrows of the captured backtrace frames that are tied to a borrow of the Backtrace itself, instead of to some short-lived Mutex guard. It also makes it semantically clearer what synchronization is needed on the capture.
1 parent da305a2 commit db4585a

File tree

2 files changed

+39
-10
lines changed

2 files changed

+39
-10
lines changed

library/std/src/backtrace.rs

+35-9
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,12 @@ mod tests;
9595
// a backtrace or actually symbolizing it.
9696

9797
use crate::backtrace_rs::{self, BytesOrWideString};
98+
use crate::cell::UnsafeCell;
9899
use crate::env;
99100
use crate::ffi::c_void;
100101
use crate::fmt;
101102
use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst};
102-
use crate::sync::Mutex;
103+
use crate::sync::Once;
103104
use crate::sys_common::backtrace::{lock, output_filename};
104105
use crate::vec::Vec;
105106

@@ -132,7 +133,7 @@ pub enum BacktraceStatus {
132133
enum Inner {
133134
Unsupported,
134135
Disabled,
135-
Captured(Mutex<Capture>),
136+
Captured(LazilyResolvedCapture),
136137
}
137138

138139
struct Capture {
@@ -171,12 +172,11 @@ enum BytesOrWide {
171172

172173
impl fmt::Debug for Backtrace {
173174
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
174-
let mut capture = match &self.inner {
175+
let capture = match &self.inner {
175176
Inner::Unsupported => return fmt.write_str("<unsupported>"),
176177
Inner::Disabled => return fmt.write_str("<disabled>"),
177-
Inner::Captured(c) => c.lock().unwrap(),
178+
Inner::Captured(c) => c.force(),
178179
};
179-
capture.resolve();
180180

181181
let frames = &capture.frames[capture.actual_start..];
182182

@@ -331,7 +331,7 @@ impl Backtrace {
331331
let inner = if frames.is_empty() {
332332
Inner::Unsupported
333333
} else {
334-
Inner::Captured(Mutex::new(Capture {
334+
Inner::Captured(LazilyResolvedCapture::new(Capture {
335335
actual_start: actual_start.unwrap_or(0),
336336
frames,
337337
resolved: false,
@@ -355,12 +355,11 @@ impl Backtrace {
355355

356356
impl fmt::Display for Backtrace {
357357
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
358-
let mut capture = match &self.inner {
358+
let capture = match &self.inner {
359359
Inner::Unsupported => return fmt.write_str("unsupported backtrace"),
360360
Inner::Disabled => return fmt.write_str("disabled backtrace"),
361-
Inner::Captured(c) => c.lock().unwrap(),
361+
Inner::Captured(c) => c.force(),
362362
};
363-
capture.resolve();
364363

365364
let full = fmt.alternate();
366365
let (frames, style) = if full {
@@ -404,6 +403,33 @@ impl fmt::Display for Backtrace {
404403
}
405404
}
406405

406+
struct LazilyResolvedCapture {
407+
sync: Once,
408+
capture: UnsafeCell<Capture>,
409+
}
410+
411+
impl LazilyResolvedCapture {
412+
fn new(capture: Capture) -> Self {
413+
LazilyResolvedCapture { sync: Once::new(), capture: UnsafeCell::new(capture) }
414+
}
415+
416+
fn force(&self) -> &Capture {
417+
self.sync.call_once(|| {
418+
// SAFETY: This exclusive reference can't overlap with any others
419+
// `Once` guarantees callers will block until this closure returns
420+
// `Once` also guarantees only a single caller will enter this closure
421+
unsafe { &mut *self.capture.get() }.resolve();
422+
});
423+
424+
// SAFETY: This shared reference can't overlap with the exclusive reference above
425+
unsafe { &*self.capture.get() }
426+
}
427+
}
428+
429+
// SAFETY: Access to the inner value is synchronized using a thread-safe `Once`
430+
// So long as `Capture` is `Sync`, `LazilyResolvedCapture` is too
431+
unsafe impl Sync for LazilyResolvedCapture where Capture: Sync {}
432+
407433
impl Capture {
408434
fn resolve(&mut self) {
409435
// If we're already resolved, nothing to do!

library/std/src/backtrace/tests.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use super::*;
33
#[test]
44
fn test_debug() {
55
let backtrace = Backtrace {
6-
inner: Inner::Captured(Mutex::new(Capture {
6+
inner: Inner::Captured(LazilyResolvedCapture::new(Capture {
77
actual_start: 1,
88
resolved: true,
99
frames: vec![
@@ -54,4 +54,7 @@ fn test_debug() {
5454
\n]";
5555

5656
assert_eq!(format!("{:#?}", backtrace), expected);
57+
58+
// Format the backtrace a second time, just to make sure lazily resolved state is stable
59+
assert_eq!(format!("{:#?}", backtrace), expected);
5760
}

0 commit comments

Comments
 (0)