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

test: implement setproctitle for windows #14042

Closed
wants to merge 2 commits into from
Closed
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
74 changes: 53 additions & 21 deletions test/parallel/test-setproctitle.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,68 @@ if (common.isSunOS)
common.skip(`Unsupported platform [${process.platform}]`);

const assert = require('assert');
const exec = require('child_process').exec;
const { exec, spawn } = require('child_process');
const path = require('path');

// The title shouldn't be too long; libuv's uv_set_process_title() out of
// security considerations no longer overwrites envp, only argv, so the
// maximum title length is possibly quite short.
let title = String(process.pid);
// maximum title length is possibly quite short (probably just 4 chars).
// We need to use a random shared secret so zombie processes do not cause a
// false positive.
// Ref: https://github.com/nodejs/node/pull/12792
const title = String(Date.now()).substr(-4, 4);

assert.notStrictEqual(process.title, title);
process.title = title;
assert.strictEqual(process.title, title);
if (process.argv[2] === 'child') {
const childTitle = process.argv[3];
assert.notStrictEqual(process.title, childTitle);
process.title = childTitle;
assert.strictEqual(process.title, childTitle);
// Just wait a little bit before exiting.
setTimeout(() => {}, common.platformTimeout(3000));
return;
}

// Test setting the title but do not try to run `ps` on Windows.
if (common.isWindows)
common.skip('Windows does not have "ps" utility');
// This should run only in the parent.
assert.strictEqual(process.argv.length, 2);
let child;
const cmd = (() => {
if (common.isWindows) {
// On Windows it is the `window` that gets the title and not the actual
// process, so we create a new `window` by using `shell: true` and `start`.
child = spawn(
'start /MIN /WAIT',
[process.execPath, __filename, 'child', title],
{ shell: true, stdio: 'ignore' },
);
const PSL = 'Powershell -NoProfile -ExecutionPolicy Unrestricted -Command';
const winTitle = '$_.mainWindowTitle';
return `${PSL} "ps | ? {${winTitle} -eq '${title}'} | % {${winTitle}}"`;
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 this is subject to race condition, where the PowerShell script may run before the spawned child process changes the title? Maybe we can use fork() instead of spawn() and use the IPC channel for sequencing? Would also have the benefit of not having to kill the child.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll research that, also I think there is a guarantee that something (not sure what) has to finish before spawn returns.
Also AFAIK it's possible to establish an IPC via spawn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

P.S. I have seen inconsistant results but I attributed them to the child exiting too soon 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Again, with IPC we could get the child to wait around instead of arbitrary timeout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't use IPC because the script is run not in the child but in a grandchild created with START...

Copy link
Member

Choose a reason for hiding this comment

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

Hm, is there any solution to this? Because without this solved it seems like there can not be any progress.

} else {
// For non Windows we just change this process's own title.
assert.notStrictEqual(process.title, title);
process.title = title;
assert.strictEqual(process.title, title);

// To pass this test on alpine, since Busybox `ps` does not
// support `-p` switch, use `ps -o` and `grep` instead.
const cmd = common.isLinux ?
`ps -o pid,args | grep '${process.pid} ${title}' | grep -v grep` :
`ps -p ${process.pid} -o args=`;
// Since Busybox's `ps` does not support `-p` switch, to pass this test on
// alpine we use `ps -o` and `grep`.
return common.isLinux ?
`ps -o pid,args | grep '${process.pid} ${title}' | grep -v grep` :
`ps -p ${child.pid} -o args=`;
}
})();

exec(cmd, common.mustCall((error, stdout, stderr) => {
// Don't need the child process anymore...
if (child) {
child.kill();
child.unref();
}
assert.ifError(error);
assert.strictEqual(stderr, '');
// We only want the second part (remove the pid).
const ret = stdout.trim().replace(/^\d+\s/, '');

// freebsd always add ' (procname)' to the process title
if (common.isFreeBSD)
title += ` (${path.basename(process.execPath)})`;

// omitting trailing whitespace and \n
assert.strictEqual(stdout.replace(/\s+$/, '').endsWith(title), true);
// `freeBSD` adds ' (procname)' to the process title.
const procname = path.basename(process.execPath);
const expected = title + (common.isFreeBSD ? ` (${procname})` : '');
assert.strictEqual(ret, expected);
}));