Skip to content

Commit e73ee1d

Browse files
authored
Rollup merge of #80736 - KodrAus:feat/lazy-resolve, r=dtolnay
use Once instead of Mutex to manage capture resolution For #78299 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. We could alternatively share `&Mutex<Capture>`s and lock on-demand, but then we could potentially forget to call `resolve()` before working with the capture. It also makes it semantically clearer what synchronization is needed on the capture. cc `@seanchen1991` `@rust-lang/project-error-handling`
2 parents 492cb39 + db4585a commit e73ee1d

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)