From 7d585b40e50c36673b12091c385f359806bc5124 Mon Sep 17 00:00:00 2001
From: "K.J. Valencik" <kjvalencik@gmail.com>
Date: Fri, 17 Nov 2023 09:36:55 -0500
Subject: [PATCH] Avoid leaving a dangling future when handling a rejection in
 JsPromise::to_future

---
 crates/neon/src/types_impl/promise.rs | 49 ++++++++++++---------------
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/crates/neon/src/types_impl/promise.rs b/crates/neon/src/types_impl/promise.rs
index 9cf5b0285..56b9e52bf 100644
--- a/crates/neon/src/types_impl/promise.rs
+++ b/crates/neon/src/types_impl/promise.rs
@@ -185,37 +185,25 @@ 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())
             }
@@ -223,20 +211,25 @@ impl JsPromise {
 
         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 })
     }