Skip to content

Commit

Permalink
process: remove _getActiveHandles and _getActiveRequests
Browse files Browse the repository at this point in the history
Remove the redundnat `_getActiveHandles` and `_getActiveRequests` since
we have a public `getActiveResourcesInfo` method introduced as part of PR #40813.
Refactored tests to use `getActiveResourcesInfo` method.

Ref: #40813
  • Loading branch information
yashLadha committed Jan 19, 2022
1 parent b9258e5 commit a6ec7c5
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 48 deletions.
4 changes: 0 additions & 4 deletions lib/internal/bootstrap/node.js
Expand Up @@ -154,10 +154,6 @@ 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.getActiveResourcesInfo = function() {
const timerCounts = internalTimers.getTimerCounts();
return ArrayPrototypeConcat(
Expand Down
34 changes: 0 additions & 34 deletions src/node_process_methods.cc
Expand Up @@ -243,21 +243,6 @@ static void Uptime(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(result);
}

static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

std::vector<Local<Value>> request_v;
for (ReqWrapBase* req_wrap : *env->req_wrap_queue()) {
AsyncWrap* w = req_wrap->GetAsyncWrap();
if (w->persistent().IsEmpty())
continue;
request_v.emplace_back(w->GetOwner());
}

args.GetReturnValue().Set(
Array::New(env->isolate(), request_v.data(), request_v.size()));
}

static void GetActiveRequestsInfo(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand All @@ -273,21 +258,6 @@ static void GetActiveRequestsInfo(const FunctionCallbackInfo<Value>& args) {
Array::New(env->isolate(), requests_info.data(), requests_info.size()));
}

// Non-static, friend of HandleWrap. Could have been a HandleWrap method but
// implemented here for consistency with GetActiveRequests().
void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

std::vector<Local<Value>> handle_v;
for (auto w : *env->handle_wrap_queue()) {
if (!HandleWrap::HasRef(w))
continue;
handle_v.emplace_back(w->GetOwner());
}
args.GetReturnValue().Set(
Array::New(env->isolate(), handle_v.data(), handle_v.size()));
}

void GetActiveHandlesInfo(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand Down Expand Up @@ -576,9 +546,7 @@ static void Initialize(Local<Object> target,
env->SetMethod(target, "cpuUsage", CPUUsage);
env->SetMethod(target, "resourceUsage", ResourceUsage);

env->SetMethod(target, "_getActiveRequests", GetActiveRequests);
env->SetMethod(target, "_getActiveRequestsInfo", GetActiveRequestsInfo);
env->SetMethod(target, "_getActiveHandles", GetActiveHandles);
env->SetMethod(target, "_getActiveHandlesInfo", GetActiveHandlesInfo);
env->SetMethod(target, "_kill", Kill);

Expand All @@ -605,9 +573,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(CPUUsage);
registry->Register(ResourceUsage);

registry->Register(GetActiveRequests);
registry->Register(GetActiveRequestsInfo);
registry->Register(GetActiveHandles);
registry->Register(GetActiveHandlesInfo);
registry->Register(Kill);

Expand Down
13 changes: 9 additions & 4 deletions test/parallel/test-process-getactivehandles.js
Expand Up @@ -30,18 +30,23 @@ function clientConnected(client) {


function checkAll() {
const handles = process._getActiveHandles();
const handles = process.getActiveResourcesInfo();
const tcpSocketResources = handles
.filter((resource) => resource === 'TCPSocketWrap').length;

assert.ok(tcpSocketResources === clients_counter + connections.length,
'17 socket resources created');

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));
const tcpServerResources = handles
.filter((resource) => resource === 'TCPServerWrap').length;
assert.ok(tcpServerResources === 1, '1 server resource exists');
server.close();
}
2 changes: 1 addition & 1 deletion test/parallel/test-process-getactiverequests.js
Expand Up @@ -7,4 +7,4 @@ const fs = require('fs');
for (let i = 0; i < 12; i++)
fs.open(__filename, 'r', common.mustCall());

assert.strictEqual(process._getActiveRequests().length, 12);
assert.strictEqual(process.getActiveResourcesInfo().length, 12);
13 changes: 8 additions & 5 deletions test/pseudo-tty/ref_keeps_node_running.js
Expand Up @@ -6,25 +6,28 @@ require('../common');
const { internalBinding } = require('internal/test/binding');
const { TTY, isTTY } = internalBinding('tty_wrap');
const strictEqual = require('assert').strictEqual;
const TTY_RESOURCE_NAME = 'TTYWrap';

strictEqual(isTTY(0), true, 'fd 0 is not a TTY');

const handle = new TTY(0);
handle.readStart();
handle.onread = () => {};

function isHandleActive(handle) {
return process._getActiveHandles().some((active) => active === handle);
function isTTYActive(count) {
return process.getActiveResourcesInfo()
.filter((active) => active === TTY_RESOURCE_NAME)
.length === count;
}

strictEqual(isHandleActive(handle), true, 'TTY handle not initially active');
strictEqual(isTTYActive(1), true, 'TTY handle not initially active');

handle.unref();

strictEqual(isHandleActive(handle), false, 'TTY handle active after unref()');
strictEqual(isTTYActive(1), false, 'TTY handle active after unref()');

handle.ref();

strictEqual(isHandleActive(handle), true, 'TTY handle inactive after ref()');
strictEqual(isTTYActive(1), true, 'TTY handle inactive after ref()');

handle.unref();

0 comments on commit a6ec7c5

Please sign in to comment.