map_each deadlock prevention

map_each previously was written such that if a chain of mappings fed
each other, a deadlock could occur because while the first one was
mapped, the second callback gets invoked and tries to update the first
value while it's still being held.

This refactor switches from std Mutex to parking_lot, allowing me to
remove a workaround for needing to run drop callbacks in a separate
thread during the drop of a DynamicGuard.

In addition to that change, the lower level `map_generational` calls now
take a DynamicGuard as their parameter. This allows these functions to
drop ownership of the referenced data during the callback.

The map_each implementation takes advantage of this by ensuring that the
guard is dropped before set is invoked, minimizing potential lock overlaps.

With this refactor, some old code of mine with complex validations now works
again.
This commit is contained in:
Jonathan Johnson 2024-04-05 16:14:26 -07:00
parent beede55f0a
commit 20ae2b7c72
No known key found for this signature in database
GPG key ID: A66D6A34D6620579
14 changed files with 214 additions and 164 deletions

1
Cargo.lock generated
View file

@ -639,6 +639,7 @@ dependencies = [
"kludgine",
"nominals",
"palette",
"parking_lot",
"plotters",
"png",
"pollster",

View file

@ -45,6 +45,7 @@ png = "0.17.10"
image = { version = "0.25.0", features = ["png"] }
plotters = { version = "0.3.5", default-features = false, optional = true }
nominals = "0.3.0"
parking_lot = "0.12.1"
# [patch.crates-io]

View file

@ -14,11 +14,7 @@ fn main() -> cushy::Result {
"Inline Widgets"
.and(callback_widget())
.into_rows()
.and(
"impl MakeWidget"
.and(ToggleMakeWidget::default())
.into_rows(),
)
.and("impl MakeWidget".and(ToggleMakeWidget).into_rows())
.and("impl Widget".and(impl_widget()).into_rows())
.into_columns()
.centered()
@ -59,7 +55,7 @@ fn callback_widget() -> impl MakeWidgetWithTag {
/// widget or any of its children aren't focusable, implementing [`MakeWidget`]
/// directly will make more sense.
#[derive(Default)]
struct ToggleMakeWidget(Toggle);
struct ToggleMakeWidget;
impl MakeWidgetWithTag for ToggleMakeWidget {
fn make_with_tag(self, id: WidgetTag) -> WidgetInstance {

View file

@ -43,7 +43,7 @@ use std::cmp::Ordering;
use std::fmt::{Debug, Display};
use std::ops::{ControlFlow, Deref, Div, DivAssign, Mul, MulAssign, Sub};
use std::str::FromStr;
use std::sync::{Arc, Condvar, Mutex, MutexGuard, OnceLock};
use std::sync::{Arc, OnceLock};
use std::thread;
use std::time::{Duration, Instant};
@ -53,10 +53,11 @@ use figures::{Angle, Point, Ranged, Rect, Size, UnscaledUnit, Zero};
use intentional::Cast;
use kempt::Set;
use kludgine::Color;
use parking_lot::{Condvar, Mutex, MutexGuard};
use crate::animation::easings::Linear;
use crate::styles::{Component, RequireInvalidation};
use crate::utils::{run_in_bg, IgnorePoison};
use crate::utils::run_in_bg;
use crate::value::{Destination, Dynamic, Source};
static ANIMATIONS: Mutex<Animating> = Mutex::new(Animating::new());
@ -67,7 +68,7 @@ fn thread_state() -> MutexGuard<'static, Animating> {
THREAD.get_or_init(|| {
thread::spawn(animation_thread);
});
ANIMATIONS.lock().ignore_poison()
ANIMATIONS.lock()
}
fn animation_thread() {
@ -75,7 +76,7 @@ fn animation_thread() {
loop {
if state.running.is_empty() {
state.last_updated = None;
state = NEW_ANIMATIONS.wait(state).ignore_poison();
NEW_ANIMATIONS.wait(&mut state);
} else {
let start = Instant::now();
let last_tick = state.last_updated.unwrap_or(start);

View file

@ -1,10 +1,10 @@
use std::sync::{Arc, Mutex, MutexGuard};
use std::sync::Arc;
use arboard::Clipboard;
use kludgine::app::{AppEvent, AsApplication};
use parking_lot::{Mutex, MutexGuard};
use crate::fonts::FontCollection;
use crate::utils::IgnorePoison;
use crate::window::sealed::WindowCommand;
use crate::window::WindowHandle;
@ -64,9 +64,7 @@ impl Cushy {
/// initialized when the window opened.
#[must_use]
pub fn clipboard_guard(&self) -> Option<MutexGuard<'_, Clipboard>> {
self.clipboard
.as_ref()
.map(|mutex| mutex.lock().ignore_poison())
self.clipboard.as_ref().map(|mutex| mutex.lock())
}
/// Returns the font collection that will be loaded in all Cushy windows.

View file

@ -1414,11 +1414,11 @@ impl<T> Trackable for T where T: sealed::Trackable {}
pub(crate) mod sealed {
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex, MutexGuard};
use std::sync::Arc;
use kempt::Set;
use parking_lot::{Mutex, MutexGuard};
use crate::utils::IgnorePoison;
use crate::widget::WidgetId;
use crate::window::WindowHandle;
@ -1445,12 +1445,12 @@ pub(crate) mod sealed {
}
pub fn invalidate(&self, widget: WidgetId) -> bool {
let mut invalidated = self.invalidated.lock().ignore_poison();
let mut invalidated = self.invalidated.lock();
invalidated.insert(widget)
}
pub fn invalidations(&self) -> MutexGuard<'_, Set<WidgetId>> {
self.invalidated.lock().ignore_poison()
self.invalidated.lock()
}
}

View file

@ -1,5 +1,5 @@
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Condvar, Mutex, MutexGuard};
use std::sync::Arc;
use std::time::{Duration, Instant};
use ahash::AHashSet;
@ -8,9 +8,9 @@ use figures::Point;
use intentional::Assert;
use kludgine::app::winit::event::{ElementState, MouseButton};
use kludgine::app::winit::keyboard::Key;
use parking_lot::{Condvar, Mutex, MutexGuard};
use crate::context::WidgetContext;
use crate::utils::IgnorePoison;
use crate::value::{Destination, Dynamic};
use crate::widget::{EventHandling, HANDLED, IGNORED};
use crate::window::KeyEvent;
@ -170,7 +170,7 @@ struct TickData {
impl TickData {
fn state(&self) -> MutexGuard<'_, TickState> {
self.state.lock().ignore_poison()
self.state.lock()
}
}
@ -218,7 +218,7 @@ where
while state.keep_running {
let current_frame = data.rendered_frame.load(Ordering::Acquire);
if state.frame == current_frame {
state = data.sync.wait(state).ignore_poison();
data.sync.wait(&mut state);
} else {
break;
}

View file

@ -1,13 +1,13 @@
use std::mem;
use std::sync::{Arc, Mutex, Weak};
use std::sync::{Arc, Weak};
use ahash::AHashMap;
use alot::{LotId, Lots};
use figures::units::{Px, UPx};
use figures::{Point, Rect, Size};
use parking_lot::Mutex;
use crate::styles::{Styles, ThemePair, VisualOrder};
use crate::utils::IgnorePoison;
use crate::value::Value;
use crate::widget::{MountedWidget, WidgetId, WidgetInstance};
use crate::window::{ThemeMode, WindowHandle};
@ -24,7 +24,7 @@ impl Tree {
widget: WidgetInstance,
parent: Option<&MountedWidget>,
) -> MountedWidget {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
let id = widget.id();
let (effective_styles, parent_id) = if let Some(parent) = parent {
(
@ -72,7 +72,7 @@ impl Tree {
parent: &MountedWidget,
children_to_unmount: &mut Vec<WidgetId>,
) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.remove_child(child.node_id, parent.node_id, children_to_unmount);
if child.widget.is_default() {
@ -84,7 +84,7 @@ impl Tree {
}
pub(crate) fn set_layout(&self, widget: LotId, rect: Rect<Px>) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
let node = &mut data.nodes[widget];
node.layout = Some(rect);
@ -104,12 +104,12 @@ impl Tree {
}
pub(crate) fn layout(&self, widget: LotId) -> Option<Rect<Px>> {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
data.nodes.get(widget).and_then(|widget| widget.layout)
}
pub(crate) fn new_frame(&self, invalidations: impl IntoIterator<Item = WidgetId>) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.render_info.clear();
for id in invalidations {
@ -122,7 +122,7 @@ impl Tree {
}
pub(crate) fn note_widget_rendered(&self, widget: LotId) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
let Some(layout) = data.nodes.get(widget).and_then(|node| node.layout) else {
return;
};
@ -134,7 +134,7 @@ impl Tree {
parent: LotId,
constraints: Size<ConstraintLimit>,
) -> Option<Size<UPx>> {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
let node = &mut data.nodes[parent];
if let Some(cached_layout) = &node.last_layout_query {
@ -161,7 +161,7 @@ impl Tree {
constraints: Size<ConstraintLimit>,
size: Size<UPx>,
) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.nodes[id].last_layout_query = Some(CachedLayoutQuery { constraints, size });
}
@ -170,7 +170,7 @@ impl Tree {
parent: LotId,
order: VisualOrder,
) -> Vec<MountedWidget> {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
let node = &data.nodes[parent];
let mut unordered = node.children.clone();
let mut ordered = Vec::<MountedWidget>::with_capacity(unordered.len());
@ -230,12 +230,12 @@ impl Tree {
}
pub(crate) fn effective_styles(&self, id: LotId) -> Styles {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
data.nodes[id].effective_styles.clone()
}
pub(crate) fn hover(&self, new_hover: Option<&MountedWidget>) -> HoverResults {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
let hovered = new_hover
.map(|new_hover| data.widget_hierarchy(new_hover.node_id, self))
.unwrap_or_default();
@ -260,12 +260,12 @@ impl Tree {
}
pub fn focus(&self, new_focus: Option<WidgetId>) -> Result<Option<MountedWidget>, ()> {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.update_tracked_widget(new_focus, self, |data| &mut data.focus)
}
pub fn previous_focus(&self, focus: WidgetId) -> Option<MountedWidget> {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
let previous = *data.previous_focuses.get(&focus)?;
data.widget_from_id(previous, self)
}
@ -274,24 +274,24 @@ impl Tree {
&self,
new_active: Option<&MountedWidget>,
) -> Result<Option<MountedWidget>, ()> {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.update_tracked_widget(new_active.map(MountedWidget::id), self, |data| {
&mut data.active
})
}
pub fn widget(&self, id: WidgetId) -> Option<MountedWidget> {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
data.widget_from_id(id, self)
}
pub(crate) fn widget_from_node(&self, id: LotId) -> Option<MountedWidget> {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
data.widget_from_node(id, self)
}
pub(crate) fn is_enabled(&self, mut id: LotId, context: &WindowHandle) -> bool {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
loop {
let Some(node) = data.nodes.get(id) else {
return false;
@ -310,23 +310,23 @@ impl Tree {
}
pub(crate) fn active_widget(&self) -> Option<LotId> {
self.data.lock().ignore_poison().active
self.data.lock().active
}
pub(crate) fn hovered_widget(&self) -> Option<LotId> {
self.data.lock().ignore_poison().hover
self.data.lock().hover
}
pub(crate) fn default_widget(&self) -> Option<LotId> {
self.data.lock().ignore_poison().defaults.last().copied()
self.data.lock().defaults.last().copied()
}
pub(crate) fn escape_widget(&self) -> Option<LotId> {
self.data.lock().ignore_poison().escapes.last().copied()
self.data.lock().escapes.last().copied()
}
pub(crate) fn is_hovered(&self, id: LotId) -> bool {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
let mut search = data.hover;
while let Some(hovered) = search {
if hovered == id {
@ -339,21 +339,21 @@ impl Tree {
}
pub(crate) fn focused_widget(&self) -> Option<LotId> {
self.data.lock().ignore_poison().focus
self.data.lock().focus
}
pub(crate) fn widgets_under_point(&self, point: Point<Px>) -> Vec<MountedWidget> {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
data.render_info.widgets_under_point(point, &data, self)
}
pub(crate) fn parent(&self, id: LotId) -> Option<LotId> {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
data.nodes.get(id).expect("missing widget").parent
}
pub(crate) fn is_child(&self, mut id: LotId, possible_parent: &WidgetInstance) -> bool {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
while let Some(node) = data.nodes.get(id) {
if &node.widget == possible_parent {
return true;
@ -371,17 +371,17 @@ impl Tree {
}
pub(crate) fn attach_styles(&self, id: LotId, styles: Value<Styles>) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.attach_styles(id, styles);
}
pub(crate) fn attach_theme(&self, id: LotId, theme: Value<ThemePair>) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.nodes.get_mut(id).expect("missing widget").theme = Some(theme);
}
pub(crate) fn attach_theme_mode(&self, id: LotId, theme: Value<ThemeMode>) {
let mut data = self.data.lock().ignore_poison();
let mut data = self.data.lock();
data.nodes.get_mut(id).expect("missing widget").theme_mode = Some(theme);
}
@ -389,7 +389,7 @@ impl Tree {
&self,
id: LotId,
) -> (Styles, Option<Value<ThemePair>>, Option<Value<ThemeMode>>) {
let data = self.data.lock().ignore_poison();
let data = self.data.lock();
let node = data.nodes.get(id).expect("missing widget");
(
node.effective_styles.clone(),
@ -399,10 +399,7 @@ impl Tree {
}
pub fn invalidate(&self, id: LotId, include_hierarchy: bool) {
self.data
.lock()
.ignore_poison()
.invalidate(id, include_hierarchy);
self.data.lock().invalidate(id, include_hierarchy);
}
}

View file

@ -1,6 +1,6 @@
use std::ops::Deref;
use std::sync::mpsc::{self, SyncSender};
use std::sync::{OnceLock, PoisonError};
use std::sync::OnceLock;
use intentional::Assert;
use kludgine::app::winit::event::Modifiers;
@ -225,19 +225,6 @@ impl<T> Deref for Lazy<T> {
}
}
pub trait IgnorePoison {
type Unwrapped;
fn ignore_poison(self) -> Self::Unwrapped;
}
impl<T> IgnorePoison for Result<T, PoisonError<T>> {
type Unwrapped = T;
fn ignore_poison(self) -> Self::Unwrapped {
self.map_or_else(PoisonError::into_inner, |g| g)
}
}
pub trait BgFunction: FnOnce() + Send + 'static {}
pub fn run_in_bg<F>(f: F)

View file

@ -7,7 +7,7 @@ use std::future::Future;
use std::hash::{BuildHasher, Hash};
use std::ops::{Add, AddAssign, Deref, DerefMut, Not};
use std::str::FromStr;
use std::sync::{Arc, Condvar, Mutex, MutexGuard, TryLockError, Weak};
use std::sync::{Arc, Weak};
use std::task::{Poll, Waker};
use std::thread::{self, ThreadId};
use std::time::{Duration, Instant};
@ -16,10 +16,11 @@ use ahash::AHashSet;
use alot::{LotId, Lots};
use intentional::Assert;
use kempt::{Map, Sort};
use parking_lot::{Condvar, Mutex, MutexGuard};
use crate::animation::{AnimationHandle, DynamicTransition, IntoAnimate, LinearInterpolate, Spawn};
use crate::context::{self, Trackable, WidgetContext};
use crate::utils::{run_in_bg, IgnorePoison, WithClone};
use crate::utils::WithClone;
use crate::widget::{
MakeWidget, MakeWidgetWithTag, OnceCallback, WidgetId, WidgetInstance, WidgetList,
};
@ -32,7 +33,7 @@ pub trait Source<T> {
/// [`Generation`].
fn try_map_generational<R>(
&self,
map: impl FnOnce(&GenerationalValue<T>) -> R,
map: impl FnOnce(DynamicGuard<'_, T, true>) -> R,
) -> Result<R, DeadlockError>;
/// Maps the contents with read-only access, providing access to the value's
@ -42,7 +43,7 @@ pub trait Source<T> {
///
/// This function panics if this value is already locked by the current
/// thread.
fn map_generational<R>(&self, map: impl FnOnce(&GenerationalValue<T>) -> R) -> R {
fn map_generational<R>(&self, map: impl FnOnce(DynamicGuard<'_, T, true>) -> R) -> R {
self.try_map_generational(map).expect("deadlocked")
}
@ -54,7 +55,7 @@ pub trait Source<T> {
/// thread.
#[must_use]
fn generation(&self) -> Generation {
self.map_generational(GenerationalValue::generation)
self.map_generational(|g| g.generation())
}
/// Maps the contents with read-only access.
@ -64,7 +65,7 @@ pub trait Source<T> {
/// This function panics if this value is already locked by the current
/// thread.
fn map_ref<R>(&self, map: impl FnOnce(&T) -> R) -> R {
self.map_generational(|gen| map(&gen.value))
self.map_generational(|gen| map(&*gen))
}
/// Returns a clone of the currently contained value.
@ -83,7 +84,7 @@ pub trait Source<T> {
/// Maps the contents with read-only access.
fn try_map_ref<R>(&self, map: impl FnOnce(&T) -> R) -> Result<R, DeadlockError> {
self.try_map_generational(|gen| map(&gen.value))
self.try_map_generational(|gen| map(&*gen))
}
/// Returns a clone of the currently contained value.
@ -91,7 +92,7 @@ pub trait Source<T> {
where
T: Clone,
{
self.try_map_generational(|gen| gen.value.clone())
self.try_map_generational(|gen| gen.clone())
}
/// Returns a clone of the currently contained value.
@ -138,7 +139,7 @@ pub trait Source<T> {
fn for_each_generational_try<F>(&self, for_each: F) -> CallbackHandle
where
T: Send + 'static,
F: for<'a> FnMut(&'a GenerationalValue<T>) -> Result<(), CallbackDisconnected>
F: for<'a> FnMut(DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected>
+ Send
+ 'static;
@ -147,7 +148,7 @@ pub trait Source<T> {
fn for_each_generational<F>(&self, mut for_each: F) -> CallbackHandle
where
T: Send + 'static,
F: for<'a> FnMut(&'a GenerationalValue<T>) + Send + 'static,
F: for<'a> FnMut(DynamicGuard<'_, T, true>) + Send + 'static,
{
self.for_each_generational_try(move |value| {
for_each(value);
@ -165,7 +166,7 @@ pub trait Source<T> {
T: Send + 'static,
F: for<'a> FnMut(&'a T) -> Result<(), CallbackDisconnected> + Send + 'static,
{
self.for_each_generational_try(move |gen| for_each(&gen.value))
self.for_each_generational_try(move |gen| for_each(&*gen))
}
/// Attaches `for_each` to this value so that it is invoked each time the
@ -248,7 +249,7 @@ pub trait Source<T> {
fn map_each_generational<R, F>(&self, mut map: F) -> Dynamic<R>
where
T: Send + 'static,
F: for<'a> FnMut(&'a GenerationalValue<T>) -> R + Send + 'static,
F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> R + Send + 'static,
R: PartialEq + Send + 'static,
{
let mapped = Dynamic::new(self.map_generational(&mut map));
@ -269,7 +270,7 @@ pub trait Source<T> {
F: for<'a> FnMut(&'a T) -> R + Send + 'static,
R: PartialEq + Send + 'static,
{
self.map_each_generational(move |gen| map(&gen.value))
self.map_each_generational(move |gen| map(&*gen))
}
/// Creates a new dynamic value that contains the result of invoking `map`
@ -520,16 +521,20 @@ pub trait Destination<T> {
impl<T> Source<T> for Arc<DynamicData<T>> {
fn try_map_generational<R>(
&self,
map: impl FnOnce(&GenerationalValue<T>) -> R,
map: impl FnOnce(DynamicGuard<'_, T, true>) -> R,
) -> Result<R, DeadlockError> {
let state = self.state()?;
Ok(map(&state.wrapped))
Ok(map(DynamicGuard {
guard: DynamicOrOwnedGuard::Dynamic(state),
accessed_mut: false,
prevent_notifications: false,
}))
}
fn for_each_generational_try<F>(&self, mut for_each: F) -> CallbackHandle
where
T: Send + 'static,
F: for<'a> FnMut(&'a GenerationalValue<T>) -> Result<(), CallbackDisconnected>
F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> Result<(), CallbackDisconnected>
+ Send
+ 'static,
{
@ -550,7 +555,7 @@ impl<T> Source<T> for Arc<DynamicData<T>> {
dynamic_for_each(self, move || {
let this = this.upgrade().ok_or(CallbackDisconnected)?;
if let Ok(value) = this.try_map_generational(GenerationalValue::clone) {
if let Ok(value) = this.try_map_generational(|g| g.guard.clone()) {
for_each(value)?;
}
@ -562,7 +567,7 @@ impl<T> Source<T> for Arc<DynamicData<T>> {
impl<T> Source<T> for Dynamic<T> {
fn try_map_generational<R>(
&self,
map: impl FnOnce(&GenerationalValue<T>) -> R,
map: impl FnOnce(DynamicGuard<'_, T, true>) -> R,
) -> Result<R, DeadlockError> {
self.0.try_map_generational(map)
}
@ -570,7 +575,7 @@ impl<T> Source<T> for Dynamic<T> {
fn for_each_generational_try<F>(&self, for_each: F) -> CallbackHandle
where
T: Send + 'static,
F: for<'a> FnMut(&'a GenerationalValue<T>) -> Result<(), CallbackDisconnected>
F: for<'a> FnMut(DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected>
+ Send
+ 'static,
{
@ -589,10 +594,10 @@ impl<T> Source<T> for Dynamic<T> {
impl<T> Source<T> for DynamicReader<T> {
fn try_map_generational<R>(
&self,
map: impl FnOnce(&GenerationalValue<T>) -> R,
map: impl FnOnce(DynamicGuard<'_, T, true>) -> R,
) -> Result<R, DeadlockError> {
self.source.try_map_generational(|generational| {
*self.read_generation.lock().ignore_poison() = generational.generation;
*self.read_generation.lock() = generational.generation();
map(generational)
})
}
@ -600,7 +605,7 @@ impl<T> Source<T> for DynamicReader<T> {
fn for_each_generational_try<F>(&self, for_each: F) -> CallbackHandle
where
T: Send + 'static,
F: for<'a> FnMut(&'a GenerationalValue<T>) -> Result<(), CallbackDisconnected>
F: for<'a> FnMut(DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected>
+ Send
+ 'static,
{
@ -735,19 +740,23 @@ impl<T> Owned<T> {
impl<T> Source<T> for Owned<T> {
fn try_map_generational<R>(
&self,
map: impl FnOnce(&GenerationalValue<T>) -> R,
map: impl FnOnce(DynamicGuard<'_, T, true>) -> R,
) -> Result<R, DeadlockError> {
Ok(map(&self.wrapped.borrow()))
Ok(map(DynamicGuard {
guard: DynamicOrOwnedGuard::Owned(self.wrapped.borrow_mut()),
accessed_mut: false,
prevent_notifications: false,
}))
}
fn for_each_generational_try<F>(&self, for_each: F) -> CallbackHandle
where
T: Send + 'static,
F: for<'a> FnMut(&'a GenerationalValue<T>) -> Result<(), CallbackDisconnected>
F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> Result<(), CallbackDisconnected>
+ Send
+ 'static,
{
let mut callbacks = self.callbacks.active.lock().ignore_poison();
let mut callbacks = self.callbacks.active.lock();
CallbackHandle(CallbackHandleInner::Single(CallbackHandleData {
id: Some(callbacks.push(Box::new(for_each))),
owner: None,
@ -760,7 +769,7 @@ impl<T> Source<T> for Owned<T> {
T: Clone + Send + 'static,
F: FnMut(GenerationalValue<T>) -> Result<(), CallbackDisconnected> + Send + 'static,
{
self.for_each_generational_try(move |value| for_each(value.clone()))
self.for_each_generational_try(move |gen| for_each(gen.guard.clone()))
}
}
@ -775,7 +784,9 @@ where
&mut updated,
));
if updated {
self.callbacks.invoke(&self.wrapped.borrow());
self.callbacks.invoke(&mut &self.wrapped, |wrapped| {
DynamicOrOwnedGuard::Owned(wrapped.borrow_mut())
});
}
Ok(result)
}
@ -829,7 +840,9 @@ where
{
fn drop(&mut self) {
if self.accessed_mut {
self.owned.callbacks.invoke(&self.borrowed);
self.owned.callbacks.invoke(&mut self.borrowed, |borrowed| {
DynamicOrOwnedGuard::OwnedRef(&mut *borrowed)
});
}
}
}
@ -850,9 +863,21 @@ impl<T> OwnedCallbacks<T>
where
T: 'static,
{
pub fn invoke(&self, value: &GenerationalValue<T>) {
let mut callbacks = self.active.lock().ignore_poison();
callbacks.drain_filter(|callback| callback.updated(value).is_err());
pub fn invoke<'a, U>(
&self,
user: &'a mut U,
value: impl for<'b> Fn(&'b mut U) -> DynamicOrOwnedGuard<'b, T>,
) {
let mut callbacks = self.active.lock();
callbacks.drain_filter(|callback| {
callback
.updated(DynamicGuard {
guard: value(user),
accessed_mut: false,
prevent_notifications: false,
})
.is_err()
});
}
}
@ -861,19 +886,21 @@ where
T: 'static,
{
fn remove(&self, id: LotId) {
self.active.lock().ignore_poison().remove(id);
self.active.lock().remove(id);
}
}
trait OwnedCallbackFn<T>: Send + 'static {
fn updated(&mut self, value: &GenerationalValue<T>) -> Result<(), CallbackDisconnected>;
fn updated(&mut self, value: DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected>;
}
impl<F, T> OwnedCallbackFn<T> for F
where
F: for<'a> FnMut(&'a GenerationalValue<T>) -> Result<(), CallbackDisconnected> + Send + 'static,
F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> Result<(), CallbackDisconnected>
+ Send
+ 'static,
{
fn updated(&mut self, value: &GenerationalValue<T>) -> Result<(), CallbackDisconnected> {
fn updated(&mut self, value: DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected> {
self(value)
}
}
@ -1104,7 +1131,7 @@ impl<T> Dynamic<T> {
/// dynamic.
pub fn try_lock(&self) -> Result<DynamicGuard<'_, T>, DeadlockError> {
Ok(DynamicGuard {
guard: self.0.state()?,
guard: DynamicOrOwnedGuard::Dynamic(self.0.state()?),
accessed_mut: false,
prevent_notifications: false,
})
@ -1112,7 +1139,7 @@ impl<T> Dynamic<T> {
fn lock_inner<const READONLY: bool>(&self) -> DynamicGuard<'_, T, READONLY> {
DynamicGuard {
guard: self.0.state().expect("deadlocked"),
guard: DynamicOrOwnedGuard::Dynamic(self.0.state().expect("deadlocked")),
accessed_mut: false,
prevent_notifications: false,
}
@ -1344,9 +1371,19 @@ where
}
}
impl<'a, T> DynamicMutexGuard<'a, T> {
fn unlocked(&mut self, while_unlocked: impl FnOnce()) {
MutexGuard::unlocked(&mut self.guard, || {
let current_state = self.dynamic.during_callback_state.lock().take();
while_unlocked();
*self.dynamic.during_callback_state.lock() = current_state;
});
}
}
impl<'a, T> Drop for DynamicMutexGuard<'a, T> {
fn drop(&mut self) {
let mut during_state = self.dynamic.during_callback_state.lock().ignore_poison();
let mut during_state = self.dynamic.during_callback_state.lock();
*during_state = None;
drop(during_state);
self.dynamic.sync.notify_all();
@ -1379,20 +1416,19 @@ struct DynamicData<T> {
impl<T> DynamicData<T> {
fn state(&self) -> Result<DynamicMutexGuard<'_, T>, DeadlockError> {
let mut during_sync = self.during_callback_state.lock().ignore_poison();
let mut during_sync = self.during_callback_state.lock();
let current_thread_id = std::thread::current().id();
let guard = loop {
match self.state.try_lock() {
Ok(g) => break g,
Err(TryLockError::Poisoned(poision)) => break poision.into_inner(),
Err(TryLockError::WouldBlock) => loop {
Some(g) => break g,
None => loop {
match &*during_sync {
Some(state) if state.locked_thread == current_thread_id => {
return Err(DeadlockError)
}
Some(_) => {
during_sync = self.sync.wait(during_sync).ignore_poison();
self.sync.wait(&mut during_sync);
}
None => break,
}
@ -1443,7 +1479,7 @@ where
T: Send + 'static,
{
let state = this.state().expect("deadlocked");
let mut data = state.callbacks.callbacks.lock().ignore_poison();
let mut data = state.callbacks.callbacks.lock();
CallbackHandle(CallbackHandleInner::Single(CallbackHandleData {
id: Some(data.callbacks.push(Box::new(map))),
owner: Some(this.clone()),
@ -1816,7 +1852,7 @@ struct ChangeCallbacksData {
impl CallbackCollection for ChangeCallbacksData {
fn remove(&self, id: LotId) {
let mut data = self.callbacks.lock().ignore_poison();
let mut data = self.callbacks.lock();
data.callbacks.remove(id);
}
}
@ -1842,7 +1878,7 @@ struct ChangeCallbacks {
impl Drop for ChangeCallbacks {
fn drop(&mut self) {
let mut currently_executing = self.data.currently_executing.lock().expect("lock poisoned");
let mut currently_executing = self.data.currently_executing.lock();
let current_thread = thread::current().id();
loop {
match &*currently_executing {
@ -1854,7 +1890,7 @@ impl Drop for ChangeCallbacks {
drop(currently_executing);
// Invoke the callbacks
let mut state = self.data.callbacks.lock().ignore_poison();
let mut state = self.data.callbacks.lock();
// If the callbacks have already been invoked by another
// thread such that the callbacks observed the value our
// thread wrote, we can skip the callbacks.
@ -1870,8 +1906,7 @@ impl Drop for ChangeCallbacks {
// Remove ourselves as the current executor, notifying any
// other threads that are waiting.
currently_executing =
self.data.currently_executing.lock().expect("lock poisoned");
currently_executing = self.data.currently_executing.lock();
*currently_executing = None;
drop(currently_executing);
self.data.sync.notify_all();
@ -1886,11 +1921,7 @@ impl Drop for ChangeCallbacks {
return;
}
Some(_) => {
currently_executing = self
.data
.sync
.wait(currently_executing)
.expect("lock poisoned");
self.data.sync.wait(&mut currently_executing);
}
}
}
@ -1964,13 +1995,57 @@ impl<T> DerefMut for GenerationalValue<T> {
}
}
#[derive(Debug)]
enum DynamicOrOwnedGuard<'a, T> {
Dynamic(DynamicMutexGuard<'a, T>),
Owned(RefMut<'a, GenerationalValue<T>>),
OwnedRef(&'a mut GenerationalValue<T>),
}
impl<'a, T> DynamicOrOwnedGuard<'a, T> {
fn note_changed(&mut self) -> Option<ChangeCallbacks> {
match self {
Self::Dynamic(guard) => Some(guard.note_changed()),
Self::Owned(_) | Self::OwnedRef(_) => None,
}
}
fn unlocked(&mut self, while_unlocked: impl FnOnce()) {
match self {
Self::Dynamic(guard) => guard.unlocked(while_unlocked),
Self::Owned(_) | Self::OwnedRef(_) => while_unlocked(),
}
}
}
impl<'a, T> Deref for DynamicOrOwnedGuard<'a, T> {
type Target = GenerationalValue<T>;
fn deref(&self) -> &Self::Target {
match self {
Self::Dynamic(guard) => &guard.wrapped,
Self::Owned(r) => r,
Self::OwnedRef(r) => r,
}
}
}
impl<'a, T> DerefMut for DynamicOrOwnedGuard<'a, T> {
fn deref_mut(&mut self) -> &mut Self::Target {
match self {
Self::Dynamic(guard) => &mut guard.wrapped,
Self::Owned(r) => r,
Self::OwnedRef(r) => r,
}
}
}
/// An exclusive reference to the contents of a [`Dynamic`].
///
/// If the contents are accessed through [`DerefMut`], all obververs will be
/// notified of a change when this guard is dropped.
#[derive(Debug)]
pub struct DynamicGuard<'a, T, const READONLY: bool = false> {
guard: DynamicMutexGuard<'a, T>,
guard: DynamicOrOwnedGuard<'a, T>,
accessed_mut: bool,
prevent_notifications: bool,
}
@ -1982,7 +2057,7 @@ impl<T, const READONLY: bool> DynamicGuard<'_, T, READONLY> {
/// will remain unchanged while the guard is held.
#[must_use]
pub fn generation(&self) -> Generation {
self.guard.wrapped.generation
self.guard.generation
}
/// Prevent any access through [`DerefMut`] from triggering change
@ -1996,22 +2071,22 @@ impl<'a, T, const READONLY: bool> Deref for DynamicGuard<'a, T, READONLY> {
type Target = T;
fn deref(&self) -> &Self::Target {
&self.guard.wrapped.value
&self.guard.value
}
}
impl<'a, T> DerefMut for DynamicGuard<'a, T, false> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.accessed_mut = true;
&mut self.guard.wrapped.value
&mut self.guard.value
}
}
impl<T, const READONLY: bool> Drop for DynamicGuard<'_, T, READONLY> {
fn drop(&mut self) {
if self.accessed_mut && !self.prevent_notifications {
let mut callbacks = Some(self.guard.note_changed());
run_in_bg(move || drop(callbacks.take()));
let callbacks = self.guard.note_changed();
self.guard.unlocked(|| drop(callbacks));
}
}
}
@ -2103,7 +2178,7 @@ impl<T> DynamicReader<T> {
#[must_use]
pub fn lock(&self) -> DynamicGuard<'_, T, true> {
DynamicGuard {
guard: self.source.state().expect("deadlocked"),
guard: DynamicOrOwnedGuard::Dynamic(self.source.state().expect("deadlocked")),
accessed_mut: false,
prevent_notifications: false,
}
@ -2113,7 +2188,7 @@ impl<T> DynamicReader<T> {
/// reader.
#[must_use]
pub fn read_generation(&self) -> Generation {
*self.read_generation.lock().ignore_poison()
*self.read_generation.lock()
}
/// Returns true if the dynamic has been modified since the last time the
@ -2142,13 +2217,12 @@ impl<T> DynamicReader<T> {
self.source
.during_callback_state
.lock()
.ignore_poison()
.as_ref()
.map_or(true, |state| state.locked_thread
!= std::thread::current().id()),
"deadlocked"
);
let mut state = self.source.state.lock().ignore_poison();
let mut state = self.source.state.lock();
loop {
if state.wrapped.generation != self.read_generation() {
return true;
@ -2159,14 +2233,14 @@ impl<T> DynamicReader<T> {
}
// Wait for a notification of a change, which is synch
state = self.source.sync.wait(state).ignore_poison();
self.source.sync.wait(&mut state);
}
}
/// Returns true if this reader still has any writers connected to it.
#[must_use]
pub fn connected(&self) -> bool {
let state = self.source.state.lock().ignore_poison();
let state = self.source.state.lock();
state.readers < Arc::strong_count(&self.source) && state.on_disconnect.is_some()
}
@ -2936,7 +3010,7 @@ macro_rules! impl_tuple_for_each {
move |$var: &$type| {
$(let $rvar = $rvar.read();)+
let mut for_each =
for_each.lock().ignore_poison();
for_each.lock();
(for_each)(($(&$avar,)+));
}
}));
@ -3108,7 +3182,7 @@ macro_rules! impl_tuple_for_each_cloned {
$handles += $var.for_each_cloned((&$for_each, $(&$rvar,)+).with_clone(|(for_each, $($rvar,)+)| {
move |$var: $type| {
$(let $rvar = $rvar.get();)+
if let Ok(mut for_each) =
if let Some(mut for_each) =
for_each.try_lock() {
(for_each)(($($avar,)+));
}
@ -3259,7 +3333,7 @@ impl Validations {
{
let validation = Dynamic::new(Validation::None);
let mut message_mapping = Self::map_to_message(move |value| check(value));
let error_message = dynamic.map_each_generational(move |value| message_mapping(value));
let error_message = dynamic.map_each_generational(move |gen| message_mapping(&gen.guard));
validation.set_source((&self.state, &error_message).for_each_cloned({
let mut f = self.generate_validation(dynamic);
@ -3453,7 +3527,7 @@ impl WhenValidation<'_> {
let validation = Dynamic::new(Validation::None);
let mut map_to_message = Validations::map_to_message(move |value| check(value));
let error_message =
dynamic.map_each_generational(move |generational| map_to_message(generational));
dynamic.map_each_generational(move |generational| map_to_message(&generational.guard));
let mut f = self.validations.generate_validation(dynamic);
let not = self.not;

View file

@ -5,7 +5,7 @@ use std::clone::Clone;
use std::fmt::{self, Debug};
use std::ops::{ControlFlow, Deref, DerefMut};
use std::sync::atomic::{self, AtomicU64};
use std::sync::{Arc, Mutex, MutexGuard};
use std::sync::Arc;
use std::{slice, vec};
use alot::LotId;
@ -15,6 +15,7 @@ use intentional::Assert;
use kludgine::app::winit::event::{Ime, MouseButton, MouseScrollDelta, TouchPhase};
use kludgine::app::winit::window::CursorIcon;
use kludgine::Color;
use parking_lot::{Mutex, MutexGuard};
use crate::app::{Application, Open, PendingApp, Run};
use crate::context::sealed::Trackable as _;
@ -35,7 +36,6 @@ use crate::styles::{
IntoDynamicComponentValue, Styles, ThemePair, VisualOrder,
};
use crate::tree::{Tree, WeakTree};
use crate::utils::IgnorePoison;
use crate::value::{Dynamic, Generation, IntoDynamic, IntoValue, Validation, Value};
use crate::widgets::checkbox::{Checkable, CheckboxState};
use crate::widgets::layers::{OverlayLayer, Tooltipped};
@ -1404,8 +1404,8 @@ pub struct WidgetInstance {
impl Debug for WidgetInstance {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.data.widget.try_lock() {
Ok(widget) => widget.summarize(f),
Err(_) => f.debug_struct("WidgetInstance").finish_non_exhaustive(),
Some(widget) => widget.summarize(f),
None => f.debug_struct("WidgetInstance").finish_non_exhaustive(),
}
}
}
@ -1539,7 +1539,7 @@ impl WidgetInstance {
/// occur due to other widget locks being held.
#[must_use]
pub fn lock(&self) -> WidgetGuard<'_> {
WidgetGuard(self.data.widget.lock().ignore_poison())
WidgetGuard(self.data.widget.lock())
}
/// Returns the id of the widget that should receive focus after this

View file

@ -1,7 +1,7 @@
//! Widgets that stack in the Z-direction.
use std::fmt::{self, Debug};
use std::sync::{Arc, Mutex};
use std::sync::Arc;
use std::time::Duration;
use alot::{LotId, OrderedLots};
@ -9,11 +9,11 @@ use cushy::widget::{RootBehavior, WidgetInstance};
use figures::units::{Lp, Px, UPx};
use figures::{IntoSigned, IntoUnsigned, Point, Rect, Size, Zero};
use intentional::Assert;
use parking_lot::Mutex;
use crate::animation::easings::EaseOutQuadradic;
use crate::animation::{AnimationHandle, AnimationTarget, IntoAnimate, Spawn, ZeroToOne};
use crate::context::{AsEventContext, EventContext, GraphicsContext, LayoutContext, Trackable};
use crate::utils::IgnorePoison;
use crate::value::{Destination, Dynamic, DynamicGuard, IntoValue, Source, Value};
use crate::widget::{
Callback, MakeWidget, MountedChildren, MountedWidget, Widget, WidgetId, WidgetList, WidgetRef,
@ -641,7 +641,7 @@ struct OverlayLayout {
impl Drop for OverlayLayout {
fn drop(&mut self) {
if let Some(on_dismiss) = &self.on_dismiss {
let mut on_dismiss = on_dismiss.lock().ignore_poison();
let mut on_dismiss = on_dismiss.lock();
on_dismiss.invoke(());
}
}

View file

@ -144,9 +144,7 @@ where
fn hover(&mut self, local: Point<Px>, context: &mut EventContext<'_>) -> Option<CursorIcon> {
if let Some(tick) = &self.tick {
let Some(size) = context.last_layout().map(|rect| rect.size) else {
return None;
};
let size = context.last_layout().map(|rect| rect.size)?;
let world =
tilemap::translate_coordinates(local, context.kludgine.scale(), self.zoom, size);

View file

@ -11,7 +11,7 @@ use std::num::{NonZeroU32, TryFromIntError};
use std::ops::{Deref, DerefMut, Not};
use std::path::Path;
use std::string::ToString;
use std::sync::{mpsc, Arc, Mutex, MutexGuard, OnceLock};
use std::sync::{mpsc, Arc, OnceLock};
use std::time::{Duration, Instant};
use ahash::AHashMap;
@ -37,6 +37,7 @@ use kludgine::drawing::Drawing;
use kludgine::shapes::Shape;
use kludgine::wgpu::{self, CompositeAlphaMode, COPY_BYTES_PER_ROW_ALIGNMENT};
use kludgine::{Color, DrawableExt, Kludgine, KludgineId, Origin, Texture};
use parking_lot::{Mutex, MutexGuard};
use tracing::Level;
use unicode_segmentation::UnicodeSegmentation;
@ -2216,11 +2217,7 @@ impl InnerWindowHandle {
if let Some(handle) = pending.handle.get() {
let _result = handle.send(message);
} else {
pending
.commands
.lock()
.expect("lock poisoned")
.push(message);
pending.commands.lock().push(message);
}
}
InnerWindowHandle::Known(handle) => {
@ -2287,7 +2284,7 @@ impl PendingWindow {
let initialized = pending.handle.set(handle.clone());
assert!(initialized.is_ok());
for command in pending.commands.lock().expect("poisoned").drain(..) {
for command in pending.commands.lock().drain(..) {
let _result = handle.send(command);
}
@ -3170,7 +3167,7 @@ impl Capture {
slice.map_async(wgpu::MapMode::Read, {
let map_result = map_result.clone();
move |result| {
*map_result.lock().assert("thread panicked") = Some(result);
*map_result.lock() = Some(result);
}
});
@ -3179,7 +3176,7 @@ impl Capture {
loop {
device.poll(wgpu::Maintain::Poll);
let mut result = map_result.lock().assert("thread panicked");
let mut result = map_result.lock();
if let Some(result) = result.take() {
result?;
break;