From 6f9062ee533f931500f5aeeae82b1efbe8e26b30 Mon Sep 17 00:00:00 2001 From: valadaptive <79560998+valadaptive@users.noreply.github.com> Date: Sun, 5 May 2024 06:22:48 -0400 Subject: [PATCH] Fix AsyncFileDialog blocking on Windows (#191) ThreadFuture would send a mutex to the thread it spawned, which would then immediately lock that mutex to pass it into whichever blocking callback wanted to write to the data inside it. Meanwhile, calling `poll` on that ThreadFuture would *also* lock that mutex, blocking until the spawned thread finished running and hence defeating the entire purpose of using a future, and possibly even causing the *spawned* thread to block instead if `poll` was called fast enough, causing a deadlock. This is fixed by separating the `data` and `waker` into two separate mutexes; calling `poll` always sets the `waker` but doesn't lock the mutex for `data`. --- CHANGELOG.md | 1 + src/backend/win_cid/thread_future.rs | 51 ++++++++++++++++------------ 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ff5367..5d3470d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased - Move from `objc` crates to `objc2` crates. +- Fix `AsyncFileDialog` blocking the executor on Windows (#191) ## 0.14.0 - i18n for GTK and XDG Portal diff --git a/src/backend/win_cid/thread_future.rs b/src/backend/win_cid/thread_future.rs index 6399ff5..2c1f05d 100644 --- a/src/backend/win_cid/thread_future.rs +++ b/src/backend/win_cid/thread_future.rs @@ -1,36 +1,32 @@ use std::pin::Pin; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, TryLockError}; use std::task::{Context, Poll, Waker}; struct FutureState { - waker: Option, - data: Option, + waker: Mutex>, + data: Mutex>, } -unsafe impl Send for FutureState {} - pub struct ThreadFuture { - state: Arc>>, + state: Arc>, } unsafe impl Send for ThreadFuture {} -impl ThreadFuture { +impl ThreadFuture { pub fn new) + Send + 'static>(f: F) -> Self { - let state = Arc::new(Mutex::new(FutureState { - waker: None, - data: None, - })); + let state = Arc::new(FutureState { + waker: Mutex::new(None), + data: Mutex::new(None), + }); { let state = state.clone(); std::thread::spawn(move || { - let mut state = state.lock().unwrap(); - - f(&mut state.data); + f(&mut state.data.lock().unwrap()); - if let Some(waker) = state.waker.take() { + if let Some(waker) = state.waker.lock().unwrap().take() { waker.wake(); } }); @@ -44,13 +40,26 @@ impl std::future::Future for ThreadFuture { type Output = R; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let mut state = self.state.lock().unwrap(); + let state = &self.state; + let data = state.data.try_lock(); - if state.data.is_some() { - Poll::Ready(state.data.take().unwrap()) - } else { - state.waker = Some(cx.waker().clone()); - Poll::Pending + match data { + Ok(mut data) => { + match data.take() { + Some(data) => Poll::Ready(data), + None => { + *state.waker.lock().unwrap() = Some(cx.waker().clone()); + Poll::Pending + } + } + } + Err(TryLockError::Poisoned(err)) => { + panic!("{}", err); + } + Err(TryLockError::WouldBlock) => { + *state.waker.lock().unwrap() = Some(cx.waker().clone()); + Poll::Pending + } } } }