skyler
/
egui
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove the `deadlock_detection` feature (#7497) 

* related #7494 

Removes the `deadlock_detection` feature, since we now have a more
primitive panic-after-30s deadlock detection which works well enough and
even detects kinds of deadlocks that the `deadlock_detection` feature
never supported.
This commit is contained in:
Lucas Meurer 2025-09-04 12:57:09 +02:00 committed by GitHub
parent d66fa63e20
commit 80d61a7c53
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 94 additions and 395 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock
View File

@ -1535,7 +1535,6 @@ version = "0.32.1"
dependencies = [
"ab_glyph",
"ahash",
"backtrace",
"bytemuck",
"criterion",
"document-features",

5
crates/egui/Cargo.toml
View File

@ -44,11 +44,6 @@ cint = ["epaint/cint"]
## Enable the [`hex_color`] macro.
color-hex = ["epaint/color-hex"]
## This will automatically detect deadlocks due to double-locking on the same thread.
## If your app freezes, you may want to enable this!
## Only affects [`epaint::mutex::RwLock`] (which egui uses a lot).
deadlock_detection = ["epaint/deadlock_detection"]
## If set, egui will use `include_bytes!` to bundle some fonts.
## If you plan on specifying your own fonts you may disable this feature.
default_fonts = ["epaint/default_fonts"]

3
crates/egui_demo_app/tests/test_demo_app.rs
View File

@ -62,6 +62,9 @@ fn test_demo_app() {
.type_text("file://../eframe/data/icon.png");
harness.get_by_role_and_label(Role::Button, "✔").click();
// Wait for the image to load
harness.try_run_realtime().ok();
}
_ => {}
}

10
crates/epaint/Cargo.toml
View File

@ -40,11 +40,6 @@ cint = ["ecolor/cint"]
## Enable the [`hex_color`] macro.
color-hex = ["ecolor/color-hex"]
## This will automatically detect deadlocks due to double-locking on the same thread.
## If your app freezes, you may want to enable this!
## Only affects [`mutex::RwLock`] (which epaint and egui uses a lot).
deadlock_detection = ["dep:backtrace"]
## If set, epaint will use `include_bytes!` to bundle some fonts.
## If you plan on specifying your own fonts you may disable this feature.
default_fonts = ["epaint_default_fonts"]
@ -94,11 +89,6 @@ serde = { workspace = true, optional = true, features = ["derive", "rc"] }
epaint_default_fonts = { workspace = true, optional = true }
# native:
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
backtrace = { workspace = true, optional = true }
[dev-dependencies]
criterion.workspace = true
mimalloc.workspace = true

470
crates/epaint/src/mutex.rs
View File

@ -1,394 +1,107 @@
//! Helper module that adds extra checks when the `deadlock_detection` feature is turned on.
//! Wrappers around `parking_lot` locks, with a simple deadlock detection mechanism.
// ----------------------------------------------------------------------------
#[cfg(not(feature = "deadlock_detection"))]
const DEADLOCK_DURATION: std::time::Duration = std::time::Duration::from_secs(30);
const DEADLOCK_DURATION: std::time::Duration = std::time::Duration::from_secs(10);
#[cfg(not(feature = "deadlock_detection"))]
mod mutex_impl {
use super::DEADLOCK_DURATION;
/// Provides interior mutability.
///
/// It's tailored for internal use in egui should only be used for short locks (as a guideline,
/// locks should never be held longer than a single frame). In debug builds, when a lock can't
/// be acquired within 10 seconds, we assume a deadlock and will panic.
///
/// This is a thin wrapper around [`parking_lot::Mutex`].
#[derive(Default)]
pub struct Mutex<T>(parking_lot::Mutex<T>);
/// Provides interior mutability.
/// The lock you get from [`Mutex`].
pub use parking_lot::MutexGuard;
impl<T> Mutex<T> {
#[inline(always)]
pub fn new(val: T) -> Self {
Self(parking_lot::Mutex::new(val))
}
/// Try to acquire the lock.
///
/// This is a thin wrapper around [`parking_lot::Mutex`], except if
/// the feature `deadlock_detection` is turned enabled, in which case
/// extra checks are added to detect deadlocks.
#[derive(Default)]
pub struct Mutex<T>(parking_lot::Mutex<T>);
/// The lock you get from [`Mutex`].
pub use parking_lot::MutexGuard;
impl<T> Mutex<T> {
#[inline(always)]
pub fn new(val: T) -> Self {
Self(parking_lot::Mutex::new(val))
}
#[inline(always)]
pub fn lock(&self) -> MutexGuard<'_, T> {
if cfg!(debug_assertions) {
self.0
.try_lock_for(DEADLOCK_DURATION)
.expect("Looks like a deadlock!")
} else {
self.0.lock()
}
}
}
}
#[cfg(feature = "deadlock_detection")]
mod mutex_impl {
/// Provides interior mutability.
///
/// This is a thin wrapper around [`parking_lot::Mutex`], except if
/// the feature `deadlock_detection` is turned enabled, in which case
/// extra checks are added to detect deadlocks.
#[derive(Default)]
pub struct Mutex<T>(parking_lot::Mutex<T>);
/// The lock you get from [`Mutex`].
pub struct MutexGuard<'a, T>(parking_lot::MutexGuard<'a, T>, *const ());
#[derive(Default)]
struct HeldLocks(Vec<*const ()>);
impl HeldLocks {
#[inline(always)]
fn insert(&mut self, lock: *const ()) {
// Very few locks will ever be held at the same time, so a linear search is fast
assert!(
!self.0.contains(&lock),
"Recursively locking a Mutex in the same thread is not supported"
);
self.0.push(lock);
}
#[inline(always)]
fn remove(&mut self, lock: *const ()) {
self.0.retain(|&ptr| ptr != lock);
}
}
thread_local! {
static HELD_LOCKS_TLS: std::cell::RefCell<HeldLocks> = Default::default();
}
impl<T> Mutex<T> {
#[inline(always)]
pub fn new(val: T) -> Self {
Self(parking_lot::Mutex::new(val))
}
pub fn lock(&self) -> MutexGuard<'_, T> {
// Detect if we are recursively taking out a lock on this mutex.
// use a pointer to the inner data as an id for this lock
let ptr = std::ptr::from_ref::<parking_lot::Mutex<_>>(&self.0).cast::<()>();
// Store it in thread local storage while we have a lock guard taken out
HELD_LOCKS_TLS.with(|held_locks| {
held_locks.borrow_mut().insert(ptr);
});
MutexGuard(self.0.lock(), ptr)
}
#[inline(always)]
pub fn into_inner(self) -> T {
self.0.into_inner()
}
}
impl<T> Drop for MutexGuard<'_, T> {
fn drop(&mut self) {
let ptr = self.1;
HELD_LOCKS_TLS.with(|held_locks| {
held_locks.borrow_mut().remove(ptr);
});
}
}
impl<T> std::ops::Deref for MutexGuard<'_, T> {
type Target = T;
#[inline(always)]
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl<T> std::ops::DerefMut for MutexGuard<'_, T> {
#[inline(always)]
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}
}
// ----------------------------------------------------------------------------
#[cfg(not(feature = "deadlock_detection"))]
mod rw_lock_impl {
use super::DEADLOCK_DURATION;
/// The lock you get from [`RwLock::read`].
pub use parking_lot::MappedRwLockReadGuard as RwLockReadGuard;
/// The lock you get from [`RwLock::write`].
pub use parking_lot::MappedRwLockWriteGuard as RwLockWriteGuard;
/// Provides interior mutability.
///
/// This is a thin wrapper around [`parking_lot::RwLock`], except if
/// the feature `deadlock_detection` is turned enabled, in which case
/// extra checks are added to detect deadlocks.
#[derive(Default)]
pub struct RwLock<T: ?Sized>(parking_lot::RwLock<T>);
impl<T> RwLock<T> {
#[inline(always)]
pub fn new(val: T) -> Self {
Self(parking_lot::RwLock::new(val))
}
}
impl<T: ?Sized> RwLock<T> {
#[inline(always)]
pub fn read(&self) -> RwLockReadGuard<'_, T> {
let guard = if cfg!(debug_assertions) {
self.0
.try_read_for(DEADLOCK_DURATION)
.expect("Looks like a deadlock!")
} else {
self.0.read()
};
parking_lot::RwLockReadGuard::map(guard, |v| v)
}
#[inline(always)]
pub fn write(&self) -> RwLockWriteGuard<'_, T> {
let guard = if cfg!(debug_assertions) {
self.0
.try_write_for(DEADLOCK_DURATION)
.expect("Looks like a deadlock!")
} else {
self.0.write()
};
parking_lot::RwLockWriteGuard::map(guard, |v| v)
}
}
}
#[cfg(feature = "deadlock_detection")]
mod rw_lock_impl {
use std::{
ops::{Deref, DerefMut},
sync::Arc,
thread::ThreadId,
};
use ahash::HashMap;
use parking_lot::{MappedRwLockReadGuard, MappedRwLockWriteGuard};
/// The lock you get from [`RwLock::read`].
pub struct RwLockReadGuard<'a, T> {
// The option is used only because we need to `take()` the guard out of self
// when doing remappings (`map()`), i.e. it's used as a safe `ManuallyDrop`.
guard: Option<MappedRwLockReadGuard<'a, T>>,
holders: Arc<parking_lot::Mutex<HashMap<ThreadId, backtrace::Backtrace>>>,
}
impl<'a, T> RwLockReadGuard<'a, T> {
#[inline]
pub fn map<U, F>(mut s: Self, f: F) -> RwLockReadGuard<'a, U>
where
F: FnOnce(&T) -> &U,
{
RwLockReadGuard {
guard: s
.guard
.take()
.map(|g| parking_lot::MappedRwLockReadGuard::map(g, f)),
holders: Arc::clone(&s.holders),
}
}
}
impl<T> Deref for RwLockReadGuard<'_, T> {
type Target = T;
fn deref(&self) -> &Self::Target {
self.guard.as_ref().unwrap()
}
}
impl<T> Drop for RwLockReadGuard<'_, T> {
fn drop(&mut self) {
let tid = std::thread::current().id();
self.holders.lock().remove(&tid);
}
}
/// The lock you get from [`RwLock::write`].
pub struct RwLockWriteGuard<'a, T> {
// The option is used only because we need to `take()` the guard out of self
// when doing remappings (`map()`), i.e. it's used as a safe `ManuallyDrop`.
guard: Option<MappedRwLockWriteGuard<'a, T>>,
holders: Arc<parking_lot::Mutex<HashMap<ThreadId, backtrace::Backtrace>>>,
}
impl<'a, T> RwLockWriteGuard<'a, T> {
#[inline]
pub fn map<U, F>(mut s: Self, f: F) -> RwLockWriteGuard<'a, U>
where
F: FnOnce(&mut T) -> &mut U,
{
RwLockWriteGuard {
guard: s
.guard
.take()
.map(|g| parking_lot::MappedRwLockWriteGuard::map(g, f)),
holders: Arc::clone(&s.holders),
}
}
}
impl<T> Deref for RwLockWriteGuard<'_, T> {
type Target = T;
fn deref(&self) -> &Self::Target {
self.guard.as_ref().unwrap()
}
}
impl<T> DerefMut for RwLockWriteGuard<'_, T> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.guard.as_mut().unwrap()
}
}
impl<T> Drop for RwLockWriteGuard<'_, T> {
fn drop(&mut self) {
let tid = std::thread::current().id();
self.holders.lock().remove(&tid);
}
}
/// Provides interior mutability.
///
/// This is a thin wrapper around [`parking_lot::RwLock`], except if
/// the feature `deadlock_detection` is turned enabled, in which case
/// extra checks are added to detect deadlocks.
#[derive(Default)]
pub struct RwLock<T> {
lock: parking_lot::RwLock<T>,
// Technically we'd need a list of backtraces per thread-id since parking_lot's
// read-locks are reentrant.
// In practice it's not that useful to have the whole list though, so we only
// keep track of the first backtrace for now.
holders: Arc<parking_lot::Mutex<HashMap<ThreadId, backtrace::Backtrace>>>,
}
impl<T> RwLock<T> {
pub fn new(val: T) -> Self {
Self {
lock: parking_lot::RwLock::new(val),
holders: Default::default(),
}
}
pub fn read(&self) -> RwLockReadGuard<'_, T> {
let tid = std::thread::current().id();
// If it is write-locked, and we locked it (reentrancy deadlock)
let would_deadlock =
self.lock.is_locked_exclusive() && self.holders.lock().contains_key(&tid);
assert!(
!would_deadlock,
"{} DEAD-LOCK DETECTED ({:?})!\n\
Trying to grab read-lock at:\n{}\n\
which is already exclusively held by current thread at:\n{}\n\n",
std::any::type_name::<Self>(),
tid,
format_backtrace(&mut make_backtrace()),
format_backtrace(self.holders.lock().get_mut(&tid).unwrap())
);
self.holders
.lock()
.entry(tid)
.or_insert_with(make_backtrace);
RwLockReadGuard {
guard: parking_lot::RwLockReadGuard::map(self.lock.read(), |v| v).into(),
holders: Arc::clone(&self.holders),
}
}
pub fn write(&self) -> RwLockWriteGuard<'_, T> {
let tid = std::thread::current().id();
// If it is locked in any way, and we locked it (reentrancy deadlock)
let would_deadlock = self.lock.is_locked() && self.holders.lock().contains_key(&tid);
assert!(
!would_deadlock,
"{} DEAD-LOCK DETECTED ({:?})!\n\
Trying to grab write-lock at:\n{}\n\
which is already held by current thread at:\n{}\n\n",
std::any::type_name::<Self>(),
tid,
format_backtrace(&mut make_backtrace()),
format_backtrace(self.holders.lock().get_mut(&tid).unwrap())
);
self.holders
.lock()
.entry(tid)
.or_insert_with(make_backtrace);
RwLockWriteGuard {
guard: parking_lot::RwLockWriteGuard::map(self.lock.write(), |v| v).into(),
holders: Arc::clone(&self.holders),
}
}
#[inline(always)]
pub fn into_inner(self) -> T {
self.lock.into_inner()
}
}
fn make_backtrace() -> backtrace::Backtrace {
backtrace::Backtrace::new_unresolved()
}
fn format_backtrace(backtrace: &mut backtrace::Backtrace) -> String {
backtrace.resolve();
let stacktrace = format!("{backtrace:?}");
// Remove irrelevant parts of the stacktrace:
let end_offset = stacktrace
.find("std::sys_common::backtrace::__rust_begin_short_backtrace")
.unwrap_or(stacktrace.len());
let stacktrace = &stacktrace[..end_offset];
let first_interesting_function = "epaint::mutex::rw_lock_impl::make_backtrace\n";
if let Some(start_offset) = stacktrace.find(first_interesting_function) {
stacktrace[start_offset + first_interesting_function.len()..].to_owned()
/// ## Panics
/// Will panic in debug builds if the lock can't be acquired within 10 seconds.
#[inline(always)]
#[cfg_attr(debug_assertions, track_caller)]
pub fn lock(&self) -> MutexGuard<'_, T> {
if cfg!(debug_assertions) {
self.0
.try_lock_for(DEADLOCK_DURATION)
.expect("Looks like a deadlock!")
} else {
stacktrace.to_owned()
self.0.lock()
}
}
}
// ----------------------------------------------------------------------------
pub use mutex_impl::{Mutex, MutexGuard};
pub use rw_lock_impl::{RwLock, RwLockReadGuard, RwLockWriteGuard};
/// The lock you get from [`RwLock::read`].
pub use parking_lot::MappedRwLockReadGuard as RwLockReadGuard;
/// The lock you get from [`RwLock::write`].
pub use parking_lot::MappedRwLockWriteGuard as RwLockWriteGuard;
/// Provides interior mutability.
///
/// It's tailored for internal use in egui should only be used for short locks (as a guideline,
/// locks should never be held longer than a single frame). In debug builds, when a lock can't
/// be acquired within 10 seconds, we assume a deadlock and will panic.
///
/// This is a thin wrapper around [`parking_lot::RwLock`].
#[derive(Default)]
pub struct RwLock<T: ?Sized>(parking_lot::RwLock<T>);
impl<T> RwLock<T> {
#[inline(always)]
pub fn new(val: T) -> Self {
Self(parking_lot::RwLock::new(val))
}
}
impl<T: ?Sized> RwLock<T> {
/// Try to acquire read-access to the lock.
///
/// ## Panics
/// Will panic in debug builds if the lock can't be acquired within 10 seconds.
#[inline(always)]
#[cfg_attr(debug_assertions, track_caller)]
pub fn read(&self) -> RwLockReadGuard<'_, T> {
let guard = if cfg!(debug_assertions) {
self.0
.try_read_for(DEADLOCK_DURATION)
.expect("Looks like a deadlock!")
} else {
self.0.read()
};
parking_lot::RwLockReadGuard::map(guard, |v| v)
}
/// Try to acquire write-access to the lock.
///
/// ## Panics
/// Will panic in debug builds if the lock can't be acquired within 10 seconds.
#[inline(always)]
#[cfg_attr(debug_assertions, track_caller)]
pub fn write(&self) -> RwLockWriteGuard<'_, T> {
let guard = if cfg!(debug_assertions) {
self.0
.try_write_for(DEADLOCK_DURATION)
.expect("Looks like a deadlock!")
} else {
self.0.write()
};
parking_lot::RwLockWriteGuard::map(guard, |v| v)
}
}
// ----------------------------------------------------------------------------
impl<T> Clone for Mutex<T>
where
@ -434,7 +147,6 @@ mod tests {
}
#[cfg(not(target_arch = "wasm32"))]
#[cfg(feature = "deadlock_detection")]
#[cfg(test)]
mod tests_rwlock {
#![allow(clippy::disallowed_methods)] // Ok for tests