From ebb0a83f69589da40907d25de7dc630a6a06c493 Mon Sep 17 00:00:00 2001
From: wytzepiet <wytzepiet@gmail.com>
Date: Sun, 31 May 2026 23:10:30 +0200
Subject: [PATCH] fix(engine): don't crash when the runtime is dropped without
 close()

The fjs bridge function captured the Ctx it runs in. Since the function
lives on the global object, the context referenced itself and could never
be freed, so dropping the runtime without close() (e.g. via Dart's GC)
aborted in JS_FreeRuntime:

  Assertion failed: (list_empty(&rt->gc_obj_list)), quickjs.c:2308

Take ctx as a call-time argument instead of capturing it. The bridge
function no longer pins its context, so the runtime frees cleanly on both
the close() and drop paths. No API change.

Adds a test that drops an engine-with-bridge without close(); it aborted
before this change and passes now.

Fixes #8.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 libfjs/src/api/engine.rs         | 11 ++++++++---
 libfjs/src/tests/engine_tests.rs | 27 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/libfjs/src/api/engine.rs b/libfjs/src/api/engine.rs
index 6b416a4..1b33975 100644
--- a/libfjs/src/api/engine.rs
+++ b/libfjs/src/api/engine.rs
@@ -1260,10 +1260,15 @@ fn new_bridge_call<'js>(
     ctx: rquickjs::Ctx<'js>,
     bridge: Arc<BridgeCallback>,
 ) -> rquickjs::CaughtResult<'js, rquickjs::Function<'js>> {
-    let ctx_for_catch = ctx.clone();
     rquickjs::Function::new(
         ctx.clone(),
-        move |args: rquickjs::function::Rest<rquickjs::Value<'js>>| -> rquickjs::Result<Promise<'js>> {
+        // `ctx` is a parameter, not a capture: capturing it would keep this
+        // function's context alive forever (the function lives on the global),
+        // so dropping the runtime without `close()` would crash in
+        // `JS_FreeRuntime`. See issue #8.
+        move |ctx: rquickjs::Ctx<'js>,
+              args: rquickjs::function::Rest<rquickjs::Value<'js>>|
+              -> rquickjs::Result<Promise<'js>> {
             if args.0.len() > 1 {
                 return Err(rquickjs::Error::TooManyArgs {
                     expected: 1,
@@ -1297,5 +1302,5 @@ fn new_bridge_call<'js>(
             })
         },
     )
-    .catch(&ctx_for_catch)
+    .catch(&ctx)
 }
diff --git a/libfjs/src/tests/engine_tests.rs b/libfjs/src/tests/engine_tests.rs
index 72803fc..fcde682 100644
--- a/libfjs/src/tests/engine_tests.rs
+++ b/libfjs/src/tests/engine_tests.rs
@@ -252,6 +252,33 @@ async fn test_engine_init_with_bridge() {
     engine.close().await.unwrap();
 }
 
+/// Regression test for issue #8: dropping an engine that has a bridge, without
+/// calling `close()` (as Dart's GC does), must not crash. It used to abort in
+/// `JS_FreeRuntime` because the bridge kept its own context alive.
+///
+/// Note: a regression is a process abort (SIGABRT), not a normal test failure —
+/// the second engine below only runs if the first drop was clean.
+#[tokio::test]
+async fn test_engine_drop_with_bridge_without_close_does_not_abort() {
+    {
+        let engine = JsEngine::create(None, None, None).await.unwrap();
+        engine
+            .init(|value| Box::pin(async move { JsResult::Ok(value) }))
+            .await
+            .unwrap();
+        let result = engine.eval(JsCode::Code("1 + 1".to_string()), None).await;
+        assert!(matches!(result.unwrap(), JsValue::Integer(2)));
+        // Drop WITHOUT close(), mimicking Dart's GC finalizer / hot restart.
+        drop(engine);
+    }
+
+    // Reached only if the drop above did not abort the process.
+    let engine = JsEngine::create(None, None, None).await.unwrap();
+    engine.init_without_bridge().await.unwrap();
+    let result = engine.eval(JsCode::Code("2 + 3".to_string()), None).await;
+    assert!(matches!(result.unwrap(), JsValue::Integer(5)));
+}
+
 #[tokio::test]
 async fn test_engine_double_init_fails() {
     let engine = JsEngine::create(None, None, None).await.unwrap();