promise: more properly implement Future::poll

The future won't ever complete if you don't connect the waker
from the context!

Prove this out by making the windowops functions async and
verifying them in the async example

promise/src/lib.rs

@@ -45,6 +45,7 @@ enum FutureState<T> {
 struct CoreData<T> {
     result: Option<Result<T, Error>>,
     propagate: Option<NextFunc<T>>,
+    waker: Option<std::task::Waker>,
 }
 
 struct Core<T> {
@@ -78,6 +79,9 @@ impl<T> Drop for Promise<T> {
                 locked.result = Some(err);
             }
             core.cond.notify_one();
+            if let Some(waker) = locked.waker.take() {
+                waker.wake();
+            }
         }
     }
 }
@@ -88,6 +92,7 @@ impl<T> Promise<T> {
             data: Mutex::new(CoreData {
                 result: None,
                 propagate: None,
+                waker: None,
             }),
             cond: Condvar::new(),
         });
@@ -118,7 +123,12 @@ impl<T> Promise<T> {
                 let mut locked = core.data.lock().unwrap();
                 match locked.propagate.take() {
                     Some(func) => func(result),
-                    None => locked.result = Some(result),
+                    None => {
+                        locked.result = Some(result);
+                        if let Some(waker) = locked.waker.take() {
+                            waker.wake();
+                        }
+                    }
                 }
                 core.cond.notify_one();
             }
@@ -297,7 +307,7 @@ impl<T: Send + 'static> Future<T> {
 impl<T: Send + 'static> std::future::Future for Future<T> {
     type Output = Result<T, Error>;
 
-    fn poll(self: Pin<&mut Self>, _ctx: &mut Context) -> Poll<Self::Output> {
+    fn poll(self: Pin<&mut Self>, ctx: &mut Context) -> Poll<Self::Output> {
         // This should be safe because we're not moving the Future,
         // but instead replacing a field, and since no one is able to
         // reference the state field, we should be ok with moving that.
@@ -309,6 +319,8 @@ impl<T: Send + 'static> std::future::Future for Future<T> {
                 let mut locked = core.data.lock().unwrap();
                 if let Some(result) = locked.result.take() {
                     return Poll::Ready(result);
+                } else {
+                    locked.waker = Some(ctx.waker().clone());
                 }
                 drop(locked);
                 myself.state = FutureState::Waiting(core);

src/frontend/gui/termwindow.rs

@@ -192,7 +192,9 @@ impl WindowCallbacks for TermWindow {
                     self.update_title();
                 }
             }
-            _ => context.invalidate(),
+            _ => {
+                context.invalidate();
+            }
         }
 
         // When hovering over a hyperlink, show an appropriate
@@ -436,6 +438,7 @@ impl TermWindow {
                 };
 
                 window.show();
+                Ok(())
             });
         } else {
             window.show();

window/examples/async.rs

@@ -71,9 +71,9 @@ async fn spawn_window() -> Result<(), Box<dyn std::error::Error>> {
         }),
     )?;
 
-    eprintln!("here I am");
-    win.show();
-    eprintln!("and here");
+    eprintln!("before show");
+    win.show().await?;
+    eprintln!("after show");
     win.apply(|myself, _win| {
         eprintln!("doing apply");
         if let Some(myself) = myself.downcast_ref::<MyWindow>() {
@@ -82,7 +82,9 @@ async fn spawn_window() -> Result<(), Box<dyn std::error::Error>> {
                 myself.allow_close, myself.cursor_pos
             );
         }
-    });
+        Ok(())
+    })
+    .await?;
     eprintln!("done with spawn_window");
     Ok(())
 }
@@ -92,6 +94,7 @@ fn main() -> Fallible<()> {
     conn.spawn_task(async {
         eprintln!("running this async block");
         spawn_window().await.ok();
+        eprintln!("end of async block");
     });
     conn.run_message_loop()
 }

window/examples/basic.rs

@@ -103,6 +103,7 @@ fn spawn_window() -> Fallible<()> {
                 myself.allow_close, myself.cursor_pos
             );
         }
+        Ok(())
     });
     Ok(())
 }

window/examples/basic_opengl.rs

@@ -42,9 +42,12 @@ fn spawn_window() -> Fallible<()> {
     )?;
 
     #[cfg(feature = "opengl")]
