mirror of
https://github.com/zed-industries/zed.git
synced 2026-10-21 05:31:11 +00:00
gpui: Report the origin of conflicting app borrows (#65863)
Record where each app borrow group begins and include that location when a conflicting borrow panics, helping investigate [DELTA-SITE-6J](https://zed-dev.sentry.io/issues/7801086851/) without capturing stacks or adding per-borrow logging. Includes coverage for overlapping shared borrows and recovery after unwinding; the focused test and Wasm compile check pass. Runtime overhead has not been benchmarked. Release Notes: - [GPUI] Improved app borrow panic diagnostics to include the conflicting borrow origins.
This commit is contained in:
parent
92e8e2b8a0
commit
d94887e08e
1 changed files with 85 additions and 6 deletions
|
|
@ -83,9 +83,25 @@ pub const SHUTDOWN_TIMEOUT: Duration = Duration::from_millis(200);
|
|||
#[doc(hidden)]
|
||||
pub struct AppCell {
|
||||
app: RefCell<App>,
|
||||
borrow_origin: Cell<Option<(&'static std::panic::Location<'static>, &'static str)>>,
|
||||
}
|
||||
|
||||
impl AppCell {
|
||||
#[cold]
|
||||
#[track_caller]
|
||||
fn borrow_failed(&self, kind: &str) -> ! {
|
||||
let attempted = std::panic::Location::caller();
|
||||
if let Some((origin, existing_kind)) = self.borrow_origin.get() {
|
||||
panic!(
|
||||
"RefCell already borrowed: attempted {kind} App borrow at {attempted}; \
|
||||
existing {existing_kind} borrow group began at {origin}"
|
||||
);
|
||||
}
|
||||
panic!(
|
||||
"RefCell already borrowed: attempted {kind} App borrow at {attempted}; origin unavailable"
|
||||
);
|
||||
}
|
||||
|
||||
#[doc(hidden)]
|
||||
#[track_caller]
|
||||
pub fn borrow(&self) -> AppRef<'_> {
|
||||
|
|
@ -93,17 +109,27 @@ impl AppCell {
|
|||
let thread_id = std::thread::current().id();
|
||||
eprintln!("borrowed {thread_id:?}");
|
||||
}
|
||||
AppRef(self.app.borrow())
|
||||
// Keep the first origin across overlapping shared borrows. The old
|
||||
// origin may remain after release; the next group replaces it.
|
||||
let starts_borrow_group = self.app.try_borrow_mut().is_ok();
|
||||
let borrow = match self.app.try_borrow() {
|
||||
Ok(borrow) => borrow,
|
||||
Err(_) => self.borrow_failed("shared"),
|
||||
};
|
||||
if starts_borrow_group {
|
||||
self.borrow_origin
|
||||
.set(Some((std::panic::Location::caller(), "shared")));
|
||||
}
|
||||
AppRef(borrow)
|
||||
}
|
||||
|
||||
#[doc(hidden)]
|
||||
#[track_caller]
|
||||
pub fn borrow_mut(&self) -> AppRefMut<'_> {
|
||||
if option_env!("TRACK_THREAD_BORROWS").is_some() {
|
||||
let thread_id = std::thread::current().id();
|
||||
eprintln!("borrowed {thread_id:?}");
|
||||
match self.try_borrow_mut() {
|
||||
Ok(borrow) => borrow,
|
||||
Err(_) => self.borrow_failed("mutable"),
|
||||
}
|
||||
AppRefMut(self.app.borrow_mut())
|
||||
}
|
||||
|
||||
#[doc(hidden)]
|
||||
|
|
@ -113,7 +139,10 @@ impl AppCell {
|
|||
let thread_id = std::thread::current().id();
|
||||
eprintln!("borrowed {thread_id:?}");
|
||||
}
|
||||
Ok(AppRefMut(self.app.try_borrow_mut()?))
|
||||
let borrow = self.app.try_borrow_mut()?;
|
||||
self.borrow_origin
|
||||
.set(Some((std::panic::Location::caller(), "mutable")));
|
||||
Ok(AppRefMut(borrow))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -948,6 +977,7 @@ impl App {
|
|||
let _ref_counts = entities.ref_counts_drop_handle();
|
||||
|
||||
let app = Rc::new_cyclic(|this| AppCell {
|
||||
borrow_origin: Cell::new(None),
|
||||
app: RefCell::new(App {
|
||||
this: this.clone(),
|
||||
platform: platform.clone(),
|
||||
|
|
@ -3665,6 +3695,55 @@ mod test {
|
|||
TestAppContext, Window,
|
||||
};
|
||||
|
||||
#[gpui::test]
|
||||
fn app_borrow_diagnostics(cx: &mut TestAppContext) {
|
||||
use std::panic::{AssertUnwindSafe, catch_unwind};
|
||||
|
||||
let app = cx.app.clone();
|
||||
let first_line = line!() + 1;
|
||||
let first = app.borrow();
|
||||
let second = app.borrow();
|
||||
drop(first);
|
||||
assert!(app.try_borrow_mut().is_err());
|
||||
let attempted_line = line!() + 1;
|
||||
let panic = catch_unwind(AssertUnwindSafe(|| drop(app.borrow_mut())));
|
||||
let message = *panic
|
||||
.expect_err("conflicting borrow")
|
||||
.downcast::<String>()
|
||||
.unwrap();
|
||||
assert!(message.contains(&format!(
|
||||
"mutable App borrow at {}:{attempted_line}:",
|
||||
file!()
|
||||
)));
|
||||
assert!(message.contains(&format!(
|
||||
"shared borrow group began at {}:{first_line}:",
|
||||
file!()
|
||||
)));
|
||||
drop(second);
|
||||
|
||||
let origin_line = line!() + 2;
|
||||
let panic = catch_unwind(AssertUnwindSafe(|| {
|
||||
let _borrow = app.try_borrow_mut().expect("initial borrow");
|
||||
drop(app.borrow());
|
||||
}));
|
||||
let message = *panic
|
||||
.expect_err("conflicting borrow")
|
||||
.downcast::<String>()
|
||||
.unwrap();
|
||||
assert!(message.contains("attempted shared App borrow"));
|
||||
assert!(message.contains(&format!(
|
||||
"mutable borrow group began at {}:{origin_line}:",
|
||||
file!()
|
||||
)));
|
||||
|
||||
let next_line = line!() + 1;
|
||||
let _borrow = app.borrow();
|
||||
assert_eq!(
|
||||
app.borrow_origin.get().expect("new origin").0.line(),
|
||||
next_line
|
||||
);
|
||||
}
|
||||
|
||||
struct RenderCounter(Rc<Cell<usize>>);
|
||||
|
||||
impl Render for RenderCounter {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue