From aa56df8b044d7ed3b68a90f87c8992a04922fb2a Mon Sep 17 00:00:00 2001
From: "K.J. Valencik" <kjvalencik@gmail.com>
Date: Fri, 22 Jul 2022 15:13:36 -0400
Subject: [PATCH] fix: More accurate lifetime bounds on scoped methods to allow
 valid code to compile

Prior to this change, it was impossible to escape a value in `execute_scoped` that was created in an outer scope. This is because the
function definition allowed for the outer and inner lifetimes to be entirely unrelated.

This is technically a breaking change, but in many cases the more strict bound will cause code to be more likely to compile.
---
 crates/neon/src/context/mod.rs | 18 +++++++++---------
 test/napi/lib/functions.js     |  3 +++
 test/napi/src/js/functions.rs  |  8 ++++++++
 test/napi/src/lib.rs           |  1 +
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/crates/neon/src/context/mod.rs b/crates/neon/src/context/mod.rs
index 90d51543d..296fca5fc 100644
--- a/crates/neon/src/context/mod.rs
+++ b/crates/neon/src/context/mod.rs
@@ -242,9 +242,10 @@ pub trait Context<'a>: ContextInternal<'a> {
     /// Handles created in the new scope are kept alive only for the duration of the computation and cannot escape.
     ///
     /// This method can be useful for limiting the life of temporary values created during long-running computations, to prevent leaks.
-    fn execute_scoped<T, F>(&mut self, f: F) -> T
+    fn execute_scoped<'b, T, F>(&mut self, f: F) -> T
     where
-        F: for<'b> FnOnce(ExecuteContext<'b>) -> T,
+        'a: 'b,
+        F: FnOnce(ExecuteContext<'b>) -> T,
     {
         let env = self.env();
         let scope = unsafe { HandleScope::new(env.to_raw()) };
@@ -263,17 +264,17 @@ pub trait Context<'a>: ContextInternal<'a> {
     /// Handles created in the new scope are kept alive only for the duration of the computation and cannot escape, with the exception of the result value, which is rooted in the outer context.
     ///
     /// This method can be useful for limiting the life of temporary values created during long-running computations, to prevent leaks.
-    fn compute_scoped<V, F>(&mut self, f: F) -> JsResult<'a, V>
+    fn compute_scoped<'b, V, F>(&mut self, f: F) -> JsResult<'a, V>
     where
+        'a: 'b,
         V: Value,
-        F: for<'b, 'c> FnOnce(ComputeContext<'b, 'c>) -> JsResult<'b, V>,
+        F: FnOnce(ComputeContext<'b>) -> JsResult<'b, V>,
     {
         let env = self.env();
         let scope = unsafe { EscapableHandleScope::new(env.to_raw()) };
         let cx = ComputeContext {
             env,
             phantom_inner: PhantomData,
-            phantom_outer: PhantomData,
         };
 
         let escapee = unsafe { scope.escape(f(cx)?.to_raw()) };
@@ -585,19 +586,18 @@ impl<'a> ContextInternal<'a> for ExecuteContext<'a> {
 impl<'a> Context<'a> for ExecuteContext<'a> {}
 
 /// An execution context of a scope created by [`Context::compute_scoped()`](Context::compute_scoped).
-pub struct ComputeContext<'a, 'outer> {
+pub struct ComputeContext<'a> {
     env: Env,
     phantom_inner: PhantomData<&'a ()>,
-    phantom_outer: PhantomData<&'outer ()>,
 }
 
-impl<'a, 'b> ContextInternal<'a> for ComputeContext<'a, 'b> {
+impl<'a> ContextInternal<'a> for ComputeContext<'a> {
     fn env(&self) -> Env {
         self.env
     }
 }
 
-impl<'a, 'b> Context<'a> for ComputeContext<'a, 'b> {}
+impl<'a> Context<'a> for ComputeContext<'a> {}
 
 /// An execution context of a function call.
 ///
diff --git a/test/napi/lib/functions.js b/test/napi/lib/functions.js
index 05a7a0f7b..2f1ce976b 100644
--- a/test/napi/lib/functions.js
+++ b/test/napi/lib/functions.js
@@ -174,7 +174,10 @@ describe("JsFunction", function () {
   });
 
   it("computes a value in a scoped computation", function () {
+    const o = {};
+
     assert.equal(addon.compute_scoped(), 99);
+    assert.equal(addon.recompute_scoped(o), o);
   });
 
   it("catches an exception with cx.try_catch", function () {
diff --git a/test/napi/src/js/functions.rs b/test/napi/src/js/functions.rs
index 78ac033aa..209bbe8f6 100644
--- a/test/napi/src/js/functions.rs
+++ b/test/napi/src/js/functions.rs
@@ -215,6 +215,14 @@ pub fn compute_scoped(mut cx: FunctionContext) -> JsResult<JsNumber> {
     Ok(i)
 }
 
+// Simple identity function to verify that a handle can be moved to `compute_scoped`
+// closure and re-escaped.
+pub fn recompute_scoped(mut cx: FunctionContext) -> JsResult<JsValue> {
+    let value = cx.argument::<JsValue>(0)?;
+
+    cx.compute_scoped(move |_| Ok(value))
+}
+
 pub fn throw_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
     let v = cx
         .argument_opt(0)
diff --git a/test/napi/src/lib.rs b/test/napi/src/lib.rs
index 0bed7ef10..e35315fb0 100644
--- a/test/napi/src/lib.rs
+++ b/test/napi/src/lib.rs
@@ -193,6 +193,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
     cx.export_function("check_string_and_number", check_string_and_number)?;
     cx.export_function("execute_scoped", execute_scoped)?;
     cx.export_function("compute_scoped", compute_scoped)?;
+    cx.export_function("recompute_scoped", recompute_scoped)?;
 
     cx.export_function("return_js_array", return_js_array)?;
     cx.export_function("return_js_array_with_number", return_js_array_with_number)?;