From 0642e68c98ff72861bb95d475d9031d0e73000fc Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 13 Jul 2018 18:32:17 +0200 Subject: [PATCH 1/4] test: add gc tracking to common API --- test/common/README.md | 15 +++++++++++++++ test/common/index.js | 27 +++++++++++++++++++++++++++ test/parallel/test-common-gc.js | 15 +++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 test/parallel/test-common-gc.js diff --git a/test/common/README.md b/test/common/README.md index 111ce45b00360c..69844ef3dfc2e7 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -326,6 +326,21 @@ See `common.expectWarning()` for usage. Indicates whether 'opensslCli' is supported. +### onGC(target, listener) +* `target` [<Object>] +* `listener` [<Object>] + * `listener.ongc` [<Function>] + +Installs a GC listener for the collection of `target`. + +This uses `async_hooks` for GC tracking. This means that it enables +`async_hooks` tracking, which may affect the test functionality. It also +means that between a `global.gc()` call and the listener being invoked +a full `setImmediate()` invocation passes. + +`listener` is an object to make it easier to use a closure; the target object +should not be in scope when `listener.ongc` is created. + ### platformTimeout(ms) * `ms` [<number>] * return [<number>] diff --git a/test/common/index.js b/test/common/index.js index 904e5a8ef379fa..1df37db54be493 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -882,3 +882,30 @@ exports.isCPPSymbolsNotMapped = exports.isWindows || exports.isAIX || exports.isLinuxPPCBE || exports.isFreeBSD; + +const gcTrackerMap = new WeakMap(); +const gcTrackerTag = 'NODE_TEST_COMMON_GC_TRACKER'; + +exports.onGC = function(obj, gcListener) { + const async_hooks = require('async_hooks'); + + const onGcAsyncHook = async_hooks.createHook({ + init: exports.mustCallAtLeast(function(id, type, trigger, resource) { + if (this.trackedId === undefined) { + assert.strictEqual(type, gcTrackerTag); + this.trackedId = id; + } + }), + destroy(id) { + assert.notStrictEqual(this.trackedId, -1); + if (id === this.trackedId) { + this.gcListener.ongc(); + onGcAsyncHook.disable(); + } + } + }).enable(); + onGcAsyncHook.gcListener = gcListener; + + gcTrackerMap.set(obj, new async_hooks.AsyncResource(gcTrackerTag)); + obj = null; +}; diff --git a/test/parallel/test-common-gc.js b/test/parallel/test-common-gc.js new file mode 100644 index 00000000000000..210b1d6d5f49ea --- /dev/null +++ b/test/parallel/test-common-gc.js @@ -0,0 +1,15 @@ +'use strict'; +// Flags: --expose-gc +const common = require('../common'); + +{ + const gcListener = { ongc: common.mustCall() }; + common.onGC({}, gcListener); + global.gc(); +} + +{ + const gcListener = { ongc: common.mustNotCall() }; + common.onGC(process, gcListener); + global.gc(); +} From 96c1b990a6de208aed55cf72ac81679a8d3e8f91 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 13 Jul 2018 18:32:35 +0200 Subject: [PATCH 2/4] test: refactor test-net-connect-memleak, move to parallel This enables the test to run as part of the regular test suite. --- .../test-net-connect-memleak.js | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) rename test/{pummel => parallel}/test-net-connect-memleak.js (64%) diff --git a/test/pummel/test-net-connect-memleak.js b/test/parallel/test-net-connect-memleak.js similarity index 64% rename from test/pummel/test-net-connect-memleak.js rename to test/parallel/test-net-connect-memleak.js index 7546a6caeb62ea..afcc61f173509f 100644 --- a/test/pummel/test-net-connect-memleak.js +++ b/test/parallel/test-net-connect-memleak.js @@ -26,32 +26,32 @@ const common = require('../common'); const assert = require('assert'); const net = require('net'); -console.log('Run this test with --expose-gc'); -assert.strictEqual( - typeof global.gc, - 'function' -); -net.createServer(function() {}).listen(common.PORT); - -let before = 0; +// Test that the implicit listener for an 'connect' event on net.Sockets is +// added using `once()`, i.e. can be gc'ed once that event has occurred. + +const server = net.createServer(common.mustCall()).listen(0); + +let collected = false; +const gcListener = { ongc() { collected = true; } }; + { - // 2**26 == 64M entries - global.gc(); - const junk = new Array(2 ** 26).fill(0); - before = process.memoryUsage().rss; + const gcObject = {}; + common.onGC(gcObject, gcListener); - net.createConnection(common.PORT, '127.0.0.1', function() { - assert.notStrictEqual(junk.length, 0); // keep reference alive - setTimeout(done, 10); - global.gc(); - }); + const sock = net.createConnection( + server.address().port, + common.mustCall(() => { + assert.strictEqual(gcObject, gcObject); // keep reference alive + assert.strictEqual(collected, false); + setImmediate(done, sock); + })); } -function done() { +function done(sock) { global.gc(); - const after = process.memoryUsage().rss; - const reclaimed = (before - after) / 1024; - console.log('%d kB reclaimed', reclaimed); - assert(reclaimed > 128 * 1024); // It's around 256 MB on x64. - process.exit(); + setImmediate(() => { + assert.strictEqual(collected, true); + sock.end(); + server.close(); + }); } From ed980296631860211a523fdb433550af1bc49440 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 13 Jul 2018 18:32:35 +0200 Subject: [PATCH 3/4] test: refactor test-tls-connect-memleak, move to parallel This enables the test to run as part of the regular test suite. --- .../test-tls-connect-memleak.js | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) rename test/{pummel => parallel}/test-tls-connect-memleak.js (65%) diff --git a/test/pummel/test-tls-connect-memleak.js b/test/parallel/test-tls-connect-memleak.js similarity index 65% rename from test/pummel/test-tls-connect-memleak.js rename to test/parallel/test-tls-connect-memleak.js index 6809b23baf3926..95f71acdc3b57b 100644 --- a/test/pummel/test-tls-connect-memleak.js +++ b/test/parallel/test-tls-connect-memleak.js @@ -30,36 +30,36 @@ const assert = require('assert'); const tls = require('tls'); const fixtures = require('../common/fixtures'); -assert.strictEqual( - typeof global.gc, - 'function', - `Type of global.gc is not a function. Type: ${typeof global.gc}.` + - ' Run this test with --expose-gc' -); +// Test that the implicit listener for an 'connect' event on tls.Sockets is +// added using `once()`, i.e. can be gc'ed once that event has occurred. -tls.createServer({ +const server = tls.createServer({ cert: fixtures.readSync('test_cert.pem'), key: fixtures.readSync('test_key.pem') -}).listen(common.PORT); +}).listen(0); + +let collected = false; +const gcListener = { ongc() { collected = true; } }; { - // 2**26 == 64M entries - const junk = new Array(2 ** 26).fill(0); + const gcObject = {}; + common.onGC(gcObject, gcListener); - const options = { rejectUnauthorized: false }; - tls.connect(common.PORT, '127.0.0.1', options, function() { - assert.notStrictEqual(junk.length, 0); // keep reference alive - setTimeout(done, 10); - global.gc(); - }); + const sock = tls.connect( + server.address().port, + { rejectUnauthorized: false }, + common.mustCall(() => { + assert.strictEqual(gcObject, gcObject); // keep reference alive + assert.strictEqual(collected, false); + setImmediate(done, sock); + })); } -function done() { - const before = process.memoryUsage().rss; +function done(sock) { global.gc(); - const after = process.memoryUsage().rss; - const reclaimed = (before - after) / 1024; - console.log('%d kB reclaimed', reclaimed); - assert(reclaimed > 256 * 1024); // it's more like 512M on x64 - process.exit(); + setImmediate(() => { + assert.strictEqual(collected, true); + sock.end(); + server.close(); + }); } From 24bd33b439065566f926996d5926c586dcdadded Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 13 Jul 2018 19:27:53 +0200 Subject: [PATCH 4/4] fixup! test: add gc tracking to common API --- test/common/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index 69844ef3dfc2e7..256b846a4f53b3 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -321,15 +321,10 @@ otherwise. ### noWarnCode See `common.expectWarning()` for usage. -### opensslCli -* [<boolean>] - -Indicates whether 'opensslCli' is supported. - ### onGC(target, listener) * `target` [<Object>] * `listener` [<Object>] - * `listener.ongc` [<Function>] + * `ongc` [<Function>] Installs a GC listener for the collection of `target`. @@ -339,7 +334,12 @@ means that between a `global.gc()` call and the listener being invoked a full `setImmediate()` invocation passes. `listener` is an object to make it easier to use a closure; the target object -should not be in scope when `listener.ongc` is created. +should not be in scope when `listener.ongc()` is created. + +### opensslCli +* [<boolean>] + +Indicates whether 'opensslCli' is supported. ### platformTimeout(ms) * `ms` [<number>]