Skip to content
Open
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/node-core-library",
"comment": "Introduce a `commandLine` property to `IProcessInfo` that gets populated with the CLI that was passed into the process. In Node 24, the process name is set to \"MainThread\", so this restores some previous functionality from before Node 24.",
"type": "minor"
}
],
"packageName": "@rushstack/node-core-library"
}
1 change: 1 addition & 0 deletions common/reviews/api/node-core-library.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ export interface IProblemPattern {
// @public
export interface IProcessInfo {
childProcessInfos: IProcessInfo[];
commandLine?: string;
parentProcessInfo: IProcessInfo | undefined;
processId: number;
processName: string;
Expand Down
74 changes: 62 additions & 12 deletions libraries/node-core-library/src/Executable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,14 @@ export interface IProcessInfo {
*/
processName: string;

/**
* The full command line of the process, when available.
*
* @remarks On some platforms this may be empty or truncated for kernel processes
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a bit more specificity on which platforms would be good.

* or when the OS does not expose the command line.
*/
commandLine?: string;

/**
* The process ID.
*/
Expand Down Expand Up @@ -288,27 +296,62 @@ export function parseProcessListOutput(
// PPID PID COMMAND
// 51234 56784 process name
const NAME_GROUP: 'name' = 'name';
const COMMAND_LINE_GROUP: 'command' = 'command';
const PROCESS_ID_GROUP: 'pid' = 'pid';
const PARENT_PROCESS_ID_GROUP: 'ppid' = 'ppid';
const PROCESS_LIST_ENTRY_REGEX: RegExp = new RegExp(
`^\\s*(?<${PARENT_PROCESS_ID_GROUP}>\\d+)\\s+(?<${PROCESS_ID_GROUP}>\\d+)\\s+(?<${NAME_GROUP}>.+?)\\s*$`
`^\\s*(?<${PARENT_PROCESS_ID_GROUP}>\\d+)\\s+(?<${PROCESS_ID_GROUP}>\\d+)\\s+(?<${NAME_GROUP}>[^\\s]+)(?:\\s+(?<${COMMAND_LINE_GROUP}>.+))?\\s*$`
);

function parseProcessInfoEntry(
line: string,
existingProcessInfoById: Map<number, IProcessInfo>,
platform: NodeJS.Platform
): void {
const processListEntryRegex: RegExp = PROCESS_LIST_ENTRY_REGEX;
const match: RegExpMatchArray | null = line.match(processListEntryRegex);
if (!match?.groups) {
throw new InternalError(`Invalid process list entry: ${line}`);
let processName: string;
let commandLine: string | undefined;
let processId: number;
let parentProcessId: number;

if (platform === 'win32') {
if (line.includes('\t')) {
// Tab-delimited output (PowerShell path with CommandLine)
const win32Match: RegExpMatchArray | null = line.match(
/^\s*(?<ppid>\d+)\s+(?<pid>\d+)\s+(?<name>[^\s]+)(?:\s+(?<cmd>.+))?\s*$/
Copy link
Member

Choose a reason for hiding this comment

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

Feels like this should be a const along with the other regex, and use the same capture group consts as well.

Copy link
Member

Choose a reason for hiding this comment

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

Also, doesn't this regex do the same as the const regex above? They seem to contain the same content.

);
if (!win32Match?.groups) {
throw new InternalError(`Invalid process list entry: ${line}`);
}
processName = win32Match.groups.name;
const cmd: string | undefined = win32Match.groups.cmd;
commandLine = cmd && cmd.length > 0 ? cmd : undefined;
processId = parseInt(win32Match.groups.pid, 10);
parentProcessId = parseInt(win32Match.groups.ppid, 10);
} else {
// Legacy space-delimited listing: treat everything after pid as name, no command line
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that process names can have spaces? Is that why tab-delimited was done later? Also, the regex used for the tab-delimited output seems to not actually care about tabs specifically, just whitespace, which seems to mean that the above regex should also work identically here (or that the above regex has an issue and would push the other segments of the space-containing process name into the command group)

const tokens: string[] = line.trim().split(/\s+/);
if (tokens.length < 3) {
throw new InternalError(`Invalid process list entry: ${line}`);
}
const [ppidString, pidString, ...nameParts] = tokens;
processName = nameParts.join(' ');
commandLine = undefined;
processId = parseInt(pidString, 10);
parentProcessId = parseInt(ppidString, 10);
}
} else {
const processListEntryRegex: RegExp = PROCESS_LIST_ENTRY_REGEX;
const match: RegExpMatchArray | null = line.match(processListEntryRegex);
if (!match?.groups) {
throw new InternalError(`Invalid process list entry: ${line}`);
}
processName = match.groups[NAME_GROUP];
const parsedCommandLine: string | undefined = match.groups[COMMAND_LINE_GROUP];
commandLine = parsedCommandLine && parsedCommandLine.length > 0 ? parsedCommandLine : undefined;
processId = parseInt(match.groups[PROCESS_ID_GROUP], 10);
parentProcessId = parseInt(match.groups[PARENT_PROCESS_ID_GROUP], 10);
}

const processName: string = match.groups[NAME_GROUP];
const processId: number = parseInt(match.groups[PROCESS_ID_GROUP], 10);
const parentProcessId: number = parseInt(match.groups[PARENT_PROCESS_ID_GROUP], 10);

// Only care about the parent process if it is not the same as the current process.
let parentProcessInfo: IProcessInfo | undefined;
if (parentProcessId !== processId) {
Expand All @@ -334,10 +377,16 @@ function parseProcessInfoEntry(
parentProcessInfo,
childProcessInfos: []
};
if (commandLine !== undefined) {
processInfo.commandLine = commandLine;
}
existingProcessInfoById.set(processId, processInfo);
} else {
// Update placeholder entry
processInfo.processName = processName;
if (commandLine !== undefined) {
processInfo.commandLine = commandLine;
}
processInfo.parentProcessInfo = parentProcessInfo;
}

Expand Down Expand Up @@ -368,11 +417,11 @@ function getProcessListProcessOptions(): ICommandLineOptions {
if (OS_PLATFORM === 'win32') {
command = 'powershell.exe';
// Order of declared properties sets the order of the output.
// Put name last to simplify parsing, since it can contain spaces.
// Emit tab-delimited columns to allow the command line to contain spaces.
args = [
'-NoProfile',
'-Command',
`'PPID PID Name'; Get-CimInstance Win32_Process | % { '{0} {1} {2}' -f $_.ParentProcessId, $_.ProcessId, $_.Name }`
`'PPID\`tPID\`tName\`tCommandLine'; Get-CimInstance Win32_Process | % { '{0}\`t{1}\`t{2}\`t{3}' -f $_.ParentProcessId, $_.ProcessId, $_.Name, ($_.CommandLine -replace "\`t", " ") }`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`'PPID\`tPID\`tName\`tCommandLine'; Get-CimInstance Win32_Process | % { '{0}\`t{1}\`t{2}\`t{3}' -f $_.ParentProcessId, $_.ProcessId, $_.Name, ($_.CommandLine -replace "\`t", " ") }`
`"PPID\`0PID\`0Name\`0CommandLine"; Get-CimInstance Win32_Process | % { "{0}\`0{1}\`0{2}\`0{3}" -f $_.ParentProcessId, $_.ProcessId, $_.Name, $_.CommandLine }`

Using NUL as the separator guarantees that it won't be encountered in any of the values. May need to run some tests though to figure out the right quoting to get them to be emitted when piping through the two translation layers, though

];
} else {
command = 'ps';
Expand All @@ -382,7 +431,8 @@ function getProcessListProcessOptions(): ICommandLineOptions {
// Order of declared properties sets the order of the output. We will
// need to request the "comm" property last in order to ensure that the
// process names are not truncated on certain platforms
args = ['-Awo', 'ppid,pid,comm'];
// Include both the comm (thread name) and args (full command line) columns.
args = ['-Awwxo', 'ppid,pid,comm,args'];
}
return { path: command, args };
}
Expand Down
48 changes: 44 additions & 4 deletions libraries/node-core-library/src/test/Executable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,22 +386,62 @@ describe('Executable process list', () => {
' 1 3 process1\n'
];

test('captures command line when present (win32)', () => {
const processListMap: Map<number, IProcessInfo> = parseProcessListOutput(
[
'PPID PID NAME\r\n',
'0 100 node.exe\tC:\\Program Files\\nodejs\\node.exe --foo --bar=baz\r\n',
'100 101 helper.exe\thelper.exe --opt arg\r\n'
],
'win32'
);
const results: IProcessInfo[] = [...processListMap.values()].sort((a, b) => a.processId - b.processId);
expect(results.length).toBe(3);
expect(results).toMatchSnapshot();
});

test('captures command line when present (linux)', () => {
const processListMap: Map<number, IProcessInfo> = parseProcessListOutput(
[
'PPID PID COMMAND\n',
'0 10 node /usr/bin/node --foo --bar=baz\n',
'10 11 child /tmp/child.sh arg1\n'
],
'linux'
);
const results: IProcessInfo[] = [...processListMap.values()].sort((a, b) => a.processId - b.processId);
expect(results.length).toBe(3);
expect(results).toMatchSnapshot();
});

test('contains the current command line (sync)', () => {
const results: ReadonlyMap<number, IProcessInfo> = Executable.getProcessInfoById();
const currentProcessInfo: IProcessInfo | undefined = results.get(process.pid);
expect(currentProcessInfo).toBeDefined();
// Ensure we recorded some command line text for the current process
expect(currentProcessInfo?.commandLine?.length ?? 0).toBeGreaterThan(0);
});

test('contains the current pid (sync)', () => {
const results: ReadonlyMap<number, IProcessInfo> = Executable.getProcessInfoById();
const currentProcessInfo: IProcessInfo | undefined = results.get(process.pid);
expect(currentProcessInfo).toBeDefined();
expect(currentProcessInfo?.parentProcessInfo?.processId).toEqual(process.ppid);
// TODO: Fix parsing of process name as "MainThread" for Node 24
expect(currentProcessInfo?.processName).toMatch(/(node(\.exe)|MainThread)?$/i);
const processIdentity: string = `${currentProcessInfo?.processName ?? ''} ${
currentProcessInfo?.commandLine ?? ''
}`;
expect(processIdentity).toMatch(/node(\.exe)?/i);
});

test('contains the current pid (async)', async () => {
const results: ReadonlyMap<number, IProcessInfo> = await Executable.getProcessInfoByIdAsync();
const currentProcessInfo: IProcessInfo | undefined = results.get(process.pid);
expect(currentProcessInfo).toBeDefined();
expect(currentProcessInfo?.parentProcessInfo?.processId).toEqual(process.ppid);
// TODO: Fix parsing of process name as "MainThread" for Node 24
expect(currentProcessInfo?.processName).toMatch(/(node(\.exe)|MainThread)?$/i);
const processIdentity: string = `${currentProcessInfo?.processName ?? ''} ${
currentProcessInfo?.commandLine ?? ''
}`;
expect(processIdentity).toMatch(/node(\.exe)?/i);
});

test('parses win32 output', () => {
Expand Down
Loading
Loading