Skip to content

test: Make N-API weak-ref GC tests asynchronous #13121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 30, 2017
Merged

test: Make N-API weak-ref GC tests asynchronous #13121

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 99 additions & 69 deletions test/addons-napi/test_reference/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,80 +11,110 @@ const test_reference = require(`./build/${common.buildType}/test_reference`);
// of a finalizer callback increments the finalizeCount property.
assert.strictEqual(test_reference.finalizeCount, 0);

{
// External value without a finalizer
let value = test_reference.createExternal();
assert.strictEqual(test_reference.finalizeCount, 0);
assert.strictEqual(typeof value, 'object');
test_reference.checkExternal(value);
value = null;
global.gc();
assert.strictEqual(test_reference.finalizeCount, 0);
// Run each test function in sequence,
// with an async delay and GC call between each.
function runTests(i, title, tests) {
if (tests[i]) {
if (typeof tests[i] === 'string') {
title = tests[i];
runTests(i + 1, title, tests);
} else {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you replace the try...catch, throw, and console.error() call with a call to assert.doesNotThrow()?

Copy link
Member Author

@jasongin jasongin May 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

That could work, except assert.doesNotThrow() loses the stack trace of the original exception; it only displays the message. So I prefer not to use it except when checking for a very specific exceptional condition. In this case, the original exception stack trace is likely to point to a line number of another assertion in the file, so that's very helpful information.

tests[i]();
} catch (e) {
console.error('Test failed: ' + title);
throw e;
}
setImmediate(() => {
global.gc();
runTests(i + 1, title, tests);
});
}
}
}
runTests(0, undefined, [

{
// External value with a finalizer
let value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
assert.strictEqual(typeof value, 'object');
test_reference.checkExternal(value);
value = null;
global.gc();
assert.strictEqual(test_reference.finalizeCount, 1);
}

{
// Strong reference
let value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.createReference(value, 1);
assert.strictEqual(test_reference.referenceValue, value);
value = null;
global.gc(); // Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.deleteReference();
global.gc(); // Value should be GC'd because the strong ref was deleted
assert.strictEqual(test_reference.finalizeCount, 1);
}

{
// Strong reference, increment then decrement to weak reference
let value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.createReference(value, 1);
value = null;
global.gc(); // Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);
'External value without a finalizer',
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to make it an array of objects instead of doing the string check for the title ?
ex
{ title: 'External value without a finalizer', steps: [() => {...}, () => {...}, etc ]}

Copy link
Member Author

@jasongin jasongin May 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

That would work too, but would be more complicated to do the nested iteration recursively. I didn't see a need to over-design this just for these few tests. I can do it if you want. :)

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking it would actually be easier/less code, but if you don't think that's the case I don't want to hold up getting this landed. I may take a look/PR later on when I have time if what I was thinking can simply, if not then I'll leave it be.

() => {
const value = test_reference.createExternal();
assert.strictEqual(test_reference.finalizeCount, 0);
assert.strictEqual(typeof value, 'object');
test_reference.checkExternal(value);
},
() => {
assert.strictEqual(test_reference.finalizeCount, 0);
},

assert.strictEqual(test_reference.incrementRefcount(), 2);
global.gc(); // Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);

assert.strictEqual(test_reference.decrementRefcount(), 1);
global.gc(); // Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);
'External value with a finalizer',
() => {
const value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
assert.strictEqual(typeof value, 'object');
test_reference.checkExternal(value);
},
() => {
assert.strictEqual(test_reference.finalizeCount, 1);
},

assert.strictEqual(test_reference.decrementRefcount(), 0);
global.gc(); // Value should be GC'd because the ref is now weak!
assert.strictEqual(test_reference.finalizeCount, 1);
'Weak reference',
() => {
const value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.createReference(value, 0);
assert.strictEqual(test_reference.referenceValue, value);
},
() => {
// Value should be GC'd because there is only a weak ref
assert.strictEqual(test_reference.referenceValue, undefined);
assert.strictEqual(test_reference.finalizeCount, 1);
test_reference.deleteReference();
},

test_reference.deleteReference();
global.gc(); // Value was already GC'd
assert.strictEqual(test_reference.finalizeCount, 1);
}
'Strong reference',
() => {
const value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.createReference(value, 1);
assert.strictEqual(test_reference.referenceValue, value);
},
() => {
// Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.deleteReference();
},
() => {
// Value should be GC'd because the strong ref was deleted
assert.strictEqual(test_reference.finalizeCount, 1);
},

{
// Weak reference
let value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.createReference(value, 0);
assert.strictEqual(test_reference.referenceValue, value);
value = null;
setImmediate(common.mustCall(() => {
// This test only works if gc() is called from an immediate callback.
global.gc(); // Value should be GC'd because there is only a weak ref
assert.strictEqual(test_reference.referenceValue, undefined);
'Strong reference, increment then decrement to weak reference',
() => {
const value = test_reference.createExternalWithFinalize();
assert.strictEqual(test_reference.finalizeCount, 0);
test_reference.createReference(value, 1);
},
() => {
// Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);
assert.strictEqual(test_reference.incrementRefcount(), 2);
},
() => {
// Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);
assert.strictEqual(test_reference.decrementRefcount(), 1);
},
() => {
// Value should NOT be GC'd because there is a strong ref
assert.strictEqual(test_reference.finalizeCount, 0);
assert.strictEqual(test_reference.decrementRefcount(), 0);
},
() => {
// Value should be GC'd because the ref is now weak!
assert.strictEqual(test_reference.finalizeCount, 1);
test_reference.deleteReference();
}));
}
},
() => {
// Value was already GC'd
assert.strictEqual(test_reference.finalizeCount, 1);
},
]);