-    win.enable_opengl(|_any, _window, maybe_ctx| match maybe_ctx {
-        Ok(_ctx) => eprintln!("opengl enabled!"),
-        Err(err) => eprintln!("opengl fail: {}", err),
+    win.enable_opengl(|_any, _window, maybe_ctx| {
+        match maybe_ctx {
+            Ok(_ctx) => eprintln!("opengl enabled!"),
+            Err(err) => eprintln!("opengl fail: {}", err),
+        };
+        Ok(())
     });
 
     #[cfg(not(feature = "opengl"))]
@@ -60,6 +63,7 @@ fn spawn_window() -> Fallible<()> {
                 myself.allow_close, myself.cursor_pos
             );
         }
+        Ok(())
     });
     Ok(())
 }

window/src/lib.rs

@@ -1,3 +1,4 @@
+use promise::Future;
 use std::any::Any;
 pub mod bitmaps;
 pub mod color;
@@ -151,54 +152,63 @@ pub trait WindowCallbacks: Any {
 
 pub trait WindowOps {
     /// Show a hidden window
-    fn show(&self);
+    fn show(&self) -> Future<()>;
 
     /// Hide a visible window
-    fn hide(&self);
+    fn hide(&self) -> Future<()>;
 
     /// Schedule the window to be closed
-    fn close(&self);
+    fn close(&self) -> Future<()>;
 
     /// Change the cursor
-    fn set_cursor(&self, cursor: Option<MouseCursor>);
+    fn set_cursor(&self, cursor: Option<MouseCursor>) -> Future<()>;
 
     /// Invalidate the window so that the entire client area will
     /// be repainted shortly
-    fn invalidate(&self);
+    fn invalidate(&self) -> Future<()>;
 
     /// Change the titlebar text for the window
-    fn set_title(&self, title: &str);
+    fn set_title(&self, title: &str) -> Future<()>;
 
     /// Resize the inner or client area of the window
-    fn set_inner_size(&self, width: usize, height: usize);
+    fn set_inner_size(&self, width: usize, height: usize) -> Future<()>;
 
     /// inform the windowing system of the current textual
     /// cursor input location.  This is used primarily for
     /// the platform specific input method editor
-    fn set_text_cursor_position(&self, _cursor: Rect) {}
+    fn set_text_cursor_position(&self, _cursor: Rect) -> Future<()> {
+        Future::ok(())
+    }
 
     /// Schedule a callback on the data associated with the window.
     /// The `Any` that is passed in corresponds to the WindowCallbacks
     /// impl you passed to `new_window`, pre-converted to Any so that
     /// you can `downcast_ref` or `downcast_mut` it and operate on it.
-    fn apply<F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps)>(&self, func: F)
+    fn apply<R, F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps) -> failure::Fallible<R>>(
+        &self,
+        func: F,
+    ) -> promise::Future<R>
     where
-        Self: Sized;
+        Self: Sized,
+        R: Send + 'static;
 
     #[cfg(feature = "opengl")]
     fn enable_opengl<
+        R,
         F: Send
             + 'static
             + Fn(
                 &mut dyn Any,
                 &dyn WindowOps,
                 failure::Fallible<std::rc::Rc<glium::backend::Context>>,
-            ),
+            ) -> failure::Fallible<R>,
     >(
         &self,
         func: F,
-    ) where
-        Self: Sized;
+    ) -> promise::Future<R>
+    where
+        Self: Sized,
+        R: Send + 'static;
 }
 
 pub trait WindowOpsMut {

window/src/os/macos/connection.rs

@@ -52,16 +52,23 @@ impl Connection {
         self.windows.borrow().get(&window_id).map(Rc::clone)
     }
 
-    pub(crate) fn with_window_inner<F: FnMut(&mut WindowInner) + Send + 'static>(
+    pub(crate) fn with_window_inner<R, F: FnMut(&mut WindowInner) -> Fallible<R> + Send + 'static>(
         window_id: usize,
         mut f: F,
-    ) {
+    ) -> promise::Future<R>
+    where
+        R: Send + 'static,
+    {
+        let mut prom = promise::Promise::new();
+        let future = prom.get_future().unwrap();
         SpawnQueueExecutor {}.execute(Box::new(move || {
             if let Some(handle) = Connection::get().unwrap().window_by_id(window_id) {
                 let mut inner = handle.borrow_mut();
-                f(&mut inner);
+                prom.result(f(&mut inner));
             }
         }));
+
+        future
     }
 
     pub fn executor() -> impl BasicExecutor {

window/src/os/macos/window.rs

@@ -23,6 +23,7 @@ use objc::declare::ClassDecl;
 use objc::rc::{StrongPtr, WeakPtr};
 use objc::runtime::{Class, Object, Protocol, Sel};
 use objc::*;
+use promise::Future;
 use std::any::Any;
 use std::cell::RefCell;
 use std::ffi::c_void;
@@ -342,64 +343,99 @@ impl Window {
 }
 
 impl WindowOps for Window {
-    fn close(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.close());
+    fn close(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.close();
+            Ok(())
+        })
     }
 
