Skip to content

Commit

Permalink
process: deprecate _getActive* private APIs in favour of async_hooks
Browse files Browse the repository at this point in the history
The `process._getActiveRequests()` and `process._getActiveHandles()`
functions are not needed anymore because the `async_hooks` module
already provides functionality that helps us to keep a track of the
active resources.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>
  • Loading branch information
RaisinTen committed Nov 13, 2021
1 parent dd52c05 commit bcf7c12
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 62 deletions.
15 changes: 15 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -3019,6 +3019,21 @@ should have the same effect. The receiving end should also check the
[`readable.readableEnded`][] value on [`http.IncomingMessage`][] to get whether
it was an aborted or graceful destroy.

### DEP0157: `process._getActiveRequests()`, `process._getActiveHandles()` private APIs

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40804
description: Runtime deprecation.
-->

Type: Runtime

The `process._getActiveRequests()` and `process._getActiveHandles()` functions
are not needed anymore because the `async_hooks` module already provides
functionality that helps us to keep a track of the active resources.

[Legacy URL API]: url.md#legacy-url-api
[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
Expand Down
13 changes: 10 additions & 3 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,16 @@ const rawMethods = internalBinding('process_methods');
process.dlopen = rawMethods.dlopen;
process.uptime = rawMethods.uptime;

// TODO(joyeecheung): either remove them or make them public
process._getActiveRequests = rawMethods._getActiveRequests;
process._getActiveHandles = rawMethods._getActiveHandles;
process._getActiveRequests = deprecate(
rawMethods._getActiveRequests,
'process._getActiveRequests() is deprecated. Please use the `async_hooks`' +
' module instead.',
'DEP0157');
process._getActiveHandles = deprecate(
rawMethods._getActiveHandles,
'process._getActiveHandles() is deprecated. Please use the `async_hooks`' +
' module instead.',
'DEP0157');

// TODO(joyeecheung): remove these
process.reallyExit = rawMethods.reallyExit;
Expand Down
61 changes: 61 additions & 0 deletions test/parallel/test-async-hooks-track-active-handles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
'use strict';

require('../common');

const assert = require('assert');
const async_hooks = require('async_hooks');
const net = require('net');

const activeHandles = new Map();
async_hooks.createHook({
init(asyncId, type, triggerAsyncId, resource) {
if (['TCPSERVERWRAP', 'TCPWRAP'].includes(type))
activeHandles.set(asyncId, resource);
},
destroy(asyncId) {
activeHandles.delete(asyncId);
}
}).enable();

const NUM = 8;
const connections = [];
const clients = [];
let clients_counter = 0;

const server = net.createServer(function listener(c) {
connections.push(c);
}).listen(0, makeConnection);


function makeConnection() {
if (clients_counter >= NUM) return;
net.connect(server.address().port, function connected() {
clientConnected(this);
makeConnection();
});
}


function clientConnected(client) {
clients.push(client);
if (++clients_counter >= NUM)
checkAll();
}


function checkAll() {
const handles = Array.from(activeHandles.values());

clients.forEach(function(item) {
assert.ok(handles.includes(item._handle));
item.destroy();
});

connections.forEach(function(item) {
assert.ok(handles.includes(item._handle));
item.end();
});

assert.ok(handles.includes(server._handle));
server.close();
}
23 changes: 23 additions & 0 deletions test/parallel/test-async-hooks-track-active-requests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';

const common = require('../common');

const assert = require('assert');
const async_hooks = require('async_hooks');
const fs = require('fs');

let numFsReqCallbacks = 0;
async_hooks.createHook({
init(asyncId, type, triggerAsyncId, resource) {
if (type === 'FSREQCALLBACK')
++numFsReqCallbacks;
},
destroy(asyncId) {
--numFsReqCallbacks;
}
}).enable();

for (let i = 0; i < 12; i++)
fs.open(__filename, 'r', common.mustCall());

assert.strictEqual(numFsReqCallbacks, 12);
50 changes: 7 additions & 43 deletions test/parallel/test-process-getactivehandles.js
Original file line number Diff line number Diff line change
@@ -1,47 +1,11 @@
'use strict';

require('../common');
const assert = require('assert');
const net = require('net');
const NUM = 8;
const connections = [];
const clients = [];
let clients_counter = 0;
const { expectWarning } = require('../common');

const server = net.createServer(function listener(c) {
connections.push(c);
}).listen(0, makeConnection);
expectWarning(
'DeprecationWarning',
'process._getActiveHandles() is deprecated. Please use the `async_hooks` ' +
'module instead.',
'DEP0157');


function makeConnection() {
if (clients_counter >= NUM) return;
net.connect(server.address().port, function connected() {
clientConnected(this);
makeConnection();
});
}


function clientConnected(client) {
clients.push(client);
if (++clients_counter >= NUM)
checkAll();
}


function checkAll() {
const handles = process._getActiveHandles();

clients.forEach(function(item) {
assert.ok(handles.includes(item));
item.destroy();
});

connections.forEach(function(item) {
assert.ok(handles.includes(item));
item.end();
});

assert.ok(handles.includes(server));
server.close();
}
process._getActiveHandles();
13 changes: 7 additions & 6 deletions test/parallel/test-process-getactiverequests.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const { expectWarning } = require('../common');

for (let i = 0; i < 12; i++)
fs.open(__filename, 'r', common.mustCall());
expectWarning(
'DeprecationWarning',
'process._getActiveRequests() is deprecated. Please use the `async_hooks` ' +
'module instead.',
'DEP0157');

assert.strictEqual(process._getActiveRequests().length, 12);
process._getActiveRequests();
10 changes: 3 additions & 7 deletions test/pseudo-tty/ref_keeps_node_running.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,14 @@ const handle = new TTY(0);
handle.readStart();
handle.onread = () => {};

function isHandleActive(handle) {
return process._getActiveHandles().some((active) => active === handle);
}

strictEqual(isHandleActive(handle), true, 'TTY handle not initially active');
strictEqual(handle.hasRef(), true, 'TTY handle not initially active');

handle.unref();

strictEqual(isHandleActive(handle), false, 'TTY handle active after unref()');
strictEqual(handle.hasRef(), false, 'TTY handle active after unref()');

handle.ref();

strictEqual(isHandleActive(handle), true, 'TTY handle inactive after ref()');
strictEqual(handle.hasRef(), true, 'TTY handle inactive after ref()');

handle.unref();
28 changes: 25 additions & 3 deletions test/sequential/test-net-connect-econnrefused.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,30 @@
// Verify that connect reqs are properly cleaned up.

const common = require('../common');

const assert = require('assert');
const async_hooks = require('async_hooks');
const net = require('net');

const activeRequestsMap = new Map();
const activeHandlesMap = new Map();
async_hooks.createHook({
init(asyncId, type, triggerAsyncId, resource) {
switch (type) {
case 'TCPCONNECTWRAP':
activeRequestsMap.set(asyncId, resource);
break;
case 'TCPWRAP':
activeHandlesMap.set(asyncId, resource);
break;
}
},
destroy(asyncId) {
activeRequestsMap.delete(asyncId);
activeHandlesMap.delete(asyncId);
}
}).enable();

const ROUNDS = 5;
const ATTEMPTS_PER_ROUND = 50;
let rounds = 1;
Expand Down Expand Up @@ -54,9 +75,10 @@ function pummel() {

function check() {
setTimeout(common.mustCall(function() {
assert.strictEqual(process._getActiveRequests().length, 0);
const activeHandles = process._getActiveHandles();
assert.ok(activeHandles.every((val) => val.constructor.name !== 'Socket'));
const activeRequests = Array.from(activeRequestsMap.values());
assert.strictEqual(activeRequests.length, 0);
const activeHandles = Array.from(activeHandlesMap.values());
assert.ok(activeHandles.every((val) => val.constructor.name === 'TCP'));
}), 0);
}

Expand Down

0 comments on commit bcf7c12

Please sign in to comment.