[refactor] Create helper struct Mutex (with double-lock detection)

This commit is contained in:
Emil Ernerfeldt 2020-10-14 00:29:11 +02:00
parent c2a0705c6f
commit 570860ec5f
7 changed files with 82 additions and 41 deletions

View file

@ -3,12 +3,14 @@ use std::sync::{
Arc, Arc,
}; };
use { use ahash::AHashMap;
ahash::AHashMap,
parking_lot::{Mutex, MutexGuard},
};
use crate::{animation_manager::AnimationManager, paint::*, *}; use crate::{
animation_manager::AnimationManager,
mutex::{Mutex, MutexGuard},
paint::*,
*,
};
#[derive(Clone, Copy, Default)] #[derive(Clone, Copy, Default)]
struct PaintStats { struct PaintStats {
@ -60,15 +62,15 @@ pub struct Context {
impl Clone for Context { impl Clone for Context {
fn clone(&self) -> Self { fn clone(&self) -> Self {
Context { Context {
options: Mutex::new(lock(&self.options, "options").clone()), options: self.options.clone(),
fonts: self.fonts.clone(), fonts: self.fonts.clone(),
memory: self.memory.clone(), memory: self.memory.clone(),
animation_manager: self.animation_manager.clone(), animation_manager: self.animation_manager.clone(),
input: self.input.clone(), input: self.input.clone(),
graphics: Mutex::new(self.graphics.lock().clone()), graphics: self.graphics.clone(),
output: Mutex::new(self.output.lock().clone()), output: self.output.clone(),
used_ids: Mutex::new(self.used_ids.lock().clone()), used_ids: self.used_ids.clone(),
paint_stats: Mutex::new(*self.paint_stats.lock()), paint_stats: self.paint_stats.clone(),
repaint_requests: self.repaint_requests.load(SeqCst).into(), repaint_requests: self.repaint_requests.load(SeqCst).into(),
} }
} }
@ -84,15 +86,15 @@ impl Context {
} }
pub fn memory(&self) -> MutexGuard<'_, Memory> { pub fn memory(&self) -> MutexGuard<'_, Memory> {
lock(&self.memory, "memory") self.memory.lock()
} }
pub fn graphics(&self) -> MutexGuard<'_, GraphicLayers> { pub fn graphics(&self) -> MutexGuard<'_, GraphicLayers> {
lock(&self.graphics, "graphics") self.graphics.lock()
} }
pub fn output(&self) -> MutexGuard<'_, Output> { pub fn output(&self) -> MutexGuard<'_, Output> {
lock(&self.output, "output") self.output.lock()
} }
/// Call this if there is need to repaint the UI, i.e. if you are showing an animation. /// Call this if there is need to repaint the UI, i.e. if you are showing an animation.
@ -127,15 +129,15 @@ impl Context {
/// Will become active at the start of the next frame. /// Will become active at the start of the next frame.
/// `pixels_per_point` will be ignored (overwritten at start of each frame with the contents of input) /// `pixels_per_point` will be ignored (overwritten at start of each frame with the contents of input)
pub fn set_fonts(&self, font_definitions: FontDefinitions) { pub fn set_fonts(&self, font_definitions: FontDefinitions) {
lock(&self.options, "options").font_definitions = font_definitions; self.options.lock().font_definitions = font_definitions;
} }
pub fn style(&self) -> Arc<Style> { pub fn style(&self) -> Arc<Style> {
lock(&self.options, "options").style.clone() self.options.lock().style.clone()
} }
pub fn set_style(&self, style: impl Into<Arc<Style>>) { pub fn set_style(&self, style: impl Into<Arc<Style>>) {
lock(&self.options, "options").style = style.into(); self.options.lock().style = style.into();
} }
pub fn pixels_per_point(&self) -> f32 { pub fn pixels_per_point(&self) -> f32 {
@ -183,7 +185,7 @@ impl Context {
self.used_ids.lock().clear(); self.used_ids.lock().clear();
self.input = std::mem::take(&mut self.input).begin_frame(new_raw_input); self.input = std::mem::take(&mut self.input).begin_frame(new_raw_input);
let mut font_definitions = lock(&self.options, "options").font_definitions.clone(); let mut font_definitions = self.options.lock().font_definitions.clone();
font_definitions.pixels_per_point = self.input.pixels_per_point(); font_definitions.pixels_per_point = self.input.pixels_per_point();
let same_as_current = match &self.fonts { let same_as_current = match &self.fonts {
None => false, None => false,
@ -220,7 +222,7 @@ impl Context {
} }
fn paint(&self) -> PaintJobs { fn paint(&self) -> PaintJobs {
let mut paint_options = lock(&self.options, "options").paint_options; let mut paint_options = self.options.lock().paint_options;
paint_options.aa_size = 1.0 / self.pixels_per_point(); paint_options.aa_size = 1.0 / self.pixels_per_point();
let paint_commands = self.drain_paint_lists(); let paint_commands = self.drain_paint_lists();
let num_primitives = paint_commands.len(); let num_primitives = paint_commands.len();
@ -532,9 +534,9 @@ impl Context {
CollapsingHeader::new("Painting") CollapsingHeader::new("Painting")
.default_open(true) .default_open(true)
.show(ui, |ui| { .show(ui, |ui| {
let mut paint_options = lock(&self.options, "options").paint_options; let mut paint_options = self.options.lock().paint_options;
paint_options.ui(ui); paint_options.ui(ui);
lock(&self.options, "options").paint_options = paint_options; self.options.lock().paint_options = paint_options;
}); });
} }
@ -651,17 +653,3 @@ impl PaintStats {
ui.label(format!("Triangles: {}", self.num_triangles)); ui.label(format!("Triangles: {}", self.num_triangles));
} }
} }
#[cfg(debug_assertions)]
fn lock<'m, T>(mutex: &'m Mutex<T>, what: &'static str) -> MutexGuard<'m, T> {
// TODO: detect if we are trying to lock the same mutex *from the same thread*.
// at the moment we just panic on any double-locking of a mutex (so no multithreaded support in debug builds)
mutex
.try_lock()
.unwrap_or_else(|| panic!("The Mutex for {} is already locked. Probably a bug", what))
}
#[cfg(not(debug_assertions))]
fn lock<'m, T>(mutex: &'m Mutex<T>, _what: &'static str) -> MutexGuard<'m, T> {
mutex.lock()
}

View file

@ -60,6 +60,7 @@ mod layout;
pub mod math; pub mod math;
mod memory; mod memory;
pub mod menu; pub mod menu;
pub mod mutex;
pub mod paint; pub mod paint;
mod painter; mod painter;
mod style; mod style;

49
egui/src/mutex.rs Normal file
View file

@ -0,0 +1,49 @@
//! Helper module for a Mutex that
//! detects double-locking on the same thread in debug mode.
// TODO: feature-flag for `parking_lot` vs `std::sync::Mutex`.
pub use parking_lot::MutexGuard;
#[derive(Default)]
pub struct Mutex<T>(parking_lot::Mutex<T>);
impl<T> Mutex<T> {
#[inline(always)]
pub fn new(val: T) -> Self {
Self(parking_lot::Mutex::new(val))
}
#[cfg(debug_assertions)]
pub fn lock(&self) -> MutexGuard<'_, T> {
// TODO: detect if we are trying to lock the same mutex from the same thread (bad)
// vs locking it from another thread (fine).
// At the moment we just panic on any double-locking of a mutex (so no multithreaded support in debug builds)
self.0
.try_lock()
.expect("The Mutex is already locked. Probably a bug")
}
#[inline(always)]
#[cfg(not(debug_assertions))]
pub fn lock(&self) -> MutexGuard<'_, T> {
self.0.lock()
}
}
impl<T> Clone for Mutex<T>
where
T: Clone,
{
fn clone(&self) -> Self {
Self::new(self.lock().clone())
}
}
// impl<T> PartialEq for Mutex<T>
// where
// T: PartialEq,
// {
// fn eq(&self, other: &Self) -> bool {
// std::ptr::eq(self, other) || self.lock().eq(&other.lock())
// }
// }

View file

@ -2,11 +2,14 @@ use std::sync::Arc;
use { use {
ahash::AHashMap, ahash::AHashMap,
parking_lot::{Mutex, RwLock}, parking_lot::RwLock,
rusttype::{point, Scale}, rusttype::{point, Scale},
}; };
use crate::math::{vec2, Vec2}; use crate::{
math::{vec2, Vec2},
mutex::Mutex,
};
use super::texture_atlas::TextureAtlas; use super::texture_atlas::TextureAtlas;

View file

@ -4,7 +4,7 @@ use std::{
sync::Arc, sync::Arc,
}; };
use parking_lot::Mutex; use crate::mutex::Mutex;
use super::{ use super::{
font::Font, font::Font,

View file

@ -2,7 +2,7 @@
use std::{hash::Hash, sync::Arc}; use std::{hash::Hash, sync::Arc};
use crate::{color::*, containers::*, layout::*, paint::*, widgets::*, *}; use crate::{color::*, containers::*, layout::*, mutex::MutexGuard, paint::*, widgets::*, *};
/// Represents a region of the screen /// Represents a region of the screen
/// with a type of layout (horizontal or vertical). /// with a type of layout (horizontal or vertical).
@ -154,13 +154,13 @@ impl Ui {
/// The `Memory` of the `Context` associated with the `Ui`. /// The `Memory` of the `Context` associated with the `Ui`.
/// Equivalent to `.ctx().memory()`. /// Equivalent to `.ctx().memory()`.
pub fn memory(&self) -> parking_lot::MutexGuard<'_, Memory> { pub fn memory(&self) -> MutexGuard<'_, Memory> {
self.ctx().memory() self.ctx().memory()
} }
/// The `Output` of the `Context` associated with the `Ui`. /// The `Output` of the `Context` associated with the `Ui`.
/// Equivalent to `.ctx().output()`. /// Equivalent to `.ctx().output()`.
pub fn output(&self) -> parking_lot::MutexGuard<'_, Output> { pub fn output(&self) -> MutexGuard<'_, Output> {
self.ctx().output() self.ctx().output()
} }

View file

@ -6,7 +6,7 @@ pub mod webgl;
pub use backend::*; pub use backend::*;
use parking_lot::Mutex; use egui::mutex::Mutex;
use std::sync::Arc; use std::sync::Arc;
use wasm_bindgen::prelude::*; use wasm_bindgen::prelude::*;