Skip to content

feat(engine): add WeakJsObject weak reference type#5437

Open
GordonYuanyc wants to merge 3 commits into
boa-dev:mainfrom
GordonYuanyc:fix-issue-5420
Open

feat(engine): add WeakJsObject weak reference type#5437
GordonYuanyc wants to merge 3 commits into
boa-dev:mainfrom
GordonYuanyc:fix-issue-5420

Conversation

@GordonYuanyc

Copy link
Copy Markdown

This Pull Request closes #5420.

It adds a public WeakJsObject type to boa_engine, giving embedders a weak reference to a JsObject that does not keep the object alive across garbage collections. This is the object-level counterpart of boa_gc::WeakGc.

It changes the following:

  • Adds WeakJsObject<T> in core/engine/src/object/weak.rs with new, upgrade and is_upgradable, mirroring the surface of WeakGc, plus Clone, Debug and From<&JsObject>. The mechanism already existed internally (the WeakRef builtin builds a WeakGc<VTableObject<T>> via WeakGc::new(target.inner())), but JsObject::inner() is pub(crate), so embedders had no public way to construct one.
  • Deliberately does not implement PartialEq/Eq/Hash. Forwarding them 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, which breaks Eq reflexivity and the Hash/Eq invariant for a collected key. These can be added later without a breaking change if a collection-stable identity is wanted.
  • Adds unit tests covering upgrade while live, upgrade returning None after collection, an upgraded handle keeping the referent alive across a later collection, multiple weak references dropping together, and a typed (non-erased) object.

GordonYuanyc and others added 3 commits July 19, 2026 17:41
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>
@GordonYuanyc
GordonYuanyc requested a review from a team as a code owner July 19, 2026 21:49
@github-actions github-actions Bot added the Waiting On Review Waiting on reviews from the maintainers label Jul 19, 2026
@github-actions github-actions Bot added this to the v1.0.0 milestone Jul 19, 2026
@github-actions

Copy link
Copy Markdown

Test262 conformance changes

Test result main count PR count difference
Total 53,125 53,125 0
Passed 51,072 51,072 0
Ignored 1,482 1,482 0
Failed 571 571 0
Panics 0 0 0
Conformance 96.14% 96.14% 0.00%

Tested main commit: 6ef5370c1cff770c51f3e4f8600590f11d66eeaf
Tested PR commit: cd7e25648879ada9c47c8a5c24133f18c38e4e29
Compare commits: 6ef5370...cd7e256

@jedel1043 jedel1043 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, I just have a couple of nitpicks.

Comment on lines +79 to +86
// `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()
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already solved by the Debug implementation on JsObject, which uses a recursion limiter

impl<T: NativeObject> Debug for JsObject<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let limiter = RecursionLimiter::new(self.as_ref());
// Typically, using `!limiter.live` would be good enough here.
// However, the JS object hierarchy involves quite a bit of repetition, and the sheer amount of data makes
// understanding the Debug output impossible; limiting the usefulness of it.
//
// Instead, we check if the object has appeared before in the entire graph. This means that objects will appear
// at most once, hopefully making things a bit clearer.
if !limiter.visited && !limiter.live {
let ptr: *const _ = self.as_ref();
let ptr = ptr.cast::<()>();
let obj = self.borrow();
let kind = obj.data().type_name_of_value();
if self.is_callable() {
let name_prop = obj
.properties()
.get(&PropertyKey::String(js_string!("name")));
let name = match name_prop {
None => JsString::default(),
Some(prop) => prop
.value()
.and_then(JsValue::as_string)
.unwrap_or_default(),
};
return f.write_fmt(format_args!("({:?}) {:?} 0x{:X}", kind, name, ptr as usize));
}
f.write_fmt(format_args!("({:?}) 0x{:X}", kind, ptr as usize))
} else {
f.write_str("{ ... }")
}
}
}

You should use that instead.

Comment on lines +88 to +194
#[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);
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.74%. Comparing base (6ddc2b4) to head (cd7e256).
⚠️ Report is 998 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/object/weak.rs 83.33% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No public weak reference API for JsObject

2 participants