Skip to content
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.

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

process: deprecate _getActive* private APIs in favour of async_hooks #40804

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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;
Copy link
Member

Choose a reason for hiding this comment

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

I think we should leave the actual test in place since we're not removing these APIs yet.

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