-    fn hide(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.hide());
+    fn hide(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.hide();
+            Ok(())
+        })
     }
-    fn show(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.show());
+
+    fn show(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.show();
+            Ok(())
+        })
     }
-    fn set_cursor(&self, cursor: Option<MouseCursor>) {
+
+    fn set_cursor(&self, cursor: Option<MouseCursor>) -> Future<()> {
         Connection::with_window_inner(self.0, move |inner| {
             let _ = inner.set_cursor(cursor);
-        });
+            Ok(())
+        })
     }
-    fn invalidate(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.invalidate());
+
+    fn invalidate(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.invalidate();
+            Ok(())
+        })
     }
-    fn set_title(&self, title: &str) {
+
+    fn set_title(&self, title: &str) -> Future<()> {
         let title = title.to_owned();
-        Connection::with_window_inner(self.0, move |inner| inner.set_title(&title));
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_title(&title);
+            Ok(())
+        })
     }
 
-    fn set_inner_size(&self, width: usize, height: usize) {
-        Connection::with_window_inner(self.0, move |inner| inner.set_inner_size(width, height));
+    fn set_inner_size(&self, width: usize, height: usize) -> Future<()> {
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_inner_size(width, height);
+            Ok(())
+        })
     }
 
-    fn set_text_cursor_position(&self, cursor: Rect) {
-        Connection::with_window_inner(self.0, move |inner| inner.set_text_cursor_position(cursor));
+    fn set_text_cursor_position(&self, cursor: Rect) -> Future<()> {
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_text_cursor_position(cursor);
+            Ok(())
+        })
     }
 
-    fn apply<F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps)>(&self, func: F)
+    fn apply<R, F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps) -> Fallible<R>>(
+        &self,
+        func: F,
+    ) -> promise::Future<R>
     where
         Self: Sized,
+        R: Send + 'static,
     {
         Connection::with_window_inner(self.0, move |inner| {
             let window = Window(inner.window_id);
 
             if let Some(window_view) = WindowView::get_this(unsafe { &**inner.view }) {
-                func(window_view.inner.borrow_mut().callbacks.as_any(), &window);
+                func(window_view.inner.borrow_mut().callbacks.as_any(), &window)
+            } else {
+                failure::bail!("apply: window is invalid");
             }
-        });
+        })
     }
 
     #[cfg(feature = "opengl")]
     fn enable_opengl<
+        R,
         F: Send
             + 'static
             + Fn(
                 &mut dyn Any,
                 &dyn WindowOps,
                 failure::Fallible<std::rc::Rc<glium::backend::Context>>,
-            ),
+            ) -> failure::Fallible<R>,
     >(
         &self,
         func: F,
-    ) where
+    ) -> promise::Future<R>
+    where
         Self: Sized,
+        R: Send + 'static,
     {
         Connection::with_window_inner(self.0, move |inner| {
             let window = Window(inner.window_id);
@@ -414,9 +450,11 @@ impl WindowOps for Window {
                     window_view.inner.borrow_mut().callbacks.as_any(),
                     &window,
                     glium_context.map(|pair| pair.context),
-                );
+                )
+            } else {
+                failure::bail!("enable_opengl: window is invalid");
             }
-        });
+        })
     }
 }
 

window/src/os/windows/connection.rs

@@ -135,18 +135,25 @@ impl Connection {
         self.windows.borrow().get(&handle).map(Rc::clone)
     }
 
-    pub(crate) fn with_window_inner<F: FnMut(&mut WindowInner) + Send + 'static>(
+    pub(crate) fn with_window_inner<R, F: FnMut(&mut WindowInner) -> Fallible<R> + Send + 'static>(
         window: HWindow,
         mut f: F,
-    ) {
+    ) -> promise::Future<R>
+    where
+        R: Send + 'static,
+    {
+        let mut prom = promise::Promise::new();
+        let future = prom.get_future().unwrap();
         SpawnQueueExecutor {}.execute(Box::new(move || {
             if let Some(handle) = Connection::get()
                 .expect("Connection::init has not been called")
                 .get_window(window)
             {
                 let mut inner = handle.borrow_mut();
-                f(&mut inner);
+                prom.result(f(&mut inner));
             }
         }));
+
+        future
     }
 }

