Skip to content

Commit b64f5c0

Browse files
alexcrichtonalexcrichton
authored
Merge pull request #1649 from alexcrichton/fix-futures
Update futures implementation to not destroy callbacks
2 parents 15cc4fb + d32b6a9 commit b64f5c0

File tree

2 files changed

+81
-71
lines changed

2 files changed

+81
-71
lines changed

‎crates/futures/src/futures_0_3.rs

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::collections::VecDeque;
33
use std::fmt;
44
use std::future::Future;
55
use std::pin::Pin;
6+
use std::rc::Rc;
67
use std::sync::Arc;
78
use std::task::{Context, Poll};
89

@@ -24,10 +25,7 @@ use wasm_bindgen::prelude::*;
2425
///
2526
/// Currently this type is constructed with `JsFuture::from`.
2627
pub struct JsFuture {
27-
resolved: oneshot::Receiver<JsValue>,
28-
rejected: oneshot::Receiver<JsValue>,
29-
_cb_resolve: Closure<dyn FnMut(JsValue)>,
30-
_cb_reject: Closure<dyn FnMut(JsValue)>,
28+
rx: oneshot::Receiver<Result<JsValue, JsValue>>,
3129
}
3230

3331
impl fmt::Debug for JsFuture {
@@ -38,31 +36,37 @@ impl fmt::Debug for JsFuture {
3836

3937
impl From<Promise> for JsFuture {
4038
fn from(js: Promise) -> JsFuture {
41-
// Use the `then` method to schedule two callbacks, one for the
42-
// resolved value and one for the rejected value. These two callbacks
43-
// will be connected to oneshot channels which feed back into our
44-
// future.
45-
//
46-
// This may not be the speediest option today but it should work!
47-
let (tx1, rx1) = oneshot::channel();
48-
49-
let cb_resolve = Closure::once(move |val| {
50-
tx1.send(val).unwrap_throw();
51-
});
52-
53-
let (tx2, rx2) = oneshot::channel();
54-
55-
let cb_reject = Closure::once(move |val| {
56-
tx2.send(val).unwrap_throw();
57-
});
58-
59-
js.then2(&cb_resolve, &cb_reject);
60-
61-
JsFuture {
62-
resolved: rx1,
63-
rejected: rx2,
64-
_cb_resolve: cb_resolve,
65-
_cb_reject: cb_reject,
39+
// See comments in `src/lib.rs` for why we're using one self-contained
40+
// callback here.
41+
let (tx, rx) = oneshot::channel();
42+
let state = Rc::new(RefCell::new(None));
43+
let state2 = state.clone();
44+
let resolve = Closure::once(move |val| finish(&state2, Ok(val)));
45+
let state2 = state.clone();
46+
let reject = Closure::once(move |val| finish(&state2, Err(val)));
47+
48+
js.then2(&resolve, &reject);
49+
*state.borrow_mut() = Some((tx, resolve, reject));
50+
51+
return JsFuture { rx };
52+
53+
fn finish(
54+
state: &RefCell<
55+
Option<(
56+
oneshot::Sender<Result<JsValue, JsValue>>,
57+
Closure<dyn FnMut(JsValue)>,
58+
Closure<dyn FnMut(JsValue)>,
59+
)>,
60+
>,
61+
val: Result<JsValue, JsValue>,
62+
) {
63+
match state.borrow_mut().take() {
64+
// We don't have any guarantee that anyone's still listening at this
65+
// point (the Rust `JsFuture` could have been dropped) so simply
66+
// ignore any errors here.
67+
Some((tx, _, _)) => drop(tx.send(val)),
68+
None => wasm_bindgen::throw_str("cannot finish twice"),
69+
}
6670
}
6771
}
6872
}
@@ -71,16 +75,11 @@ impl Future for JsFuture {
7175
type Output = Result<JsValue, JsValue>;
7276

7377
fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
74-
// Test if either our resolved or rejected side is finished yet.
75-
if let Poll::Ready(val) = self.resolved.poll_unpin(cx) {
76-
return Poll::Ready(Ok(val.unwrap_throw()));
77-
}
78-
79-
if let Poll::Ready(val) = self.rejected.poll_unpin(cx) {
80-
return Poll::Ready(Err(val.unwrap_throw()));
78+
match self.rx.poll_unpin(cx) {
79+
Poll::Pending => Poll::Pending,
80+
Poll::Ready(Ok(val)) => Poll::Ready(val),
81+
Poll::Ready(Err(_)) => wasm_bindgen::throw_str("cannot cancel"),
8182
}
82-
83-
Poll::Pending
8483
}
8584
}
8685

‎crates/futures/src/lib.rs

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,7 @@ use wasm_bindgen::prelude::*;
128128
///
129129
/// Currently this type is constructed with `JsFuture::from`.
130130
pub struct JsFuture {
131-
resolved: oneshot::Receiver<JsValue>,
132-
rejected: oneshot::Receiver<JsValue>,
133-
callbacks: Option<(Closure<dyn FnMut(JsValue)>, Closure<dyn FnMut(JsValue)>)>,
131+
rx: oneshot::Receiver<Result<JsValue, JsValue>>,
134132
}
135133

136134
impl fmt::Debug for JsFuture {
@@ -142,28 +140,49 @@ impl fmt::Debug for JsFuture {
142140
impl From<Promise> for JsFuture {
143141
fn from(js: Promise) -> JsFuture {
144142
// Use the `then` method to schedule two callbacks, one for the
145-
// resolved value and one for the rejected value. These two callbacks
146-
// will be connected to oneshot channels which feed back into our
147-
// future.
143+
// resolved value and one for the rejected value. We're currently
144+
// assuming that JS engines will unconditionally invoke precisely one of
145+
// these callbacks, no matter what.
148146
//
149-
// This may not be the speediest option today but it should work!
150-
let (tx1, rx1) = oneshot::channel();
151-
let (tx2, rx2) = oneshot::channel();
152-
let mut tx1 = Some(tx1);
153-
let resolve = Closure::wrap(Box::new(move |val| {
154-
drop(tx1.take().unwrap().send(val));
155-
}) as Box<dyn FnMut(_)>);
156-
let mut tx2 = Some(tx2);
157-
let reject = Closure::wrap(Box::new(move |val| {
158-
drop(tx2.take().unwrap().send(val));
159-
}) as Box<dyn FnMut(_)>);
147+
// Ideally we'd have a way to cancel the callbacks getting invoked and
148+
// free up state ourselves when this `JsFuture` is dropped. We don't
149+
// have that, though, and one of the callbacks is likely always going to
150+
// be invoked.
151+
//
152+
// As a result we need to make sure that no matter when the callbacks
153+
// are invoked they are valid to be called at any time, which means they
154+
// have to be self-contained. Through the `Closure::once` and some
155+
// `Rc`-trickery we can arrange for both instances of `Closure`, and the
156+
// `Rc`, to all be destroyed once the first one is called.
157+
let (tx, rx) = oneshot::channel();
158+
let state = Rc::new(RefCell::new(None));
159+
let state2 = state.clone();
160+
let resolve = Closure::once(move |val| finish(&state2, Ok(val)));
161+
let state2 = state.clone();
162+
let reject = Closure::once(move |val| finish(&state2, Err(val)));
160163

161164
js.then2(&resolve, &reject);
165+
*state.borrow_mut() = Some((tx, resolve, reject));
166+
167+
return JsFuture { rx };
162168

163-
JsFuture {
164-
resolved: rx1,
165-
rejected: rx2,
166-
callbacks: Some((resolve, reject)),
169+
fn finish(
170+
state: &RefCell<
171+
Option<(
172+
oneshot::Sender<Result<JsValue, JsValue>>,
173+
Closure<dyn FnMut(JsValue)>,
174+
Closure<dyn FnMut(JsValue)>,
175+
)>,
176+
>,
177+
val: Result<JsValue, JsValue>,
178+
) {
179+
match state.borrow_mut().take() {
180+
// We don't have any guarantee that anyone's still listening at this
181+
// point (the Rust `JsFuture` could have been dropped) so simply
182+
// ignore any errors here.
183+
Some((tx, _, _)) => drop(tx.send(val)),
184+
None => wasm_bindgen::throw_str("cannot finish twice"),
185+
}
167186
}
168187
}
169188
}
@@ -173,19 +192,11 @@ impl Future for JsFuture {
173192
type Error = JsValue;
174193

175194
fn poll(&mut self) -> Poll<JsValue, JsValue> {
176-
// Test if either our resolved or rejected side is finished yet. Note
177-
// that they will return errors if they're disconnected which can't
178-
// happen until we drop the `callbacks` field, which doesn't happen
179-
// till we're done, so we dont need to handle that.
180-
if let Ok(Async::Ready(val)) = self.resolved.poll() {
181-
drop(self.callbacks.take());
182-
return Ok(val.into());
183-
}
184-
if let Ok(Async::Ready(val)) = self.rejected.poll() {
185-
drop(self.callbacks.take());
186-
return Err(val);
195+
match self.rx.poll() {
196+
Ok(Async::Ready(val)) => val.map(Async::Ready),
197+
Ok(Async::NotReady) => Ok(Async::NotReady),
198+
Err(_) => wasm_bindgen::throw_str("cannot cancel"),
187199
}
188-
Ok(Async::NotReady)
189200
}
190201
}
191202

0 commit comments

Comments
 (0)