From 52d411e7138bb1599dfc64fd18f32fadd313f7dc Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 21 Aug 2018 13:38:59 -0700 Subject: [PATCH 1/2] test: move common.onGC to individual module Incrementally making `require('../common')` less of a monolith. Move the `common.onGC()` utility to a separate standalone module that is only imported when it's actually needed. --- test/common/README.md | 43 ++++++++++++------- test/common/index.js | 27 ------------ test/common/ongc.js | 32 ++++++++++++++ test/parallel/test-common-gc.js | 7 ++- .../test-gc-http-client-connaborted.js | 5 ++- test/parallel/test-gc-http-client-onerror.js | 5 ++- test/parallel/test-gc-http-client-timeout.js | 5 ++- test/parallel/test-gc-http-client.js | 3 +- test/parallel/test-gc-net-timeout.js | 5 ++- test/parallel/test-net-connect-memleak.js | 3 +- test/parallel/test-tls-connect-memleak.js | 3 +- 11 files changed, 81 insertions(+), 57 deletions(-) create mode 100644 test/common/ongc.js diff --git a/test/common/README.md b/test/common/README.md index 27634c97e11e17..73babed3546bb9 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -317,21 +317,6 @@ otherwise. ### noWarnCode See `common.expectWarning()` for usage. -### onGC(target, listener) -* `target` [<Object>] -* `listener` [<Object>] - * `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. - ### opensslCli * [<boolean>] @@ -730,6 +715,34 @@ via `NODE_TEST_*` environment variables. For example, to configure `internet.addresses.INET_HOST`, set the environment variable `NODE_TEST_INET_HOST` to a specified host. +## ongc Module + +The `ongc` module allows a garbage colleciton listener to be installed. The +module exports a single `onGC()` function. + +```js +require('../common'); +const onGC = require('../common/ongc'); + +onGC({}, { ongc() { console.log('collected'); } }); +``` + +### onGC(target, listener) +* `target` [<Object>] +* `listener` [<Object>] + * `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. + + ## tmpdir Module The `tmpdir` module supports the use of a temporary directory for testing. diff --git a/test/common/index.js b/test/common/index.js index 742a78e3679917..5d0f74e242f98e 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -867,30 +867,3 @@ 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/common/ongc.js b/test/common/ongc.js new file mode 100644 index 00000000000000..d8e27beb8f0a13 --- /dev/null +++ b/test/common/ongc.js @@ -0,0 +1,32 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const gcTrackerMap = new WeakMap(); +const gcTrackerTag = 'NODE_TEST_COMMON_GC_TRACKER'; + +function onGC(obj, gcListener) { + const async_hooks = require('async_hooks'); + + const onGcAsyncHook = async_hooks.createHook({ + init: common.mustCallAtLeast(function(id, type) { + 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; +} + +module.exports = onGC; diff --git a/test/parallel/test-common-gc.js b/test/parallel/test-common-gc.js index 210b1d6d5f49ea..96dc7e2e7b601d 100644 --- a/test/parallel/test-common-gc.js +++ b/test/parallel/test-common-gc.js @@ -1,15 +1,14 @@ 'use strict'; // Flags: --expose-gc const common = require('../common'); +const onGC = require('../common/ongc'); { - const gcListener = { ongc: common.mustCall() }; - common.onGC({}, gcListener); + onGC({}, { ongc: common.mustCall() }); global.gc(); } { - const gcListener = { ongc: common.mustNotCall() }; - common.onGC(process, gcListener); + onGC(process, { ongc: common.mustNotCall() }); global.gc(); } diff --git a/test/parallel/test-gc-http-client-connaborted.js b/test/parallel/test-gc-http-client-connaborted.js index 3218c054ed4a2a..c043c474a65c6b 100644 --- a/test/parallel/test-gc-http-client-connaborted.js +++ b/test/parallel/test-gc-http-client-connaborted.js @@ -3,7 +3,8 @@ // just like test-gc-http-client.js, // but aborting every connection that comes in. -const common = require('../common'); +require('../common'); +const onGC = require('../common/ongc'); function serverHandler(req, res) { res.connection.destroy(); @@ -36,7 +37,7 @@ function getall() { }, cb).on('error', cb); count++; - common.onGC(req, { ongc }); + onGC(req, { ongc }); })(); setImmediate(getall); diff --git a/test/parallel/test-gc-http-client-onerror.js b/test/parallel/test-gc-http-client-onerror.js index 8842da93c30dcf..ef643d255e7ed8 100644 --- a/test/parallel/test-gc-http-client-onerror.js +++ b/test/parallel/test-gc-http-client-onerror.js @@ -3,7 +3,8 @@ // just like test-gc-http-client.js, // but with an on('error') handler that does nothing. -const common = require('../common'); +require('../common'); +const onGC = require('../common/ongc'); function serverHandler(req, res) { req.resume(); @@ -42,7 +43,7 @@ function getall() { }, cb).on('error', onerror); count++; - common.onGC(req, { ongc }); + onGC(req, { ongc }); })(); setImmediate(getall); diff --git a/test/parallel/test-gc-http-client-timeout.js b/test/parallel/test-gc-http-client-timeout.js index e5cb91c06f8911..4b4d1610c3ebdb 100644 --- a/test/parallel/test-gc-http-client-timeout.js +++ b/test/parallel/test-gc-http-client-timeout.js @@ -3,7 +3,8 @@ // just like test-gc-http-client.js, // but with a timeout set -const common = require('../common'); +require('../common'); +const onGC = require('../common/ongc'); function serverHandler(req, res) { setTimeout(function() { @@ -45,7 +46,7 @@ function getall() { }); count++; - common.onGC(req, { ongc }); + onGC(req, { ongc }); })(); setImmediate(getall); diff --git a/test/parallel/test-gc-http-client.js b/test/parallel/test-gc-http-client.js index 5248a1504dfd37..431c6387e171a0 100644 --- a/test/parallel/test-gc-http-client.js +++ b/test/parallel/test-gc-http-client.js @@ -3,6 +3,7 @@ // just a simple http server and client. const common = require('../common'); +const onGC = require('../common/ongc'); function serverHandler(req, res) { res.writeHead(200, { 'Content-Type': 'text/plain' }); @@ -34,7 +35,7 @@ function getall() { }, cb); count++; - common.onGC(req, { ongc }); + onGC(req, { ongc }); setImmediate(getall); } diff --git a/test/parallel/test-gc-net-timeout.js b/test/parallel/test-gc-net-timeout.js index 236980efa3d682..9651e4a2a686dc 100644 --- a/test/parallel/test-gc-net-timeout.js +++ b/test/parallel/test-gc-net-timeout.js @@ -3,7 +3,8 @@ // just like test-gc-http-client-timeout.js, // but using a net server/client instead -const common = require('../common'); +require('../common'); +const onGC = require('../common/ongc'); function serverHandler(sock) { sock.setTimeout(120000); @@ -44,7 +45,7 @@ function getall() { }); count++; - common.onGC(req, { ongc }); + onGC(req, { ongc }); setImmediate(getall); } diff --git a/test/parallel/test-net-connect-memleak.js b/test/parallel/test-net-connect-memleak.js index afcc61f173509f..4efceee0eae138 100644 --- a/test/parallel/test-net-connect-memleak.js +++ b/test/parallel/test-net-connect-memleak.js @@ -23,6 +23,7 @@ // Flags: --expose-gc const common = require('../common'); +const onGC = require('../common/ongc'); const assert = require('assert'); const net = require('net'); @@ -36,7 +37,7 @@ const gcListener = { ongc() { collected = true; } }; { const gcObject = {}; - common.onGC(gcObject, gcListener); + onGC(gcObject, gcListener); const sock = net.createConnection( server.address().port, diff --git a/test/parallel/test-tls-connect-memleak.js b/test/parallel/test-tls-connect-memleak.js index 95f71acdc3b57b..0278e6c7159a68 100644 --- a/test/parallel/test-tls-connect-memleak.js +++ b/test/parallel/test-tls-connect-memleak.js @@ -26,6 +26,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +const onGC = require('../common/ongc'); const assert = require('assert'); const tls = require('tls'); const fixtures = require('../common/fixtures'); @@ -43,7 +44,7 @@ const gcListener = { ongc() { collected = true; } }; { const gcObject = {}; - common.onGC(gcObject, gcListener); + onGC(gcObject, gcListener); const sock = tls.connect( server.address().port, From fe7db9c3b5c27efa356c18b6bd6505134738076a Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 22 Aug 2018 06:37:09 -0700 Subject: [PATCH 2/2] [Squash] nit --- test/common/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/README.md b/test/common/README.md index 73babed3546bb9..8cb64ee8f10c6e 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -717,7 +717,7 @@ variable `NODE_TEST_INET_HOST` to a specified host. ## ongc Module -The `ongc` module allows a garbage colleciton listener to be installed. The +The `ongc` module allows a garbage collection listener to be installed. The module exports a single `onGC()` function. ```js