window/src/os/windows/window.rs

@@ -211,7 +211,7 @@ impl Window {
     }
 }
 
-fn schedule_show_window(hwnd: HWindow, show: bool) {
+fn schedule_show_window(hwnd: HWindow, show: bool) -> Future<()> {
     // ShowWindow can call to the window proc and may attempt
     // to lock inner, so we avoid locking it ourselves here
     Future::with_executor(Connection::executor(), move || {
@@ -219,7 +219,7 @@ fn schedule_show_window(hwnd: HWindow, show: bool) {
             ShowWindow(hwnd.0, if show { SW_NORMAL } else { SW_HIDE });
         }
         Ok(())
-    });
+    })
 }
 
 impl WindowOpsMut for WindowInner {
@@ -282,60 +282,87 @@ impl WindowOpsMut for WindowInner {
 }
 
 impl WindowOps for Window {
-    fn close(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.close());
+    fn close(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.close();
+            Ok(())
+        })
     }
 
-    fn show(&self) {
-        schedule_show_window(self.0, true);
+    fn show(&self) -> Future<()> {
+        schedule_show_window(self.0, true)
     }
 
-    fn hide(&self) {
-        schedule_show_window(self.0, false);
+    fn hide(&self) -> Future<()> {
+        schedule_show_window(self.0, false)
     }
 
-    fn set_cursor(&self, cursor: Option<MouseCursor>) {
-        Connection::with_window_inner(self.0, move |inner| inner.set_cursor(cursor));
+    fn set_cursor(&self, cursor: Option<MouseCursor>) -> Future<()> {
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_cursor(cursor);
+            Ok(())
+        })
     }
-    fn invalidate(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.invalidate());
+
+    fn invalidate(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.invalidate();
+            Ok(())
+        })
     }
-    fn set_title(&self, title: &str) {
+
+    fn set_title(&self, title: &str) -> Future<()> {
         let title = title.to_owned();
-        Connection::with_window_inner(self.0, move |inner| inner.set_title(&title));
-    }
-    fn set_text_cursor_position(&self, cursor: Rect) {
-        Connection::with_window_inner(self.0, move |inner| inner.set_text_cursor_position(cursor));
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_title(&title);
+            Ok(())
+        })
     }
 
-    fn set_inner_size(&self, width: usize, height: usize) {
-        Connection::with_window_inner(self.0, move |inner| inner.set_inner_size(width, height));
+    fn set_text_cursor_position(&self, cursor: Rect) -> Future<()> {
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_text_cursor_position(cursor);
+            Ok(())
+        })
     }
 
-    fn apply<F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps)>(&self, func: F)
+    fn set_inner_size(&self, width: usize, height: usize) -> Future<()> {
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_inner_size(width, height);
+            Ok(())
+        })
+    }
+    fn apply<R, F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps) -> Fallible<R>>(
+        &self,
+        func: F,
+    ) -> promise::Future<R>
     where
         Self: Sized,
+        R: Send + 'static,
     {
         Connection::with_window_inner(self.0, move |inner| {
             let window = Window(inner.hwnd);
-            func(inner.callbacks.borrow_mut().as_any(), &window);
-        });
+            func(inner.callbacks.borrow_mut().as_any(), &window)
+        })
     }
 
     #[cfg(feature = "opengl")]
     fn enable_opengl<
+        R,
         F: Send
             + 'static
             + Fn(
                 &mut dyn Any,
                 &dyn WindowOps,
                 failure::Fallible<std::rc::Rc<glium::backend::Context>>,
-            ),
+            ) -> failure::Fallible<R>,
     >(
         &self,
         func: F,
-    ) where
+    ) -> promise::Future<R>
+    where
         Self: Sized,
+        R: Send + 'static,
     {
         Connection::with_window_inner(self.0, move |inner| {
             let window = Window(inner.hwnd);
@@ -356,8 +383,8 @@ impl WindowOps for Window {
 
             inner.gl_state = gl_state.as_ref().map(Rc::clone).ok();
 
-            func(inner.callbacks.borrow_mut().as_any(), &window, gl_state);
-        });
+            func(inner.callbacks.borrow_mut().as_any(), &window, gl_state)
+        })
     }
 }
 

window/src/os/x11/connection.rs

