Skip to content

Commit

Permalink
Avoid leaving a dangling future when handling a rejection in JsPromis…
Browse files Browse the repository at this point in the history
…e::to_future
  • Loading branch information
kjvalencik committed Nov 17, 2023
1 parent 910a548 commit 7d585b4
Showing 1 changed file with 21 additions and 28 deletions.
49 changes: 21 additions & 28 deletions crates/neon/src/types_impl/promise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,58 +185,51 @@ impl JsPromise {
+ Send
+ 'static,
{
let then = self.get::<JsFunction, _, _>(cx, "then")?;
let catch = self.get::<JsFunction, _, _>(cx, "catch")?;

let (tx, rx) = oneshot::channel();
let take_state = {
// Note: If this becomes a bottleneck, `unsafe` could be used to avoid it.
// The promise spec guarantees that it will only be used once.
let state = Arc::new(Mutex::new(Some((f, tx))));

move || {
state
.lock()
.ok()
.and_then(|mut lock| lock.take())
// This should never happen because `self` is a native `Promise`
// and settling multiple times is a violation of the spec.
.expect("Attempted to settle JsFuture multiple times")
}
move || state.lock().ok().and_then(|mut lock| lock.take())
};

let resolve = JsFunction::new(cx, {
let take_state = take_state.clone();

move |mut cx| {
let (f, tx) = take_state();
let v = cx.argument::<JsValue>(0)?;
if let Some((f, tx)) = take_state() {
let v = cx.argument::<JsValue>(0)?;

TaskContext::with_context(cx.env(), move |cx| {
// Error indicates that the `Future` has already dropped; ignore
let _ = tx.send(f(cx, Ok(v)).map_err(Into::into));
});
TaskContext::with_context(cx.env(), move |cx| {
// Error indicates that the `Future` has already dropped; ignore
let _ = tx.send(f(cx, Ok(v)).map_err(Into::into));
});
}

Ok(cx.undefined())
}
})?;

let reject = JsFunction::new(cx, {
move |mut cx| {
let (f, tx) = take_state();
let v = cx.argument::<JsValue>(0)?;
if let Some((f, tx)) = take_state() {
let v = cx.argument::<JsValue>(0)?;

TaskContext::with_context(cx.env(), move |cx| {
// Error indicates that the `Future` has already dropped; ignore
let _ = tx.send(f(cx, Err(v)).map_err(Into::into));
});
TaskContext::with_context(cx.env(), move |cx| {
// Error indicates that the `Future` has already dropped; ignore
let _ = tx.send(f(cx, Err(v)).map_err(Into::into));
});
}

Ok(cx.undefined())
}
})?;

then.exec(cx, Handle::new_internal(Self(self.0)), [resolve.upcast()])?;
catch.exec(cx, Handle::new_internal(Self(self.0)), [reject.upcast()])?;
self.call_method_with(cx, "catch")?
.arg(reject)
.apply::<JsPromise, _>(cx)?
.call_method_with(cx, "then")?
.arg(resolve)
.exec(cx)?;

Ok(JsFuture { rx })
}
Expand Down

0 comments on commit 7d585b4

Please sign in to comment.