From 7d4918a17858e9d8a785631c5c8b7b9726102ed1 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 6 Nov 2026 02:37:05 +0800 Subject: [PATCH] js: Keep a live memo's value when its object is collected A memo's JS object marked the memo's value as its child for QuickJS's cycle collector. That is right for a signal or a disposed memo, which are freed with their object, but a live memo belongs to its owner and outlives its object: when the object and the value were in a cycle (the value holding the memo, directly or not), the collector freed the value while the graph still held it. The graph freed it again when the memo ran again or the graph went, which tripped `gc_decref_child`'s assertion in a later collection or crashed in `free_gc_object` when the engine stopped, depending on what had reused the memory. The object now marks the value only when it owns it (`Graph::held_by_handle`, the rule `drop_handle` already followed). A cycle through a live memo is then kept until its owner disposes it, as anything else the owner holds. Co-Authored-By: Claude Opus 5.5 --- crates/nsh-core/src/reactive.rs | 15 +++++++++++--- crates/nsh-js/src/reactive.rs | 36 ++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/crates/nsh-core/src/reactive.rs b/crates/nsh-core/src/reactive.rs index 54f3f7fba2..7a59125fc0 100644 --- a/crates/nsh-core/src/reactive.rs +++ b/crates/nsh-core/src/reactive.rs @@ -425,13 +425,22 @@ impl Graph { self.nodes[id as usize].handle = true; } + /// Whether node `id`'s handle owns what the node holds: it is freed with + /// the handle ([`Self::drop_handle`]), as a signal or a disposed + /// computation is. A live computation belongs to its owner and outlives + /// its handle, so what it holds is not the handle's (a garbage collector + /// must not count it among the handle's children). + pub fn held_by_handle(&self, id: u32) -> bool { + let n = &self.nodes[id as usize]; + n.kind == Kind::Signal || n.disposed + } + /// The handle of node `id` went: a signal, or a disposed computation, /// is freed now (what it held comes back, to free once the graph is no /// longer borrowed). pub fn drop_handle(&mut self, id: u32) -> Option> { - let n = &mut self.nodes[id as usize]; - n.handle = false; - if n.kind == Kind::Signal || n.disposed { + self.nodes[id as usize].handle = false; + if self.held_by_handle(id) { self.unlink_observers(id); Some(self.release(id)) } else { diff --git a/crates/nsh-js/src/reactive.rs b/crates/nsh-js/src/reactive.rs index b1e4cffe7d..c56757a01b 100644 --- a/crates/nsh-js/src/reactive.rs +++ b/crates/nsh-js/src/reactive.rs @@ -533,7 +533,10 @@ unsafe extern "C" fn gc_mark(rt: *mut qjs::JSRuntime, obj: JsValue, mark: qjs::J return; } let (id, generation) = unpack(p); - if g.live(id, generation, GENERATION_MASK) { + // Only what the object owns: the value of a live memo is its node's, + // which its owner keeps after the object goes. Marked as the object's, + // a cycle through the object would free it under the graph. + if g.live(id, generation, GENERATION_MASK) && g.held_by_handle(id) { unsafe { qjs::JS_MarkValue(rt, *g.value(id), mark) }; if let Some(f) = g.custom_equals(id) { unsafe { qjs::JS_MarkValue(rt, *f, mark) }; @@ -1473,6 +1476,37 @@ mod tests { assert_eq!(after, before, "every signal collected"); } + #[test] + fn a_live_memo_keeps_its_value_when_its_object_is_collected() { + // The memo's object goes, but its root does not: the node (and the + // value it holds) lives on in the graph. A cycle through the value + // must not let the collector free the value under the graph, which + // frees it again when the memo runs again or the graph goes. + let rt = rquickjs::Runtime::new().unwrap(); + let cx = rquickjs::Context::full(&rt).unwrap(); + cx.with(|cx| { + crate::state::install(&cx, std::rc::Rc::default(), &[], Default::default()); + super::install_globals(&cx); + let _: () = cx + .eval( + r#" + const R = globalThis.__nsh_reactive; + R.createRoot(() => { + const m = R.createMemo(() => ({ n: 1 })); + // A cycle: the value holds the memo's object. + m().memo = m; + globalThis.weak = new WeakRef(m()); + }); + "#, + ) + .unwrap(); + }); + rt.run_gc(); + let kept: bool = cx.with(|cx| cx.eval("globalThis.weak.deref()?.n === 1").unwrap()); + assert!(kept, "the live memo's value was collected"); + cx.with(|cx| crate::state::uninstall(&cx)); + } + #[test] fn effects_made_untracked_in_a_root_still_track() { // The renderer's shape: `render` (a root), `createComponent`