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`
This commit is contained in:
commit
e73ee1dde2
@ -95,11 +95,12 @@ mod tests;
|
|||||||
// a backtrace or actually symbolizing it.
|
// a backtrace or actually symbolizing it.
|
||||||
|
|
||||||
use crate::backtrace_rs::{self, BytesOrWideString};
|
use crate::backtrace_rs::{self, BytesOrWideString};
|
||||||
|
use crate::cell::UnsafeCell;
|
||||||
use crate::env;
|
use crate::env;
|
||||||
use crate::ffi::c_void;
|
use crate::ffi::c_void;
|
||||||
use crate::fmt;
|
use crate::fmt;
|
||||||
use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst};
|
use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst};
|
||||||
use crate::sync::Mutex;
|
use crate::sync::Once;
|
||||||
use crate::sys_common::backtrace::{lock, output_filename};
|
use crate::sys_common::backtrace::{lock, output_filename};
|
||||||
use crate::vec::Vec;
|
use crate::vec::Vec;
|
||||||
|
|
||||||
@ -132,7 +133,7 @@ pub enum BacktraceStatus {
|
|||||||
enum Inner {
|
enum Inner {
|
||||||
Unsupported,
|
Unsupported,
|
||||||
Disabled,
|
Disabled,
|
||||||
Captured(Mutex<Capture>),
|
Captured(LazilyResolvedCapture),
|
||||||
}
|
}
|
||||||
|
|
||||||
struct Capture {
|
struct Capture {
|
||||||
@ -171,12 +172,11 @@ enum BytesOrWide {
|
|||||||
|
|
||||||
impl fmt::Debug for Backtrace {
|
impl fmt::Debug for Backtrace {
|
||||||
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
|
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||||
let mut capture = match &self.inner {
|
let capture = match &self.inner {
|
||||||
Inner::Unsupported => return fmt.write_str("<unsupported>"),
|
Inner::Unsupported => return fmt.write_str("<unsupported>"),
|
||||||
Inner::Disabled => return fmt.write_str("<disabled>"),
|
Inner::Disabled => return fmt.write_str("<disabled>"),
|
||||||
Inner::Captured(c) => c.lock().unwrap(),
|
Inner::Captured(c) => c.force(),
|
||||||
};
|
};
|
||||||
capture.resolve();
|
|
||||||
|
|
||||||
let frames = &capture.frames[capture.actual_start..];
|
let frames = &capture.frames[capture.actual_start..];
|
||||||
|
|
||||||
@ -331,7 +331,7 @@ impl Backtrace {
|
|||||||
let inner = if frames.is_empty() {
|
let inner = if frames.is_empty() {
|
||||||
Inner::Unsupported
|
Inner::Unsupported
|
||||||
} else {
|
} else {
|
||||||
Inner::Captured(Mutex::new(Capture {
|
Inner::Captured(LazilyResolvedCapture::new(Capture {
|
||||||
actual_start: actual_start.unwrap_or(0),
|
actual_start: actual_start.unwrap_or(0),
|
||||||
frames,
|
frames,
|
||||||
resolved: false,
|
resolved: false,
|
||||||
@ -355,12 +355,11 @@ impl Backtrace {
|
|||||||
|
|
||||||
impl fmt::Display for Backtrace {
|
impl fmt::Display for Backtrace {
|
||||||
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
|
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||||
let mut capture = match &self.inner {
|
let capture = match &self.inner {
|
||||||
Inner::Unsupported => return fmt.write_str("unsupported backtrace"),
|
Inner::Unsupported => return fmt.write_str("unsupported backtrace"),
|
||||||
Inner::Disabled => return fmt.write_str("disabled backtrace"),
|
Inner::Disabled => return fmt.write_str("disabled backtrace"),
|
||||||
Inner::Captured(c) => c.lock().unwrap(),
|
Inner::Captured(c) => c.force(),
|
||||||
};
|
};
|
||||||
capture.resolve();
|
|
||||||
|
|
||||||
let full = fmt.alternate();
|
let full = fmt.alternate();
|
||||||
let (frames, style) = if full {
|
let (frames, style) = if full {
|
||||||
@ -404,6 +403,33 @@ impl fmt::Display for Backtrace {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct LazilyResolvedCapture {
|
||||||
|
sync: Once,
|
||||||
|
capture: UnsafeCell<Capture>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl LazilyResolvedCapture {
|
||||||
|
fn new(capture: Capture) -> Self {
|
||||||
|
LazilyResolvedCapture { sync: Once::new(), capture: UnsafeCell::new(capture) }
|
||||||
|
}
|
||||||
|
|
||||||
|
fn force(&self) -> &Capture {
|
||||||
|
self.sync.call_once(|| {
|
||||||
|
// SAFETY: This exclusive reference can't overlap with any others
|
||||||
|
// `Once` guarantees callers will block until this closure returns
|
||||||
|
// `Once` also guarantees only a single caller will enter this closure
|
||||||
|
unsafe { &mut *self.capture.get() }.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
// SAFETY: This shared reference can't overlap with the exclusive reference above
|
||||||
|
unsafe { &*self.capture.get() }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// SAFETY: Access to the inner value is synchronized using a thread-safe `Once`
|
||||||
|
// So long as `Capture` is `Sync`, `LazilyResolvedCapture` is too
|
||||||
|
unsafe impl Sync for LazilyResolvedCapture where Capture: Sync {}
|
||||||
|
|
||||||
impl Capture {
|
impl Capture {
|
||||||
fn resolve(&mut self) {
|
fn resolve(&mut self) {
|
||||||
// If we're already resolved, nothing to do!
|
// If we're already resolved, nothing to do!
|
||||||
|
@ -3,7 +3,7 @@ use super::*;
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_debug() {
|
fn test_debug() {
|
||||||
let backtrace = Backtrace {
|
let backtrace = Backtrace {
|
||||||
inner: Inner::Captured(Mutex::new(Capture {
|
inner: Inner::Captured(LazilyResolvedCapture::new(Capture {
|
||||||
actual_start: 1,
|
actual_start: 1,
|
||||||
resolved: true,
|
resolved: true,
|
||||||
frames: vec![
|
frames: vec![
|
||||||
@ -54,4 +54,7 @@ fn test_debug() {
|
|||||||
\n]";
|
\n]";
|
||||||
|
|
||||||
assert_eq!(format!("{:#?}", backtrace), expected);
|
assert_eq!(format!("{:#?}", backtrace), expected);
|
||||||
|
|
||||||
|
// Format the backtrace a second time, just to make sure lazily resolved state is stable
|
||||||
|
assert_eq!(format!("{:#?}", backtrace), expected);
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user