From 4763b01178d4166701ccc73678d86c39de084ad5 Mon Sep 17 00:00:00 2001 From: Chris Allen Date: Sun, 21 Mar 2021 14:06:55 -0500 Subject: [PATCH 1/3] Expanding on Root and a potential common-case --- src/handle/root.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/handle/root.rs b/src/handle/root.rs index faeed6e1e..376842286 100644 --- a/src/handle/root.rs +++ b/src/handle/root.rs @@ -43,6 +43,21 @@ impl Root { /// The caller _must_ ensure `Root::into_inner` or `Root::drop` is called /// to properly dispose of the `Root`. If the value is dropped without /// calling one of these methods, it will *panic*. + /// + /// Be careful that you aren't short-circuiting with an early return before + /// your Root value gets into_inner'd or dropped. The following will cause + /// a runtime panic when your error case gets triggered: + /// ``` + /// let callback = cx.argument::(1)?.root(&mut cx); + /// let my_log = match (create_log_entry(&id_generator, "log-emitter")) { + /// Err(_err) => { + /// return cx.throw_error("Couldn't construct log"); + /// }, + /// Ok(log) => log, + /// }; + /// ``` + /// The solution in the original case for this was to bind the callback + /// after the fallible code, right before spawning an async task. pub fn new<'a, C: Context<'a>>(cx: &mut C, value: &T) -> Self { let env = cx.env().to_raw(); let internal = unsafe { reference::new(env, value.to_raw()) }; From 9c58cf975116e86cf48493c5c8d0f5bee6760f05 Mon Sep 17 00:00:00 2001 From: Chris Allen Date: Wed, 24 Mar 2021 14:11:39 -0500 Subject: [PATCH 2/3] Fixed doctest, rewriting the docs to comport with 0.8 --- src/handle/root.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/handle/root.rs b/src/handle/root.rs index 376842286..a8e79f4b4 100644 --- a/src/handle/root.rs +++ b/src/handle/root.rs @@ -40,14 +40,20 @@ impl Root { /// garbage collected until the `Root` is dropped. A `Root` may only /// be dropped on the JavaScript thread that created it. /// - /// The caller _must_ ensure `Root::into_inner` or `Root::drop` is called + /// The caller should ensure `Root::into_inner` or `Root::drop` is called /// to properly dispose of the `Root`. If the value is dropped without - /// calling one of these methods, it will *panic*. + /// calling one of these methods, de-allocation will happen via a + /// slower path. Prior to 0.8, it would actually panic. /// /// Be careful that you aren't short-circuiting with an early return before /// your Root value gets into_inner'd or dropped. The following will cause - /// a runtime panic when your error case gets triggered: + /// the slow path starting with version 0.8 when your error case + /// gets triggered: /// ``` + /// # use neon::prelude::*; + /// # fn create_log_entry(a: &str, b: &str) -> Result { unimplemented!() } + /// # fn my_neon_function(mut cx: FunctionContext) -> JsResult { + /// # let id_generator = ""; /// let callback = cx.argument::(1)?.root(&mut cx); /// let my_log = match (create_log_entry(&id_generator, "log-emitter")) { /// Err(_err) => { @@ -55,6 +61,8 @@ impl Root { /// }, /// Ok(log) => log, /// }; + /// # Ok(cx.undefined()) + /// # } /// ``` /// The solution in the original case for this was to bind the callback /// after the fallible code, right before spawning an async task. From 804712e3d5df89fea23b54b67380e654be3cf492 Mon Sep 17 00:00:00 2001 From: Chris Allen Date: Wed, 24 Mar 2021 14:14:12 -0500 Subject: [PATCH 3/3] Deduplicating wording to present as an example --- src/handle/root.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/handle/root.rs b/src/handle/root.rs index a8e79f4b4..e562b3ba7 100644 --- a/src/handle/root.rs +++ b/src/handle/root.rs @@ -45,10 +45,9 @@ impl Root { /// calling one of these methods, de-allocation will happen via a /// slower path. Prior to 0.8, it would actually panic. /// - /// Be careful that you aren't short-circuiting with an early return before - /// your Root value gets into_inner'd or dropped. The following will cause - /// the slow path starting with version 0.8 when your error case - /// gets triggered: + /// For example, early return with a ? or an explicit return before freeing + /// a `Root` will trigger the slow path: + /// /// ``` /// # use neon::prelude::*; /// # fn create_log_entry(a: &str, b: &str) -> Result { unimplemented!() }