feat(engine): add WeakJsObject weak reference type#5437
Conversation
Introduce a WeakJsObject<T> type in boa_engine that lets embedders hold a weak reference to a JsObject without keeping it alive across garbage collections. This is the object-level counterpart of boa_gc::WeakGc. The mechanism already existed internally: the JS-level WeakRef builtin constructs a WeakGc<VTableObject<T>> via WeakGc::new(target.inner()), but JsObject::inner() is pub(crate), so embedders had no public API to build a weak object reference. WeakJsObject wraps that construction and exposes new / upgrade / is_upgradable, mirroring WeakGc's surface, plus Clone, PartialEq, Eq and Hash. Debug is implemented by hand because VTableObject intentionally does not implement Debug. Closes boa-dev#5420 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover the weak reference behaviour of WeakJsObject: upgrading while the referent is live, upgrade returning None after the referent is collected, clone sharing the same referent, equality by referent identity, and construction via the From<&JsObject> conversion. The collection test uses boa_gc::force_collect to drive the garbage collector, mirroring the existing WeakGc tests in core/gc. Refs boa-dev#5420 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The issue only asked for a straightforward WeakJsObject wrapper, and the Eq/Hash impls (mirrored from WeakGc) are unsound to expose on a public API: forwarding to WeakGc compares and hashes by the live referent, so once the object is collected two references to it stop comparing equal and change their hash. That breaks Eq reflexivity (a == a becomes false) and the Hash/Eq invariant for a collected key, making WeakJsObject unusable as a long-lived HashMap/HashSet key, which is the archetypal use of a weak reference. Drop these impls from the initial surface; they can be added back later without a breaking change once a collection-stable identity is designed. To compare weak references, upgrade them and compare the JsObjects. Also expand the tests: an upgraded handle keeps the referent alive across a later collection, all weak references to one object die together, and the generic works with a typed (non-erased) object. Refs boa-dev#5420 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Test262 conformance changes
Tested main commit: |
jedel1043
left a comment
There was a problem hiding this comment.
Mostly looks good, I just have a couple of nitpicks.
| // `VTableObject` deliberately does not implement `Debug` to avoid recursing into the object graph | ||
| // (which could overflow the stack), so we cannot derive `Debug` here. We provide a minimal, | ||
| // non-recursive implementation instead. | ||
| impl<T: NativeObject> Debug for WeakJsObject<T> { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| f.debug_struct("WeakJsObject").finish_non_exhaustive() | ||
| } | ||
| } |
There was a problem hiding this comment.
This was already solved by the Debug implementation on JsObject, which uses a recursion limiter
boa/core/engine/src/object/jsobject.rs
Lines 1290 to 1325 in 6ef5370
You should use that instead.
| #[cfg(test)] | ||
| mod tests { | ||
| use super::{JsObject, WeakJsObject}; | ||
| use boa_gc::force_collect; | ||
|
|
||
| #[test] | ||
| fn upgrade_while_referent_is_live() { | ||
| let object = JsObject::with_null_proto(); | ||
| let weak = WeakJsObject::new(&object); | ||
|
|
||
| assert!(weak.is_upgradable()); | ||
| let upgraded = weak.upgrade().expect("referent is still alive"); | ||
| assert_eq!(upgraded, object); | ||
| } | ||
|
|
||
| #[test] | ||
| fn upgrade_returns_none_after_referent_is_collected() { | ||
| let object = JsObject::with_null_proto(); | ||
| let weak = WeakJsObject::new(&object); | ||
|
|
||
| // While the strong reference is alive the weak one can be upgraded, even across a collection. | ||
| force_collect(); | ||
| assert!(weak.is_upgradable()); | ||
| assert!(weak.upgrade().is_some()); | ||
|
|
||
| // Once the last strong reference is gone the referent can be collected. | ||
| drop(object); | ||
| force_collect(); | ||
|
|
||
| assert!(!weak.is_upgradable()); | ||
| assert!(weak.upgrade().is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn clone_points_to_the_same_referent() { | ||
| let object = JsObject::with_null_proto(); | ||
| let weak = WeakJsObject::new(&object); | ||
| let cloned = weak.clone(); | ||
|
|
||
| assert_eq!( | ||
| weak.upgrade().expect("live"), | ||
| cloned.upgrade().expect("live") | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn upgraded_handle_keeps_referent_alive_across_collection() { | ||
| let object = JsObject::with_null_proto(); | ||
| let weak = WeakJsObject::new(&object); | ||
| let strong = weak.upgrade().expect("referent is still alive"); | ||
|
|
||
| // Drop the original binding; only the upgraded handle roots the referent now. | ||
| drop(object); | ||
| force_collect(); | ||
| assert!(weak.is_upgradable()); | ||
| assert_eq!( | ||
| weak.upgrade().expect("kept alive by the upgraded handle"), | ||
| strong | ||
| ); | ||
|
|
||
| // Once the upgraded handle is gone too, the referent can be collected. | ||
| drop(strong); | ||
| force_collect(); | ||
| assert!(!weak.is_upgradable()); | ||
| assert!(weak.upgrade().is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn all_weaks_to_same_referent_die_together() { | ||
| let object = JsObject::with_null_proto(); | ||
| let first = WeakJsObject::new(&object); | ||
| let cloned = first.clone(); | ||
| let second = WeakJsObject::new(&object); | ||
|
|
||
| drop(object); | ||
| force_collect(); | ||
|
|
||
| assert!(!first.is_upgradable()); | ||
| assert!(!cloned.is_upgradable()); | ||
| assert!(!second.is_upgradable()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn works_with_a_typed_object() { | ||
| use crate::builtins::OrdinaryObject; | ||
|
|
||
| let object: JsObject<OrdinaryObject> = JsObject::new_unique(None, OrdinaryObject); | ||
| let weak: WeakJsObject<OrdinaryObject> = WeakJsObject::new(&object); | ||
|
|
||
| let upgraded: JsObject<OrdinaryObject> = weak.upgrade().expect("referent is still alive"); | ||
| assert_eq!(upgraded, object); | ||
|
|
||
| drop(object); | ||
| drop(upgraded); | ||
| force_collect(); | ||
| assert!(!weak.is_upgradable()); | ||
| assert!(weak.upgrade().is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn built_from_reference_via_conversion() { | ||
| let object = JsObject::with_null_proto(); | ||
| let weak: WeakJsObject = (&object).into(); | ||
|
|
||
| assert_eq!(weak.upgrade().expect("live"), object); | ||
| } | ||
| } |
There was a problem hiding this comment.
Instead of adding tests that are kind of already covered by the WeakGc tests on boa_gc, you should convert these into a nice usage example inside examples.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5437 +/- ##
===========================================
+ Coverage 47.24% 62.74% +15.49%
===========================================
Files 476 535 +59
Lines 46892 59091 +12199
===========================================
+ Hits 22154 37074 +14920
+ Misses 24738 22017 -2721 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This Pull Request closes #5420.
It adds a public
WeakJsObjecttype toboa_engine, giving embedders a weak reference to aJsObjectthat does not keep the object alive across garbage collections. This is the object-level counterpart ofboa_gc::WeakGc.It changes the following:
WeakJsObject<T>incore/engine/src/object/weak.rswithnew,upgradeandis_upgradable, mirroring the surface ofWeakGc, plusClone,DebugandFrom<&JsObject>. The mechanism already existed internally (theWeakRefbuiltin builds aWeakGc<VTableObject<T>>viaWeakGc::new(target.inner())), butJsObject::inner()ispub(crate), so embedders had no public way to construct one.PartialEq/Eq/Hash. Forwarding them toWeakGccompares and hashes by the live referent, so once the object is collected two references to it stop comparing equal and change their hash, which breaksEqreflexivity and theHash/Eqinvariant for a collected key. These can be added later without a breaking change if a collection-stable identity is wanted.Noneafter collection, an upgraded handle keeping the referent alive across a later collection, multiple weak references dropping together, and a typed (non-erased) object.