@@ -480,16 +480,24 @@ impl Connection {
         LowPriSpawnQueueExecutor {}
     }
 
-    pub(crate) fn with_window_inner<F: FnMut(&mut WindowInner) + Send + 'static>(
+    pub(crate) fn with_window_inner<R, F: FnMut(&mut WindowInner) -> Fallible<R> + Send + 'static>(
         window: xcb::xproto::Window,
         mut f: F,
-    ) {
+    ) -> promise::Future<R>
+    where
+        R: Send + 'static,
+    {
+        let mut prom = promise::Promise::new();
+        let future = prom.get_future().unwrap();
+
         SpawnQueueExecutor {}.execute(Box::new(move || {
             if let Some(handle) = Connection::get().unwrap().window_by_id(window) {
                 let mut inner = handle.lock().unwrap();
-                f(&mut inner);
+                prom.result(f(&mut inner));
             }
         }));
+
+        future
     }
 }
 

window/src/os/x11/window.rs

@@ -6,6 +6,7 @@ use crate::{
     Operator, PaintContext, Point, Rect, Size, WindowCallbacks, WindowOps, WindowOpsMut,
 };
 use failure::Fallible;
+use promise::Future;
 use std::any::Any;
 use std::collections::VecDeque;
 use std::convert::TryInto;
@@ -485,56 +486,87 @@ impl WindowOpsMut for WindowInner {
 }
 
 impl WindowOps for Window {
-    fn close(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.close());
+    fn close(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.close();
+            Ok(())
+        })
     }
-    fn hide(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.hide());
+
+    fn hide(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.hide();
+            Ok(())
+        })
     }
-    fn show(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.show());
+
+    fn show(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.show();
+            Ok(())
+        })
     }
-    fn set_cursor(&self, cursor: Option<MouseCursor>) {
+
+    fn set_cursor(&self, cursor: Option<MouseCursor>) -> Future<()> {
         Connection::with_window_inner(self.0, move |inner| {
             let _ = inner.set_cursor(cursor);
-        });
+            Ok(())
+        })
     }
-    fn invalidate(&self) {
-        Connection::with_window_inner(self.0, |inner| inner.invalidate());
+
+    fn invalidate(&self) -> Future<()> {
+        Connection::with_window_inner(self.0, |inner| {
+            inner.invalidate();
+            Ok(())
+        })
     }
-    fn set_title(&self, title: &str) {
+
+    fn set_title(&self, title: &str) -> Future<()> {
         let title = title.to_owned();
-        Connection::with_window_inner(self.0, move |inner| inner.set_title(&title));
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_title(&title);
+            Ok(())
+        })
     }
 
-    fn set_inner_size(&self, width: usize, height: usize) {
-        Connection::with_window_inner(self.0, move |inner| inner.set_inner_size(width, height));
+    fn set_inner_size(&self, width: usize, height: usize) -> Future<()> {
+        Connection::with_window_inner(self.0, move |inner| {
+            inner.set_inner_size(width, height);
+            Ok(())
+        })
     }
 
-    fn apply<F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps)>(&self, func: F)
+    fn apply<R, F: Send + 'static + Fn(&mut dyn Any, &dyn WindowOps) -> Fallible<R>>(
+        &self,
+        func: F,
+    ) -> promise::Future<R>
     where
         Self: Sized,
+        R: Send + 'static,
     {
         Connection::with_window_inner(self.0, move |inner| {
             let window = Window(inner.window_id);
-            func(inner.callbacks.as_any(), &window);
-        });
+            func(inner.callbacks.as_any(), &window)
+        })
     }
 
     #[cfg(feature = "opengl")]
     fn enable_opengl<
+        R,
         F: Send
             + 'static
             + Fn(
                 &mut dyn Any,
                 &dyn WindowOps,
                 failure::Fallible<std::rc::Rc<glium::backend::Context>>,
-            ),
+            ) -> failure::Fallible<R>,
     >(
         &self,
         func: F,
-    ) where
+    ) -> promise::Future<R>
+    where
         Self: Sized,
+        R: Send + 'static,
     {
         Connection::with_window_inner(self.0, move |inner| {
             let window = Window(inner.window_id);
@@ -558,7 +590,7 @@ impl WindowOps for Window {
 
             inner.gl_state = gl_state.as_ref().map(Rc::clone).ok();
 
-            func(inner.callbacks.as_any(), &window, gl_state);
-        });
+            func(inner.callbacks.as_any(), &window, gl_state)
+        })
     }
 }