Skip to content

Commit

Permalink
Web: async improvements (#3805)
Browse files Browse the repository at this point in the history
- Internal: Fix dropping `Notifier` without sending a result causing `Future`s to never complete. This should never happen anyway, but now we get a panic instead of nothing if we hit a bug.
- Internal: Remove a bunch of `unwrap()`s that aren't required when correctly using `MainThreadMarker`.
- `Window::canvas()` is now able to return a reference instead of an owned value.

Extracted from #3801.
  • Loading branch information
daxpedda authored Jul 23, 2024
1 parent 5ec934b commit ef580b8
Show file tree
Hide file tree
Showing 12 changed files with 236 additions and 217 deletions.
1 change: 1 addition & 0 deletions src/changelog/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ changelog entry.
- Change signature of `EventLoop::run_app`, `EventLoopExtPumpEvents::pump_app_events` and
`EventLoopExtRunOnDemand::run_app_on_demand` to accept a `impl ApplicationHandler` directly,
instead of requiring a `&mut` reference to it.
- On Web, `Window::canvas()` now returns a reference.

### Removed

Expand Down
9 changes: 5 additions & 4 deletions src/platform/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
//! [`WindowEvent::Touch`]: crate::event::WindowEvent::Touch
//! [`Window::set_outer_position()`]: crate::window::Window::set_outer_position

use std::cell::Ref;
use std::error::Error;
use std::fmt::{self, Display, Formatter};
use std::future::Future;
Expand All @@ -67,7 +68,7 @@ pub struct HtmlCanvasElement;
pub trait WindowExtWeb {
/// Only returns the canvas if called from inside the window context (the
/// main thread).
fn canvas(&self) -> Option<HtmlCanvasElement>;
fn canvas(&self) -> Option<Ref<'_, HtmlCanvasElement>>;

/// Returns [`true`] if calling `event.preventDefault()` is enabled.
///
Expand All @@ -87,7 +88,7 @@ pub trait WindowExtWeb {

impl WindowExtWeb for Window {
#[inline]
fn canvas(&self) -> Option<HtmlCanvasElement> {
fn canvas(&self) -> Option<Ref<'_, HtmlCanvasElement>> {
self.window.canvas()
}

Expand Down Expand Up @@ -175,7 +176,7 @@ pub trait EventLoopExtWeb {
not(all(web_platform, target_feature = "exception-handling")),
doc = "[`run_app()`]: EventLoop::run_app()"
)]
/// [^1]: `run_app()` is _not_ available on WASM when the target supports `exception-handling`.
/// [^1]: `run_app()` is _not_ available on Wasm when the target supports `exception-handling`.
fn spawn_app<A: ApplicationHandler + 'static>(self, app: A);

/// Sets the strategy for [`ControlFlow::Poll`].
Expand Down Expand Up @@ -398,7 +399,7 @@ impl fmt::Display for BadAnimation {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Empty => write!(f, "No cursors supplied"),
Self::Animation => write!(f, "A supplied cursor is an animtion"),
Self::Animation => write!(f, "A supplied cursor is an animation"),
}
}
}
Expand Down
28 changes: 13 additions & 15 deletions src/platform_impl/web/async/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ pub struct Dispatcher<T: 'static>(Wrapper<T, Arc<Sender<Closure<T>>>, Closure<T>
struct Closure<T>(Box<dyn FnOnce(&T) + Send>);

impl<T> Dispatcher<T> {
#[track_caller]
pub fn new(main_thread: MainThreadMarker, value: T) -> Option<(Self, DispatchRunner<T>)> {
pub fn new(main_thread: MainThreadMarker, value: T) -> (Self, DispatchRunner<T>) {
let (sender, receiver) = channel::<Closure<T>>();
let sender = Arc::new(sender);
let receiver = Rc::new(receiver);

Wrapper::new(
let wrapper = Wrapper::new(
main_thread,
value,
|value, Closure(closure)| {
Expand All @@ -29,8 +28,7 @@ impl<T> Dispatcher<T> {
move |value| async move {
while let Ok(Closure(closure)) = receiver.next().await {
// SAFETY: The given `Closure` here isn't really `'static`, so we shouldn't
// do anything funny with it here. See
// `Self::queue()`.
// do anything funny with it here. See `Self::queue()`.
closure(value.borrow().as_ref().unwrap())
}
}
Expand All @@ -41,25 +39,25 @@ impl<T> Dispatcher<T> {
// anything funny with it here. See `Self::queue()`.
sender.send(closure).unwrap()
},
)
.map(|wrapper| (Self(wrapper.clone()), DispatchRunner { wrapper, receiver }))
);
(Self(wrapper.clone()), DispatchRunner { wrapper, receiver })
}

pub fn value(&self) -> Option<Ref<'_, T>> {
self.0.value()
pub fn value(&self, main_thread: MainThreadMarker) -> Ref<'_, T> {
self.0.value(main_thread)
}

pub fn dispatch(&self, f: impl 'static + FnOnce(&T) + Send) {
if let Some(value) = self.0.value() {
f(&value)
if let Some(main_thread) = MainThreadMarker::new() {
f(&self.0.value(main_thread))
} else {
self.0.send(Closure(Box::new(f)))
}
}

pub fn queue<R: Send>(&self, f: impl FnOnce(&T) -> R + Send) -> R {
if let Some(value) = self.0.value() {
f(&value)
if let Some(main_thread) = MainThreadMarker::new() {
f(&self.0.value(main_thread))
} else {
let pair = Arc::new((Mutex::new(None), Condvar::new()));
let closure = Box::new({
Expand Down Expand Up @@ -98,13 +96,13 @@ pub struct DispatchRunner<T: 'static> {
}

impl<T> DispatchRunner<T> {
pub fn run(&self) {
pub fn run(&self, main_thread: MainThreadMarker) {
while let Some(Closure(closure)) =
self.receiver.try_recv().expect("should only be closed when `Dispatcher` is dropped")
{
// SAFETY: The given `Closure` here isn't really `'static`, so we shouldn't do anything
// funny with it here. See `Self::queue()`.
closure(&self.wrapper.value().expect("don't call this outside the main thread"))
closure(&self.wrapper.value(main_thread))
}
}
}
25 changes: 14 additions & 11 deletions src/platform_impl/web/async/notifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,28 @@ impl<T: Clone> Notifier<T> {
if self.0.value.set(value).is_err() {
unreachable!("value set before")
}
}

pub fn notified(&self) -> Notified<T> {
Notified(Some(Arc::clone(&self.0)))
}
}

impl<T: Clone> Drop for Notifier<T> {
fn drop(&mut self) {
self.0.queue.close();

while let Ok(waker) = self.0.queue.pop() {
waker.wake()
}
}

pub fn notified(&self) -> Notified<T> {
Notified(Some(Arc::clone(&self.0)))
}
}

#[derive(Clone, Debug)]
pub struct Notified<T: Clone>(Option<Arc<Inner<T>>>);

impl<T: Clone> Future for Notified<T> {
type Output = T;
type Output = Option<T>;

fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
let this = self.0.take().expect("`Receiver` polled after completion");
Expand All @@ -54,14 +58,13 @@ impl<T: Clone> Future for Notified<T> {
}
}

let (Ok(Some(value)) | Err(Some(value))) = Arc::try_unwrap(this)
match Arc::try_unwrap(this)
.map(|mut inner| inner.value.take())
.map_err(|this| this.value.get().cloned())
else {
unreachable!("found no value despite being ready")
};

Poll::Ready(value)
{
Ok(Some(value)) | Err(Some(value)) => Poll::Ready(Some(value)),
_ => Poll::Ready(None),
}
}
}

Expand Down
9 changes: 3 additions & 6 deletions src/platform_impl/web/async/waker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ struct Handler<T> {
struct Sender(Arc<Inner>);

impl<T> WakerSpawner<T> {
#[track_caller]
pub fn new(main_thread: MainThreadMarker, value: T, handler: fn(&T, bool)) -> Option<Self> {
pub fn new(main_thread: MainThreadMarker, value: T, handler: fn(&T, bool)) -> Self {
let inner = Arc::new(Inner {
awoken: AtomicBool::new(false),
waker: AtomicWaker::new(),
Expand All @@ -31,7 +30,7 @@ impl<T> WakerSpawner<T> {

let sender = Sender(Arc::clone(&inner));

let wrapper = Wrapper::new(
Self(Wrapper::new(
main_thread,
handler,
|handler, _| {
Expand Down Expand Up @@ -73,9 +72,7 @@ impl<T> WakerSpawner<T> {
inner.0.awoken.store(true, Ordering::Relaxed);
inner.0.waker.wake();
},
)?;

Some(Self(wrapper))
))
}

pub fn waker(&self) -> Waker<T> {
Expand Down
15 changes: 4 additions & 11 deletions src/platform_impl/web/async/wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ unsafe impl<V> Send for Value<V> {}
unsafe impl<V> Sync for Value<V> {}

impl<V, S: Clone + Send, E> Wrapper<V, S, E> {
#[track_caller]
pub fn new<R: Future<Output = ()>>(
_: MainThreadMarker,
value: V,
handler: fn(&RefCell<Option<V>>, E),
receiver: impl 'static + FnOnce(Arc<RefCell<Option<V>>>) -> R,
sender_data: S,
sender_handler: fn(&S, E),
) -> Option<Self> {
) -> Self {
let value = Arc::new(RefCell::new(Some(value)));

wasm_bindgen_futures::spawn_local({
Expand All @@ -52,12 +51,7 @@ impl<V, S: Clone + Send, E> Wrapper<V, S, E> {
}
});

Some(Self {
value: Value { value, local: PhantomData },
handler,
sender_data,
sender_handler,
})
Self { value: Value { value, local: PhantomData }, handler, sender_data, sender_handler }
}

pub fn send(&self, event: E) {
Expand All @@ -68,9 +62,8 @@ impl<V, S: Clone + Send, E> Wrapper<V, S, E> {
}
}

pub fn value(&self) -> Option<Ref<'_, V>> {
MainThreadMarker::new()
.map(|_| Ref::map(self.value.value.borrow(), |value| value.as_ref().unwrap()))
pub fn value(&self, _: MainThreadMarker) -> Ref<'_, V> {
Ref::map(self.value.value.borrow(), |value| value.as_ref().unwrap())
}

pub fn with_sender_data<T>(&self, f: impl FnOnce(&S) -> T) -> T {
Expand Down
4 changes: 2 additions & 2 deletions src/platform_impl/web/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl Future for CustomCursorFuture {
panic!("`CustomCursorFuture` polled after completion")
}

let result = ready!(Pin::new(&mut self.notified).poll(cx));
let result = ready!(Pin::new(&mut self.notified).poll(cx)).unwrap();
let state = self.state.take().expect("`CustomCursorFuture` polled after completion");

Poll::Ready(result.map(|_| CustomCursor { animation: self.animation, state }))
Expand Down Expand Up @@ -662,7 +662,7 @@ async fn from_animation(
ImageState::Loading { notifier, .. } => {
let notified = notifier.notified();
drop(state);
notified.await?;
notified.await.unwrap()?;
},
ImageState::Failed(error) => return Err(error.clone()),
ImageState::Image(_) => drop(state),
Expand Down
8 changes: 3 additions & 5 deletions src/platform_impl/web/event_loop/proxy.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
use std::rc::Weak;

use super::runner::Execution;
use super::runner::WeakShared;
use crate::platform_impl::platform::r#async::Waker;

#[derive(Clone)]
pub struct EventLoopProxy {
runner: Waker<Weak<Execution>>,
runner: Waker<WeakShared>,
}

impl EventLoopProxy {
pub fn new(runner: Waker<Weak<Execution>>) -> Self {
pub fn new(runner: Waker<WeakShared>) -> Self {
Self { runner }
}

Expand Down
Loading

0 comments on commit ef580b8

Please sign in to comment.