Switched to external locking in Wgpu, introducing the MightLock side effect.

`side_effect` macro now supports async functions.

zsw-side-effect-macros/src/lib.rs

@@ -11,35 +11,46 @@ pub fn side_effect(attr: TokenStream, item: TokenStream) -> TokenStream {
 	let effects = syn::parse_macro_input!(attr as syn::Type);
 
 	// Then parse the function
-	let func = syn::parse_macro_input!(item as syn::ItemFn);
+	let mut func = syn::parse_macro_input!(item as syn::ItemFn);
 
-	// Create the outer function by wrapping the output type
-	let outer_func = {
-		let mut outer_func = func.clone();
-		// Wrap the return type
+
+	// Wrap the return type
+	func.sig.output = {
+		// Get the `->` and return type
 		let (r_arrow, return_ty) = match func.sig.output {
 			syn::ReturnType::Default =>
 				proc_macro_error::abort!(func.sig.output, "Cannot use an empty output (yet), add `-> ()`"),
 			syn::ReturnType::Type(r_arrow, ty) => (r_arrow, ty),
 		};
-		outer_func.sig.output = syn::ReturnType::Type(
+
+		// Then wrap them in out side effect
+		syn::ReturnType::Type(
 			r_arrow,
 			syn::parse_quote!(::zsw_util::WithSideEffect<#return_ty, (#effects)>),
-		);
+		)
+	};
 
-		// Wrap the body
-		// TODO: Deal with async, unsafe and what not.
-		let inner_fn_body = func.block;
-		outer_func.block = syn::parse_quote! {{
-			let mut __inner_fn = move || { #inner_fn_body };
-			::zsw_util::WithSideEffect::new(__inner_fn())
-		}};
+	// Wrap the body
+	// TODO: Deal with async, unsafe and what not.
+	func.block = {
+		let fn_body = func.block;
 
-		outer_func
+		// Check if we need to wrap async
+		let wrapped_body: syn::Expr = match func.sig.asyncness.is_some() {
+			true => syn::parse_quote! { move || async move { #fn_body } },
+			false => syn::parse_quote! { move || { #fn_body } },
+		};
+		let output_expr: syn::Expr = match func.sig.asyncness.is_some() {
+			true => syn::parse_quote! { ::zsw_util::WithSideEffect::new((#wrapped_body)().await) },
+			false => syn::parse_quote! { ::zsw_util::WithSideEffect::new((#wrapped_body)()) },
+		};
+
+
+		syn::parse_quote! { {#output_expr} }
 	};
 
 	quote::quote! {
-		#outer_func
+		#func
 	}
 	.into()
 }

zsw-util/src/lib.rs

@@ -1,7 +1,14 @@
 //! Utility
 
 // Features
-#![feature(decl_macro, generator_trait, generators, scoped_threads, mixed_integer_ops)]
+#![feature(
+	decl_macro,
+	generator_trait,
+	generators,
+	scoped_threads,
+	mixed_integer_ops,
+	never_type
+)]
 // Lints
 #![warn(
 	clippy::pedantic,
@@ -61,7 +68,7 @@ pub use {
 	display_wrapper::DisplayWrapper,
 	rect::Rect,
 	scan_dir::dir_files_iter,
-	side_effect::{extse, MightBlock, SideEffect, WithSideEffect},
+	side_effect::{extse, MightBlock, MightLock, SideEffect, WithSideEffect},
 	thread::{FutureRunner, ThreadSpawner},
 };
 

zsw-util/src/side_effect.rs

@@ -82,6 +82,12 @@ pub struct MightBlock;
 
 impl SideEffect for MightBlock {}
 
+/// Side effect to indicate a function might lock locks `Locks`.
+#[derive(Clone, Copy, Debug)]
+pub struct MightLock<Locks>(PhantomData<Locks>);
+
+impl<Locks> SideEffect for MightLock<Locks> {}
+
 /// Trait to check if two types are equal
 // TODO: Possibly make this sealed?
 pub trait Eq<T> {}

zsw-wgpu/src/lib.rs

@@ -60,13 +60,13 @@
 use {
 	anyhow::Context,
 	crossbeam::atomic::AtomicCell,
-	parking_lot::Mutex,
+	parking_lot::{Mutex, MutexGuard},
 	pollster::FutureExt,
 	std::marker::PhantomData,
 	wgpu::TextureFormat,
 	winit::{dpi::PhysicalSize, window::Window},
 	zsw_side_effect_macros::side_effect,
-	zsw_util::{extse::ParkingLotMutexSe, MightBlock},
+	zsw_util::{extse::ParkingLotMutexSe, MightBlock, MightLock},
 };
 
 /// Surface
@@ -176,16 +176,28 @@ impl<'window> Wgpu<'window> {
 		&self.queue
 	}
 
+	/// Creates a surface lock
+	///
+	/// # Blocking
+	/// Will block until any existing surface locks are dropped
+	#[side_effect(MightLock<SurfaceLock>)]
+	pub fn lock_surface(&self) -> SurfaceLock<'window, '_> {
+		// DEADLOCK: Caller is responsible to ensure we don't deadlock
+		//           We don't lock it outside of this method
+		SurfaceLock {
+			guard: self.surface.lock_se().allow::<MightBlock>(),
+			wgpu:  self as *const _,
+		}
+	}
+
 	/// Returns the current surface's size
 	///
 	/// # Warning
 	/// This surface size might change at any time, so you shouldn't
 	/// use it on `wgpu` operations that might panic on wrong surface
 	/// sizes.
-	pub fn surface_size(&self) -> PhysicalSize<u32> {
-		// DEADLOCK: We ensure this lock can't deadlock by not blocking
-		//           while locked.
-		self.surface.lock_se().allow::<MightBlock>().size
+	pub fn surface_size(&self, surface_lock: &SurfaceLock) -> PhysicalSize<u32> {
+		surface_lock.get(self).size
 	}
 
 	/// Returns the surface texture format
@@ -207,10 +219,6 @@ impl<'window> Wgpu<'window> {
 
 	/// Renders a frame using `f`
 	///
-	/// # Blocking
-	/// Deadlocks if `f` blocks.
-	/// Deadlocks if called recursively.
-	///
 	/// # Callback
 	/// Callback `f` receives the command encoder and the surface texture / size. This allows you to
 	/// start render passes to the surface texture.
@@ -219,18 +227,13 @@ impl<'window> Wgpu<'window> {
 	///
 	/// If any resize is queued, it will be executed *before* the frame starts, so the frame will start
 	/// with the new size.
-	#[side_effect(MightBlock)]
+	// TODO: Remove size from passed parameters
 	pub fn render(
 		&self,
+		surface_lock: &mut SurfaceLock<'window, '_>,
 		f: impl FnOnce(&mut wgpu::CommandEncoder, &wgpu::TextureView, PhysicalSize<u32>) -> Result<(), anyhow::Error>,
 	) -> Result<(), anyhow::Error> {
-		// Note: We want to keep the surface locked until the end of the
-		//       method to prevent any possible changes from another thread
-		//       mid-frame, which could cause panics in `wgpu` validation.
-		// DEADLOCK: We ensure this lock can't deadlock by not blocking
-		//           while locked.
-		//           Caller ensures `f` doesn't block.
-		let mut surface = self.surface.lock_se().allow::<MightBlock>();
+		let surface = surface_lock.get_mut(self);
 
 		// Check for resizes
 		if let Some(size) = self.queued_resize.take() {
@@ -271,6 +274,38 @@ impl<'window> Wgpu<'window> {
 	}
 }
 
+/// Surface lock
+#[derive(Debug)]
+pub struct SurfaceLock<'window, 'a> {
+	/// Guard
+	guard: MutexGuard<'a, Surface>,
+
+	/// `Wgpu` pointer
+	// Note: This is just to ensure caller only passes a
+	//       lock that came from the same instance
+	wgpu: *const Wgpu<'window>,
+}
+
+impl<'window, 'a> SurfaceLock<'window, 'a> {
+	/// Returns the guard after ensuring the correct `wgpu` instance was passed
+	pub fn get(&self, wgpu: &Wgpu<'window>) -> &Surface {
+		self.assert_source(wgpu);
+		&self.guard
+	}
+
+	/// Returns the guard mutable after ensuring the correct `wgpu` instance was passed
+	pub fn get_mut(&mut self, wgpu: &Wgpu<'window>) -> &mut Surface {
+		self.assert_source(wgpu);
+		&mut self.guard
+	}
+
+	/// Asserts that the correct `wgpu` instance was passed
+	fn assert_source(&self, wgpu: &Wgpu<'window>) {
+		assert_eq!(self.wgpu, wgpu, "Wrong `wgpu` instance was passed to method");
+	}
+}
+
+
 /// Configures the window surface and returns the preferred surface texture format
 fn configure_window_surface(
 	window: &Window,

zsw/src/app.rs

@@ -31,7 +31,7 @@ use {
 	zsw_panels::Panels,
 	zsw_playlist::Playlist,
 	zsw_profiles::Profiles,
-	zsw_util::{FutureRunner, Rect},
+	zsw_util::{FutureRunner, MightLock, Rect, WithSideEffect},
 	zsw_wgpu::Wgpu,
 };
 
@@ -83,6 +83,10 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> {
 	// Start all threads and then wait in the main thread for events
 	// Note: The outer result of `scope` can't be `Err` due to a panic in
 	//       another thread, since we manually join all threads at the end.
+	// DEADLOCK: We ensure all threads lock each lock in the same order,
+	//           and that we don't lock them.
+	//           All threads ensure they will eventually release any lock
+	//           they obtain.
 	thread::scope(|s| {
 		// Create the thread spawner
 		let mut thread_spawner = zsw_util::ThreadSpawner::new(s);
@@ -118,18 +122,20 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> {
 		}
 
 		// Spawn the settings window thread
+		// DEADLOCK: See above
 		thread_spawner.spawn("Settings window", || {
 			settings_window_runner
 				.run(settings_window.run(&wgpu, &egui, &window, &panels, &playlist, &profiles))
+				.map::<!, _>(WithSideEffect::allow::<MightLock<zsw_wgpu::SurfaceLock>>)
 				.into_err();
 		})?;
 
 		// Spawn the renderer thread
-		// DEADLOCK: We call `Renderer::stop` at the end
-		//           We make sure `SettingsWindow::run` runs eventually
+		// DEADLOCK: See above
 		thread_spawner.spawn("Renderer", || {
 			renderer_runner
 				.run(renderer.run(&window, &wgpu, &panels, &egui, &image_loader, &settings_window))
+				.map::<!, _>(WithSideEffect::allow::<MightLock<zsw_wgpu::SurfaceLock>>)
 				.into_err();
 		})?;
 

zsw/src/app/renderer.rs

@@ -12,7 +12,8 @@ use {
 	zsw_egui::Egui,
 	zsw_img::ImageLoader,
 	zsw_panels::Panels,
-	zsw_util::MightBlock,
+	zsw_side_effect_macros::side_effect,
+	zsw_util::{MightBlock, MightLock},
 	zsw_wgpu::Wgpu,
 };
 
@@ -26,10 +27,14 @@ impl Renderer {
 	}
 
 	/// Runs the renderer
-	pub async fn run(
+	///
+	/// # Locking
+	/// Locks the `zsw_wgpu::SurfaceLock` lock on `wgpu`
+	#[side_effect(MightLock<zsw_wgpu::SurfaceLock<'window, 'wgpu>>)]
+	pub async fn run<'window, 'wgpu>(
 		&self,
 		window: &Window,
-		wgpu: &Wgpu<'_>,
+		wgpu: &'wgpu Wgpu<'window>,
 		panels: &Panels,
 		egui: &Egui,
 		image_loader: &ImageLoader,
@@ -48,7 +53,11 @@ impl Renderer {
 			};
 
 			// Render
-			match Self::render(window, wgpu, panels, egui, settings_window).await {
+			// DEADLOCK: Caller ensures we can lock it
+			match Self::render(window, wgpu, panels, egui, settings_window)
+				.await
+				.allow::<MightLock<zsw_wgpu::SurfaceLock>>()
+			{
 				Ok(()) => (),
 				Err(err) => log::warn!("Unable to render: {err:?}"),
 			};
@@ -69,20 +78,31 @@ impl Renderer {
 	}
 
 	/// Renders
-	async fn render(
+	///
+	/// # Locking
+	/// Locks the `zsw_wgpu::SurfaceLock` lock on `wgpu`
+	#[side_effect(MightLock<zsw_wgpu::SurfaceLock<'window, 'wgpu>>)]
+	async fn render<'window, 'wgpu>(
 		window: &Window,
-		wgpu: &Wgpu<'_>,
+		wgpu: &'wgpu Wgpu<'window>,
 		panels: &Panels,
 		egui: &Egui,
 		settings_window: &SettingsWindow,
 	) -> Result<(), anyhow::Error> {
 		// Get the egui render results
-		let paint_jobs = settings_window.paint_jobs().await;
+		// DEADLOCK: We don't hold the `wgpu::SurfaceLock` lock from `wgpu`.
+		//           Caller ensures we can lock it.
+		let paint_jobs = settings_window
+			.paint_jobs()
+			.await
+			.allow::<MightLock<zsw_wgpu::SurfaceLock>>();
+
+		// Lock the wgpu surface
+		// DEADLOCK: Caller ensures we can lock it
+		let mut surface_lock = wgpu.lock_surface().allow::<MightLock<zsw_wgpu::SurfaceLock>>();
 
 		// Then render
-		// DEADLOCK: We ensure we don't block within [`Wgpu::render`].
-		//           We also ensure we don't call it recursively.
-		wgpu.render(|encoder, surface_view, surface_size| {
+		wgpu.render(&mut surface_lock, |encoder, surface_view, surface_size| {
 			// Render the panels
 			panels
 				.render(wgpu.queue(), encoder, surface_view, surface_size)
@@ -114,8 +134,5 @@ impl Renderer {
 
 			Ok(())
 		})
-		.allow::<MightBlock>()?;
-
-		Ok(())
 	}
 }

zsw/src/app/settings_window.rs

@@ -18,7 +18,8 @@ use {
 	zsw_panels::{Panel, PanelState, Panels},
 	zsw_playlist::{Playlist, PlaylistImage},
 	zsw_profiles::{Profile, Profiles},
-	zsw_util::{MightBlock, Rect},
+	zsw_side_effect_macros::side_effect,
+	zsw_util::{MightBlock, MightLock, Rect},
 	zsw_wgpu::Wgpu,
 };
 
@@ -68,6 +69,10 @@ impl SettingsWindow {
 	}
 
 	/// Runs the setting window
+	///
+	/// # Locking
+	/// Locks the `zsw_wgpu::SurfaceLock` lock on `wgpu`
+	#[side_effect(MightLock<zsw_wgpu::SurfaceLock<'_, '_>>)]
 	pub async fn run(
 		&self,
 		wgpu: &Wgpu<'_>,
@@ -78,12 +83,20 @@ impl SettingsWindow {
 		profiles: &Profiles,
 	) -> ! {
 		// Create the inner data
+		// DEADLOCK: Caller ensures we can lock it
 		// TODO: Check if it's fine to call `wgpu.surface_size`
-		let mut inner = Inner::new(wgpu.surface_size());
+		let mut inner = {
+			let surface_lock = wgpu.lock_surface().allow::<MightLock<zsw_wgpu::SurfaceLock>>();
+			Inner::new(wgpu.surface_size(&surface_lock))
+		};
 
 		loop {
 			// Get the surface size
-			let surface_size = wgpu.surface_size();
+			// DEADLOCK: Caller ensures we can lock it
+			let surface_size = {
+				let surface_lock = wgpu.lock_surface().allow::<MightLock<zsw_wgpu::SurfaceLock>>();
+				wgpu.surface_size(&surface_lock)
+			};
 
 			// Draw egui
 			let res = egui.draw(window, |ctx, frame| {
@@ -108,6 +121,12 @@ impl SettingsWindow {
 	}
 
 	/// Retrieves the paint jobs for the next frame
+	///
+	/// # Locking
+	/// Locks the `zsw_wgpu::SurfaceLock` lock on `wgpu`
+	// Note: Doesn't literally lock it, but the other side of the channel
+	//       needs to lock it in order to progress, so it's equivalent
+	#[side_effect(MightLock<zsw_wgpu::SurfaceLock<'_, '_>>)]
 	pub async fn paint_jobs(&self) -> Vec<egui::epaint::ClippedMesh> {
 		// Note: This can't return an `Err` because `self` owns a sender
 		self.paint_jobs_rx.recv().await.expect("Paint jobs sender was closed")