From 8df48d6a694ca7f94413ca3e7ae947d4ea227f78 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 19 Aug 2023 12:45:56 +0200 Subject: [PATCH 1/3] Revert "std: add auto traits to TAIT bound" This reverts commit 29e3f8b793ec3b76fdba8075adfada21ce2a220a. --- library/std/src/backtrace.rs | 7 +------ library/std/tests/backtrace.rs | 10 ++++++++++ 2 files changed, 11 insertions(+), 6 deletions(-) create mode 100644 library/std/tests/backtrace.rs diff --git a/library/std/src/backtrace.rs b/library/std/src/backtrace.rs index e7110aebdea16..2600e88266a30 100644 --- a/library/std/src/backtrace.rs +++ b/library/std/src/backtrace.rs @@ -141,11 +141,6 @@ struct Capture { frames: Vec, } -fn _assert_send_sync() { - fn _assert() {} - _assert::(); -} - /// A single frame of a backtrace. #[unstable(feature = "backtrace_frames", issue = "79676")] pub struct BacktraceFrame { @@ -428,7 +423,7 @@ impl fmt::Display for Backtrace { } } -type LazyResolve = impl (FnOnce() -> Capture) + Send + Sync + UnwindSafe; +type LazyResolve = impl FnOnce() -> Capture + UnwindSafe; fn lazy_resolve(mut capture: Capture) -> LazyResolve { move || { diff --git a/library/std/tests/backtrace.rs b/library/std/tests/backtrace.rs new file mode 100644 index 0000000000000..531c5f89cf67f --- /dev/null +++ b/library/std/tests/backtrace.rs @@ -0,0 +1,10 @@ +use std::backtrace::Backtrace; + +// Unfortunately, this cannot be a unit test because that causes problems +// with type-alias-impl-trait (the assert counts as a defining use). +#[test] +fn assert_send_sync() { + fn assert() {} + + assert::(); +} From 948bef6775ad893ffa473c83d9a13ef66a3e9e09 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 19 Aug 2023 12:48:51 +0200 Subject: [PATCH 2/3] Revert "std: use `LazyLock` to lazily resolve backtraces" This reverts commit 6776af529a163d5830fea577cf00619940b27908. --- library/std/src/backtrace.rs | 63 ++++++++++++++++++++++++------ library/std/src/backtrace/tests.rs | 6 ++- library/std/src/lib.rs | 1 - library/std/src/sync/lazy_lock.rs | 9 ----- library/std/tests/backtrace.rs | 10 ----- 5 files changed, 54 insertions(+), 35 deletions(-) delete mode 100644 library/std/tests/backtrace.rs diff --git a/library/std/src/backtrace.rs b/library/std/src/backtrace.rs index 2600e88266a30..7223142ff7f73 100644 --- a/library/std/src/backtrace.rs +++ b/library/std/src/backtrace.rs @@ -89,12 +89,13 @@ mod tests; // a backtrace or actually symbolizing it. use crate::backtrace_rs::{self, BytesOrWideString}; +use crate::cell::UnsafeCell; use crate::env; use crate::ffi::c_void; use crate::fmt; use crate::panic::UnwindSafe; use crate::sync::atomic::{AtomicUsize, Ordering::Relaxed}; -use crate::sync::LazyLock; +use crate::sync::Once; use crate::sys_common::backtrace::{lock, output_filename}; use crate::vec::Vec; @@ -133,14 +134,20 @@ pub enum BacktraceStatus { enum Inner { Unsupported, Disabled, - Captured(LazyLock), + Captured(LazilyResolvedCapture), } struct Capture { actual_start: usize, + resolved: bool, frames: Vec, } +fn _assert_send_sync() { + fn _assert() {} + _assert::(); +} + /// A single frame of a backtrace. #[unstable(feature = "backtrace_frames", issue = "79676")] pub struct BacktraceFrame { @@ -173,7 +180,7 @@ impl fmt::Debug for Backtrace { let capture = match &self.inner { Inner::Unsupported => return fmt.write_str(""), Inner::Disabled => return fmt.write_str(""), - Inner::Captured(c) => &**c, + Inner::Captured(c) => c.force(), }; let frames = &capture.frames[capture.actual_start..]; @@ -341,10 +348,11 @@ impl Backtrace { let inner = if frames.is_empty() { Inner::Unsupported } else { - Inner::Captured(LazyLock::new(lazy_resolve(Capture { + Inner::Captured(LazilyResolvedCapture::new(Capture { actual_start: actual_start.unwrap_or(0), frames, - }))) + resolved: false, + })) }; Backtrace { inner } @@ -369,7 +377,7 @@ impl<'a> Backtrace { #[must_use] #[unstable(feature = "backtrace_frames", issue = "79676")] pub fn frames(&'a self) -> &'a [BacktraceFrame] { - if let Inner::Captured(c) = &self.inner { &c.frames } else { &[] } + if let Inner::Captured(c) = &self.inner { &c.force().frames } else { &[] } } } @@ -379,7 +387,7 @@ impl fmt::Display for Backtrace { let capture = match &self.inner { Inner::Unsupported => return fmt.write_str("unsupported backtrace"), Inner::Disabled => return fmt.write_str("disabled backtrace"), - Inner::Captured(c) => &**c, + Inner::Captured(c) => c.force(), }; let full = fmt.alternate(); @@ -423,15 +431,46 @@ impl fmt::Display for Backtrace { } } -type LazyResolve = impl FnOnce() -> Capture + UnwindSafe; +struct LazilyResolvedCapture { + sync: Once, + capture: UnsafeCell, +} + +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 { + fn resolve(&mut self) { + // If we're already resolved, nothing to do! + if self.resolved { + return; + } + self.resolved = true; -fn lazy_resolve(mut capture: Capture) -> LazyResolve { - move || { // Use the global backtrace lock to synchronize this as it's a // requirement of the `backtrace` crate, and then actually resolve // everything. let _lock = lock(); - for frame in capture.frames.iter_mut() { + for frame in self.frames.iter_mut() { let symbols = &mut frame.symbols; let frame = match &frame.frame { RawFrame::Actual(frame) => frame, @@ -452,8 +491,6 @@ fn lazy_resolve(mut capture: Capture) -> LazyResolve { }); } } - - capture } } diff --git a/library/std/src/backtrace/tests.rs b/library/std/src/backtrace/tests.rs index 73543a3af548f..179a51e09a08d 100644 --- a/library/std/src/backtrace/tests.rs +++ b/library/std/src/backtrace/tests.rs @@ -44,8 +44,9 @@ fn generate_fake_frames() -> Vec { #[test] fn test_debug() { let backtrace = Backtrace { - inner: Inner::Captured(LazyLock::preinit(Capture { + inner: Inner::Captured(LazilyResolvedCapture::new(Capture { actual_start: 1, + resolved: true, frames: generate_fake_frames(), })), }; @@ -66,8 +67,9 @@ fn test_debug() { #[test] fn test_frames() { let backtrace = Backtrace { - inner: Inner::Captured(LazyLock::preinit(Capture { + inner: Inner::Captured(LazilyResolvedCapture::new(Capture { actual_start: 1, + resolved: true, frames: generate_fake_frames(), })), }; diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ac4ce222fbaac..7a85817ddb642 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -273,7 +273,6 @@ #![feature(staged_api)] #![feature(thread_local)] #![feature(try_blocks)] -#![feature(type_alias_impl_trait)] #![feature(utf8_chunks)] // tidy-alphabetical-end // diff --git a/library/std/src/sync/lazy_lock.rs b/library/std/src/sync/lazy_lock.rs index 3598598cfa02a..f21696c407475 100644 --- a/library/std/src/sync/lazy_lock.rs +++ b/library/std/src/sync/lazy_lock.rs @@ -89,15 +89,6 @@ impl T> LazyLock { LazyLock { once: Once::new(), data: UnsafeCell::new(Data { f: ManuallyDrop::new(f) }) } } - /// Creates a new lazy value that is already initialized. - #[inline] - #[cfg(test)] - pub(crate) fn preinit(value: T) -> LazyLock { - let once = Once::new(); - once.call_once(|| {}); - LazyLock { once, data: UnsafeCell::new(Data { value: ManuallyDrop::new(value) }) } - } - /// Consumes this `LazyLock` returning the stored value. /// /// Returns `Ok(value)` if `Lazy` is initialized and `Err(f)` otherwise. diff --git a/library/std/tests/backtrace.rs b/library/std/tests/backtrace.rs deleted file mode 100644 index 531c5f89cf67f..0000000000000 --- a/library/std/tests/backtrace.rs +++ /dev/null @@ -1,10 +0,0 @@ -use std::backtrace::Backtrace; - -// Unfortunately, this cannot be a unit test because that causes problems -// with type-alias-impl-trait (the assert counts as a defining use). -#[test] -fn assert_send_sync() { - fn assert() {} - - assert::(); -} From 7e1d6e6d0f1952526af385a59e7714389f59da6e Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 19 Aug 2023 12:53:19 +0200 Subject: [PATCH 3/3] std: make `LazilyResolvedCapture` unwind-safe to preserve #114331 after revert of #109075 --- library/std/src/backtrace.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/std/src/backtrace.rs b/library/std/src/backtrace.rs index 7223142ff7f73..65c4539299db6 100644 --- a/library/std/src/backtrace.rs +++ b/library/std/src/backtrace.rs @@ -93,7 +93,7 @@ use crate::cell::UnsafeCell; use crate::env; use crate::ffi::c_void; use crate::fmt; -use crate::panic::UnwindSafe; +use crate::panic::{RefUnwindSafe, UnwindSafe}; use crate::sync::atomic::{AtomicUsize, Ordering::Relaxed}; use crate::sync::Once; use crate::sys_common::backtrace::{lock, output_filename}; @@ -458,6 +458,9 @@ impl LazilyResolvedCapture { // So long as `Capture` is `Sync`, `LazilyResolvedCapture` is too unsafe impl Sync for LazilyResolvedCapture where Capture: Sync {} +impl UnwindSafe for LazilyResolvedCapture {} +impl RefUnwindSafe for LazilyResolvedCapture {} + impl Capture { fn resolve(&mut self) { // If we're already resolved, nothing to do!