From db4585aa3b1ee56e4722710d7665ee011fc11145 Mon Sep 17 00:00:00 2001
From: Ashley Mannix <kodraus@hey.com>
Date: Wed, 6 Jan 2021 10:04:05 +1000
Subject: [PATCH] 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.
---
 library/std/src/backtrace.rs       | 44 ++++++++++++++++++++++++------
 library/std/src/backtrace/tests.rs |  5 +++-
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/library/std/src/backtrace.rs b/library/std/src/backtrace.rs
index 7e8e0a621e31c..95e18ef2a6543 100644
--- a/library/std/src/backtrace.rs
+++ b/library/std/src/backtrace.rs
@@ -95,11 +95,12 @@ 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::sync::atomic::{AtomicUsize, Ordering::SeqCst};
-use crate::sync::Mutex;
+use crate::sync::Once;
 use crate::sys_common::backtrace::{lock, output_filename};
 use crate::vec::Vec;
 
@@ -132,7 +133,7 @@ pub enum BacktraceStatus {
 enum Inner {
     Unsupported,
     Disabled,
-    Captured(Mutex<Capture>),
+    Captured(LazilyResolvedCapture),
 }
 
 struct Capture {
@@ -171,12 +172,11 @@ enum BytesOrWide {
 
 impl fmt::Debug for Backtrace {
     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::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..];
 
@@ -331,7 +331,7 @@ impl Backtrace {
         let inner = if frames.is_empty() {
             Inner::Unsupported
         } else {
-            Inner::Captured(Mutex::new(Capture {
+            Inner::Captured(LazilyResolvedCapture::new(Capture {
                 actual_start: actual_start.unwrap_or(0),
                 frames,
                 resolved: false,
@@ -355,12 +355,11 @@ impl Backtrace {
 
 impl fmt::Display for Backtrace {
     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::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 (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 {
     fn resolve(&mut self) {
         // If we're already resolved, nothing to do!
diff --git a/library/std/src/backtrace/tests.rs b/library/std/src/backtrace/tests.rs
index f5f74d1eb9ae6..31cf0f702185c 100644
--- a/library/std/src/backtrace/tests.rs
+++ b/library/std/src/backtrace/tests.rs
@@ -3,7 +3,7 @@ use super::*;
 #[test]
 fn test_debug() {
     let backtrace = Backtrace {
-        inner: Inner::Captured(Mutex::new(Capture {
+        inner: Inner::Captured(LazilyResolvedCapture::new(Capture {
             actual_start: 1,
             resolved: true,
             frames: vec![
@@ -54,4 +54,7 @@ fn test_debug() {
     \n]";
 
     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);
 }