Defer future execution to the next event loop tick

Previously whenever a future readiness notification came in we would
immediately start polling a future. This ends up having two downsides,
however:

* First, the stack depth may run a risk of getting blown. There's no
  recursion limit to defer execution to later, which means that if
  futures are always ready we'll keep making the stack deeper.

* Second, and more worrisome in the near term, apparently future
  adapaters in the `futures` crate (namely the unsync oneshot channel)
  doesn't actually work if you immediately poll on readiness. This may
  or may not be a bug in the `futures` crate but it's good to fix it
  here anyway.

As a result whenever a future is ready to get polled again we defer its
polling to the next turn of the event loop. This should ensure that the
current call stack is always drained and we're effectively enqueueing
the future to be polled in the near future.
This commit is contained in:
Alex Crichton 2018-10-10 12:41:26 -07:00
parent 70e13705b4
commit a1da85a24b
2 changed files with 44 additions and 7 deletions

View File

@ -108,6 +108,7 @@ extern crate js_sys;
extern crate wasm_bindgen;
use std::cell::{Cell, RefCell};
use std::rc::Rc;
use std::sync::Arc;
use futures::executor::{self, Notify, Spawn};
@ -336,15 +337,14 @@ fn _future_to_promise(future: Box<Future<Item = JsValue, Error = JsValue>>) -> P
impl Notify for Package {
fn notify(&self, _id: usize) {
match self.notified.replace(State::Notified) {
// we need to schedule polling to resume, so we do so
// immediately for now
State::Waiting(me) => Package::poll(&me),
let me = match self.notified.replace(State::Notified) {
// we need to schedule polling to resume, so keep going
State::Waiting(me) => me,
// we were already notified, and were just notified again;
// having now coalesced the notifications we return as it's
// still someone else's job to process this
State::Notified => {}
State::Notified => return,
// the future was previously being polled, and we've just
// switched it to the "you're notified" state. We don't have
@ -353,8 +353,27 @@ fn _future_to_promise(future: Box<Future<Item = JsValue, Error = JsValue>>) -> P
// continue polling. For us, though, there's nothing else to do,
// so we bail out.
// later see
State::Polling => {}
}
State::Polling => return,
};
// Use `Promise.then` on a resolved promise to place our execution
// onto the next turn of the microtask queue, enqueueing our poll
// operation. We don't currently poll immediately as it turns out
// `futures` crate adapters aren't compatible with it and it also
// helps avoid blowing the stack by accident.
//
// Note that the `Rc`/`RefCell` trick here is basically to just
// ensure that our `Closure` gets cleaned up appropriately.
let promise = Promise::resolve(&JsValue::undefined());
let slot = Rc::new(RefCell::new(None));
let slot2 = slot.clone();
let closure = Closure::wrap(Box::new(move |_| {
let myself = slot2.borrow_mut().take();
debug_assert!(myself.is_some());
Package::poll(&me);
}) as Box<FnMut(JsValue)>);
promise.then(&closure);
*slot.borrow_mut() = Some(closure);
}
}
}

18
crates/futures/tests/tests.rs Executable file → Normal file
View File

@ -7,6 +7,7 @@ extern crate wasm_bindgen_futures;
extern crate wasm_bindgen_test;
use futures::Future;
use futures::unsync::oneshot;
use wasm_bindgen::prelude::*;
use wasm_bindgen_futures::{future_to_promise, JsFuture};
use wasm_bindgen_test::*;
@ -48,3 +49,20 @@ fn error_future_is_rejected_promise() -> impl Future<Item = (), Error = JsValue>
Ok(())
})
}
#[wasm_bindgen]
extern {
fn setTimeout(c: &Closure<FnMut()>);
}
#[wasm_bindgen_test(async)]
fn oneshot_works() -> impl Future<Item = (), Error = JsValue> {
let (tx, rx) = oneshot::channel::<u32>();
let mut tx = Some(tx);
let closure = Closure::wrap(Box::new(move || {
drop(tx.take().unwrap());
}) as Box<FnMut()>);
setTimeout(&closure);
closure.forget();
rx.then(|_| Ok(()))
}