Skip to content

Commit

Permalink
Modify the GC test so that it fails as expected
Browse files Browse the repository at this point in the history
  • Loading branch information
tronical committed Nov 6, 2023
1 parent 422cca0 commit d9ce2ae
Showing 1 changed file with 13 additions and 14 deletions.
27 changes: 13 additions & 14 deletions api/node/__test__/api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,9 @@ test('callback closure cyclic references do not prevent GC', async (t) => {

// Setup:
// A component instance with a callback installed from JS:
// * The callback captures the surrounding environment, which
// includes an extra reference to the component instance itself
// --> a cyclic reference
// * Invoking the callback clears the reference in the outer captured
// environment.
// The callback captures the surrounding environment, which
// includes an extra reference to the component instance itself
// --> a cyclic reference
//
// Note: WeakRef's deref is used to observe the GC. This means that we must
// separate the test into different jobs with await, to permit for collection.
Expand All @@ -81,13 +79,16 @@ test('callback closure cyclic references do not prevent GC', async (t) => {
let demo = new demo_module.Test();
t.is(demo.check, "initial value");
t.true(Object.hasOwn(demo, "say_hello"));
let callback_invoked = false;

let demo_weak = new WeakRef(demo);

demo.say_hello = () => {
demo = null;
callback_invoked = true;
};
function scope() {
let copy = demo;
copy.say_hello = () => {
console.log(copy.check);
};
}
scope();

t.true(demo_weak.deref() !== undefined);

Expand All @@ -98,10 +99,8 @@ test('callback closure cyclic references do not prevent GC', async (t) => {

t.true(demo_weak.deref() !== undefined);

// Invoke the callback, to clear "demo"
demo.say_hello();
t.true(callback_invoked);
t.true(demo === null);
// Clear the strong reference here
demo = null;

// After the this GC call, the instance should have been collected. Strong references
// in Rust should not keep it alive.
Expand Down

0 comments on commit d9ce2ae

Please sign in to comment.