gpui: Gate blocking executor APIs on WebAssembly (#63484)
GPUI's scheduler exposed synchronous blocking APIs on WebAssembly even though the platform implementation could only panic. This became an implicit crash when cancellation dropped a scoped background operation: preserving the borrowed task lifetime called `Scheduler::block`, which panicked with `Cannot block on wasm`. This change removes blocking scheduler and executor APIs from WebAssembly builds, including borrowed background scopes and the separate `pollster::block_on` export. Native behavior remains unchanged. Application shutdown now runs WebAssembly quit handlers asynchronously as best-effort cleanup rather than blocking the browser event-loop thread. Release Notes: - Fixed a GPUI Web crash caused by synchronous executor blocking.
This commit is contained in:
1 parent
ab1c37a4ef
commit
1344a8a846
7 files changed
+146
-114
No files matched your search
+10
-3
@@ -73,7 +73,8 @@ mod test_context;
|
||||
#[cfg(all(target_os = "macos", any(test, feature = "test-support")))]
|
||||
mod visual_test_context;
|
||||
|
||||
/// The duration for which futures returned from [Context::on_app_quit] can run before the application fully quits.
|
||||
/// The duration for which native applications wait for futures returned from
|
||||
/// [Context::on_app_quit] before fully quitting.
|
||||
pub const SHUTDOWN_TIMEOUT: Duration = Duration::from_millis(200);
|
||||
|
||||
/// Temporary(?) wrapper around [`RefCell<App>`] to help us debug any double borrows.
|
||||
@@ -969,8 +970,11 @@ impl App {
|
||||
self.entities.assert_no_new_leaks(snapshot)
|
||||
}
|
||||
|
||||
/// Quit the application gracefully. Handlers registered with [`Context::on_app_quit`]
|
||||
/// will be given `SHUTDOWN_TIMEOUT` to complete before exiting.
|
||||
/// Quit the application gracefully.
|
||||
///
|
||||
/// Native applications give handlers registered with [`Context::on_app_quit`]
|
||||
/// [`SHUTDOWN_TIMEOUT`] to complete. WebAssembly runs them asynchronously as best-effort cleanup
|
||||
/// because its event-loop thread cannot block.
|
||||
pub fn shutdown(&mut self) {
|
||||
let mut futures = Vec::new();
|
||||
|
||||
@@ -984,6 +988,7 @@ impl App {
|
||||
self.quitting = true;
|
||||
|
||||
let futures = futures::future::join_all(futures);
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
if self
|
||||
.foreground_executor
|
||||
.block_with_timeout(SHUTDOWN_TIMEOUT, futures)
|
||||
@@ -991,6 +996,8 @@ impl App {
|
||||
{
|
||||
log::error!("timed out waiting on app_will_quit");
|
||||
}
|
||||
#[cfg(target_family = "wasm")]
|
||||
self.foreground_executor.spawn(futures).detach();
|
||||
|
||||
self.quitting = false;
|
||||
}
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
use crate::{App, PlatformDispatcher, PlatformScheduler};
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
use futures::channel::mpsc;
|
||||
use futures::prelude::*;
|
||||
use gpui_util::{TryFutureExt, TryFutureExtBacktrace};
|
||||
use scheduler::Instant;
|
||||
use scheduler::Scheduler;
|
||||
use std::{future::Future, marker::PhantomData, mem, pin::Pin, rc::Rc, sync::Arc, time::Duration};
|
||||
use std::{future::Future, marker::PhantomData, rc::Rc, sync::Arc, time::Duration};
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
use std::{mem, pin::Pin};
|
||||
|
||||
pub use scheduler::{
|
||||
DedicatedExecutor, FallibleTask, LocalExecutor as SchedulerLocalExecutor, Priority, Task,
|
||||
@@ -138,8 +141,11 @@ impl BackgroundExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
/// Scoped lets you start a number of tasks and waits
|
||||
/// for all of them to complete before returning.
|
||||
/// Runs background tasks that may borrow from their environment and waits for all of them to complete.
|
||||
///
|
||||
/// Dropping the returned future cancels its tasks and synchronously waits for their futures to
|
||||
/// be destroyed before returning.
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub async fn scoped<'scope, F>(&self, scheduler: F)
|
||||
where
|
||||
F: FnOnce(&mut Scope<'scope>),
|
||||
@@ -155,8 +161,12 @@ impl BackgroundExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
/// Scoped lets you start a number of tasks and waits
|
||||
/// for all of them to complete before returning.
|
||||
/// Runs prioritized background tasks that may borrow from their environment and waits for all
|
||||
/// of them to complete.
|
||||
///
|
||||
/// Dropping the returned future cancels its tasks and synchronously waits for their futures to
|
||||
/// be destroyed before returning.
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub async fn scoped_priority<'scope, F>(&self, priority: Priority, scheduler: F)
|
||||
where
|
||||
F: FnOnce(&mut Scope<'scope>),
|
||||
@@ -408,7 +418,7 @@ impl ForegroundExecutor {
|
||||
}
|
||||
|
||||
/// Used by the test harness to run an async test in a synchronous fashion.
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
#[cfg(all(not(target_family = "wasm"), any(test, feature = "test-support")))]
|
||||
#[track_caller]
|
||||
pub fn block_test<R>(&self, future: impl Future<Output = R>) -> R {
|
||||
use std::cell::Cell;
|
||||
@@ -431,11 +441,13 @@ impl ForegroundExecutor {
|
||||
|
||||
/// Block the current thread until the given future resolves.
|
||||
/// Consider using `block_with_timeout` instead.
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub fn block_on<R>(&self, future: impl Future<Output = R>) -> R {
|
||||
self.inner.block_on(future)
|
||||
}
|
||||
|
||||
/// Block the current thread until the given future resolves or the timeout elapses.
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub fn block_with_timeout<R, Fut: Future<Output = R>>(
|
||||
&self,
|
||||
duration: Duration,
|
||||
@@ -456,6 +468,7 @@ impl ForegroundExecutor {
|
||||
}
|
||||
|
||||
/// Scope manages a set of tasks that are enqueued and waited on together. See [`BackgroundExecutor::scoped`].
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub struct Scope<'a> {
|
||||
executor: BackgroundExecutor,
|
||||
priority: Priority,
|
||||
@@ -465,6 +478,7 @@ pub struct Scope<'a> {
|
||||
lifetime: PhantomData<&'a ()>,
|
||||
}
|
||||
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
impl<'a> Scope<'a> {
|
||||
fn new(executor: BackgroundExecutor, priority: Priority) -> Self {
|
||||
let (tx, rx) = mpsc::channel(1);
|
||||
@@ -506,6 +520,7 @@ impl<'a> Scope<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
impl Drop for Scope<'_> {
|
||||
fn drop(&mut self) {
|
||||
self.tx.take().unwrap();
|
||||
|
||||
@@ -166,6 +166,7 @@ pub use util::{FutureExt, Timeout};
|
||||
pub use view::*;
|
||||
pub use window::*;
|
||||
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub use pollster::block_on;
|
||||
|
||||
/// The context trait, allows the different contexts in GPUI to be used
|
||||
|
||||
@@ -66,51 +66,42 @@ impl PlatformScheduler {
|
||||
}
|
||||
|
||||
impl Scheduler for PlatformScheduler {
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
fn block(
|
||||
&self,
|
||||
_session_id: Option<SessionId>,
|
||||
#[cfg_attr(target_family = "wasm", allow(unused_mut))] mut future: Pin<
|
||||
&mut dyn Future<Output = ()>,
|
||||
>,
|
||||
#[cfg_attr(target_family = "wasm", allow(unused_variables))] timeout: Option<Duration>,
|
||||
mut future: Pin<&mut dyn Future<Output = ()>>,
|
||||
timeout: Option<Duration>,
|
||||
) -> bool {
|
||||
#[cfg(target_family = "wasm")]
|
||||
{
|
||||
let _ = (&future, &timeout);
|
||||
panic!("Cannot block on wasm")
|
||||
use waker_fn::waker_fn;
|
||||
let deadline = timeout.map(|t| Instant::now() + t);
|
||||
let parker = parking::Parker::new();
|
||||
let unparker = parker.unparker();
|
||||
let waker = waker_fn(move || {
|
||||
unparker.unpark();
|
||||
});
|
||||
let mut cx = Context::from_waker(&waker);
|
||||
if let Poll::Ready(()) = future.as_mut().poll(&mut cx) {
|
||||
return true;
|
||||
}
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
{
|
||||
use waker_fn::waker_fn;
|
||||
let deadline = timeout.map(|t| Instant::now() + t);
|
||||
let parker = parking::Parker::new();
|
||||
let unparker = parker.unparker();
|
||||
let waker = waker_fn(move || {
|
||||
unparker.unpark();
|
||||
});
|
||||
let mut cx = Context::from_waker(&waker);
|
||||
if let Poll::Ready(()) = future.as_mut().poll(&mut cx) {
|
||||
return true;
|
||||
|
||||
let park_deadline = |deadline: Instant| {
|
||||
// Timer expirations are only delivered every ~15.6 milliseconds by default on Windows.
|
||||
// We increase the resolution during this wait so that short timeouts stay reasonably short.
|
||||
let _timer_guard = self.dispatcher.increase_timer_resolution();
|
||||
parker.park_deadline(deadline)
|
||||
};
|
||||
|
||||
loop {
|
||||
match deadline {
|
||||
Some(deadline) if !park_deadline(deadline) && deadline <= Instant::now() => {
|
||||
return false;
|
||||
}
|
||||
Some(_) => (),
|
||||
None => parker.park(),
|
||||
}
|
||||
|
||||
let park_deadline = |deadline: Instant| {
|
||||
// Timer expirations are only delivered every ~15.6 milliseconds by default on Windows.
|
||||
// We increase the resolution during this wait so that short timeouts stay reasonably short.
|
||||
let _timer_guard = self.dispatcher.increase_timer_resolution();
|
||||
parker.park_deadline(deadline)
|
||||
};
|
||||
|
||||
loop {
|
||||
match deadline {
|
||||
Some(deadline) if !park_deadline(deadline) && deadline <= Instant::now() => {
|
||||
return false;
|
||||
}
|
||||
Some(_) => (),
|
||||
None => parker.park(),
|
||||
}
|
||||
if let Poll::Ready(()) = future.as_mut().poll(&mut cx) {
|
||||
break true;
|
||||
}
|
||||
if let Poll::Ready(()) = future.as_mut().poll(&mut cx) {
|
||||
break true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -106,6 +106,7 @@ impl LocalExecutor {
|
||||
Task(TaskState::Spawned(task))
|
||||
}
|
||||
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub fn block_on<Fut: Future>(&self, future: Fut) -> Fut::Output {
|
||||
use std::cell::Cell;
|
||||
|
||||
@@ -123,6 +124,7 @@ impl LocalExecutor {
|
||||
|
||||
/// Block until the future completes or timeout occurs.
|
||||
/// Returns Ok(output) if completed, Err(future) if timed out.
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
pub fn block_with_timeout<Fut: Future>(
|
||||
&self,
|
||||
timeout: Duration,
|
||||
|
||||
@@ -83,6 +83,7 @@ pub trait Scheduler: Send + Sync {
|
||||
/// Returns `true` if the future completed, `false` if it timed out.
|
||||
/// The future is passed as a pinned mutable reference so the caller
|
||||
/// retains ownership and can continue polling or return it on timeout.
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
fn block(
|
||||
&self,
|
||||
session_id: Option<SessionId>,
|
||||
|
||||
@@ -19,7 +19,6 @@ use std::{
|
||||
future::Future,
|
||||
mem,
|
||||
ops::RangeInclusive,
|
||||
panic::{self, AssertUnwindSafe},
|
||||
pin::Pin,
|
||||
sync::{
|
||||
Arc, Weak,
|
||||
@@ -62,15 +61,15 @@ impl TestScheduler {
|
||||
|
||||
(seed..seed + num_iterations as u64)
|
||||
.map(|seed| {
|
||||
let mut unwind_safe_f = AssertUnwindSafe(&mut f);
|
||||
let mut unwind_safe_f = std::panic::AssertUnwindSafe(&mut f);
|
||||
if interactive {
|
||||
eprintln!("Running seed: {seed}");
|
||||
}
|
||||
match panic::catch_unwind(move || Self::with_seed(seed, &mut *unwind_safe_f)) {
|
||||
match std::panic::catch_unwind(move || Self::with_seed(seed, &mut *unwind_safe_f)) {
|
||||
Ok(result) => result,
|
||||
Err(error) => {
|
||||
eprintln!("\x1b[31mFailing Seed: {seed}\x1b[0m");
|
||||
panic::resume_unwind(error);
|
||||
std::panic::resume_unwind(error);
|
||||
}
|
||||
}
|
||||
})
|
||||
@@ -79,12 +78,86 @@ impl TestScheduler {
|
||||
|
||||
fn with_seed<R>(seed: u64, f: impl AsyncFnOnce(Arc<TestScheduler>) -> R) -> R {
|
||||
let scheduler = Arc::new(TestScheduler::new(TestSchedulerConfig::with_seed(seed)));
|
||||
let output = std::cell::Cell::new(None);
|
||||
let future = f(scheduler.clone());
|
||||
let result = scheduler.foreground().block_on(future);
|
||||
let future = async {
|
||||
output.set(Some(future.await));
|
||||
};
|
||||
let mut future = std::pin::pin!(future);
|
||||
scheduler.block_until(None, future.as_mut(), None);
|
||||
let result = output.take().expect("test future did not complete");
|
||||
scheduler.run(); // Ensure spawned tasks finish up before returning in tests
|
||||
result
|
||||
}
|
||||
|
||||
fn block_until(
|
||||
&self,
|
||||
session_id: Option<SessionId>,
|
||||
mut future: Pin<&mut dyn Future<Output = ()>>,
|
||||
timeout: Option<Duration>,
|
||||
) -> bool {
|
||||
if let Some(session_id) = session_id {
|
||||
self.state.lock().blocked_sessions.push(session_id);
|
||||
}
|
||||
|
||||
let deadline = timeout.map(|timeout| Instant::now() + timeout);
|
||||
let awoken = Arc::new(AtomicBool::new(false));
|
||||
let waker = Box::new(TracingWaker {
|
||||
id: None,
|
||||
awoken: awoken.clone(),
|
||||
thread: self.thread.clone(),
|
||||
state: self.state.clone(),
|
||||
});
|
||||
let waker = unsafe { Waker::new(Box::into_raw(waker) as *const (), &WAKER_VTABLE) };
|
||||
let max_ticks = if timeout.is_some() {
|
||||
self.rng
|
||||
.lock()
|
||||
.random_range(self.state.lock().timeout_ticks.clone())
|
||||
} else {
|
||||
usize::MAX
|
||||
};
|
||||
let mut cx = Context::from_waker(&waker);
|
||||
|
||||
let mut completed = false;
|
||||
for _ in 0..max_ticks {
|
||||
match future.as_mut().poll(&mut cx) {
|
||||
Poll::Ready(()) => {
|
||||
completed = true;
|
||||
break;
|
||||
}
|
||||
Poll::Pending => {}
|
||||
}
|
||||
|
||||
let mut stepped = None;
|
||||
while self.rng.lock().random() {
|
||||
let stepped = stepped.get_or_insert(false);
|
||||
if self.step() {
|
||||
*stepped = true;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let stepped = stepped.unwrap_or(true);
|
||||
let awoken = awoken.swap(false, SeqCst);
|
||||
if !stepped && !awoken {
|
||||
let parking_allowed = self.state.lock().allow_parking;
|
||||
// In deterministic mode (parking forbidden), instantly jump to the next timer.
|
||||
// In non-deterministic mode (parking allowed), let real time pass instead.
|
||||
let advanced_to_timer = !parking_allowed && self.advance_clock_to_next_timer();
|
||||
if !advanced_to_timer && !self.park(deadline) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if session_id.is_some() {
|
||||
self.state.lock().blocked_sessions.pop();
|
||||
}
|
||||
|
||||
completed
|
||||
}
|
||||
|
||||
pub fn new(config: TestSchedulerConfig) -> Self {
|
||||
Self {
|
||||
rng: Arc::new(Mutex::new(StdRng::seed_from_u64(config.seed))),
|
||||
@@ -520,72 +593,14 @@ impl Scheduler for TestScheduler {
|
||||
/// is provided. This is to allow testing a mix of deterministic and
|
||||
/// non-deterministic async behavior, such as when interacting with I/O in
|
||||
/// an otherwise deterministic test.
|
||||
#[cfg(not(target_family = "wasm"))]
|
||||
fn block(
|
||||
&self,
|
||||
session_id: Option<SessionId>,
|
||||
mut future: Pin<&mut dyn Future<Output = ()>>,
|
||||
future: Pin<&mut dyn Future<Output = ()>>,
|
||||
timeout: Option<Duration>,
|
||||
) -> bool {
|
||||
if let Some(session_id) = session_id {
|
||||
self.state.lock().blocked_sessions.push(session_id);
|
||||
}
|
||||
|
||||
let deadline = timeout.map(|timeout| Instant::now() + timeout);
|
||||
let awoken = Arc::new(AtomicBool::new(false));
|
||||
let waker = Box::new(TracingWaker {
|
||||
id: None,
|
||||
awoken: awoken.clone(),
|
||||
thread: self.thread.clone(),
|
||||
state: self.state.clone(),
|
||||
});
|
||||
let waker = unsafe { Waker::new(Box::into_raw(waker) as *const (), &WAKER_VTABLE) };
|
||||
let max_ticks = if timeout.is_some() {
|
||||
self.rng
|
||||
.lock()
|
||||
.random_range(self.state.lock().timeout_ticks.clone())
|
||||
} else {
|
||||
usize::MAX
|
||||
};
|
||||
let mut cx = Context::from_waker(&waker);
|
||||
|
||||
let mut completed = false;
|
||||
for _ in 0..max_ticks {
|
||||
match future.as_mut().poll(&mut cx) {
|
||||
Poll::Ready(()) => {
|
||||
completed = true;
|
||||
break;
|
||||
}
|
||||
Poll::Pending => {}
|
||||
}
|
||||
|
||||
let mut stepped = None;
|
||||
while self.rng.lock().random() {
|
||||
let stepped = stepped.get_or_insert(false);
|
||||
if self.step() {
|
||||
*stepped = true;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let stepped = stepped.unwrap_or(true);
|
||||
let awoken = awoken.swap(false, SeqCst);
|
||||
if !stepped && !awoken {
|
||||
let parking_allowed = self.state.lock().allow_parking;
|
||||
// In deterministic mode (parking forbidden), instantly jump to the next timer.
|
||||
// In non-deterministic mode (parking allowed), let real time pass instead.
|
||||
let advanced_to_timer = !parking_allowed && self.advance_clock_to_next_timer();
|
||||
if !advanced_to_timer && !self.park(deadline) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if session_id.is_some() {
|
||||
self.state.lock().blocked_sessions.pop();
|
||||
}
|
||||
|
||||
completed
|
||||
self.block_until(session_id, future, timeout)
|
||||
}
|
||||
|
||||
fn schedule_local(&self, session_id: SessionId, runnable: Runnable<RunnableMeta>) {
|
||||
|
||||
Reference in new issue
Block a user