From 0e041498680c1683c024e43b24a4e69dda11b936 Mon Sep 17 00:00:00 2001
From: Simon Hausmann <simon.hausmann@sixtyfps.io>
Date: Sat, 19 Dec 2020 13:49:41 +0100
Subject: [PATCH] Fix single shot timer leaks in the nodejs api

As discussed, don't use the persistent context approach for single
shot timers but store the callback directly in the global object
and "delete" it afterwards.
---
 api/sixtyfps-node/native/Cargo.toml           |  1 +
 api/sixtyfps-node/native/lib.rs               | 32 ++++++++++++++-----
 .../native/persistent_context.rs              | 12 -------
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/api/sixtyfps-node/native/Cargo.toml b/api/sixtyfps-node/native/Cargo.toml
index e5a81793d..a4e3dfe43 100644
--- a/api/sixtyfps-node/native/Cargo.toml
+++ b/api/sixtyfps-node/native/Cargo.toml
@@ -27,6 +27,7 @@ css-color-parser2 = "1.0.1"
 spin_on = "0.1" #FIXME: remove and delegate to async JS instead
 vtable = { version = "0.1.1", path="../../../helper_crates/vtable" }
 generativity = "1"
+rand = "0.7.3"
 
 [build-dependencies]
 neon-build = "0.6.0"
diff --git a/api/sixtyfps-node/native/lib.rs b/api/sixtyfps-node/native/lib.rs
index 2539cccb4..5d0b0f3de 100644
--- a/api/sixtyfps-node/native/lib.rs
+++ b/api/sixtyfps-node/native/lib.rs
@@ -9,6 +9,7 @@
 LICENSE END */
 use core::cell::RefCell;
 use neon::prelude::*;
+use rand::RngCore;
 use sixtyfps_compilerlib::langtype::Type;
 use sixtyfps_corelib::Resource;
 
@@ -491,26 +492,41 @@ declare_types! {
     }
 }
 
+fn singleshot_timer_property(id: u32) -> String {
+    format!("$__sixtyfps_singleshot_timer_{}", id)
+}
+
 fn singleshot_timer(mut cx: FunctionContext) -> JsResult<JsValue> {
     let duration_in_msecs = cx.argument::<JsNumber>(0)?.value() as u64;
     let handler = cx.argument::<JsFunction>(1)?;
 
-    let global_persistent_context = persistent_context::PersistentContext::global(&mut cx)?;
+    let global_object: Handle<JsObject> = cx.global().downcast().unwrap();
+    let unique_timer_property = {
+        let mut rng = rand::thread_rng();
+        loop {
+            let id = rng.next_u32();
+            let key = singleshot_timer_property(id);
+            if global_object.get(&mut cx, &*key)?.is_a::<JsUndefined>() {
+                break key;
+            }
+        }
+    };
 
     let handler_value = handler.as_value(&mut cx);
-    let handler_idx = global_persistent_context.allocate(&mut cx, handler_value);
+    global_object.set(&mut cx, &*unique_timer_property, handler_value).unwrap();
     let callback = move || {
         run_with_global_contect(&move |cx, _| {
-            let global_persistent_context =
-                persistent_context::PersistentContext::global(cx).unwrap();
+            let global_object: Handle<JsObject> = cx.global().downcast().unwrap();
 
-            global_persistent_context
-                .get(cx, handler_idx)
+            let callback = global_object
+                .get(cx, &*unique_timer_property)
                 .unwrap()
                 .downcast::<JsFunction>()
-                .unwrap()
-                .call::<_, _, JsValue, _>(cx, JsUndefined::new(), vec![])
                 .unwrap();
+
+            global_object.set(cx, &*unique_timer_property, JsUndefined::new()).unwrap();
+
+            callback.call::<_, _, JsValue, _>(cx, JsUndefined::new(), vec![]).unwrap();
         });
     };
 
diff --git a/api/sixtyfps-node/native/persistent_context.rs b/api/sixtyfps-node/native/persistent_context.rs
index 823fa046d..2b8acb382 100644
--- a/api/sixtyfps-node/native/persistent_context.rs
+++ b/api/sixtyfps-node/native/persistent_context.rs
@@ -40,16 +40,4 @@ impl<'a> PersistentContext<'a> {
     pub fn from_object(cx: &mut impl Context<'a>, o: Handle<'a, JsObject>) -> NeonResult<Self> {
         Ok(PersistentContext(o.get(cx, KEY)?.downcast_or_throw(cx)?))
     }
-
-    pub fn global(cx: &mut impl Context<'a>) -> NeonResult<Self> {
-        let global_object: Handle<JsObject> = cx.global().downcast().unwrap();
-        Ok(match global_object.get(cx, KEY)?.downcast() {
-            Ok(array) => PersistentContext(array),
-            Err(_) => {
-                let ctx = PersistentContext::new(cx);
-                ctx.save_to_object(cx, global_object);
-                ctx
-            }
-        })
-    }
 }