Skip to content

Commit 3680790

Browse files
committed
fix(desktop): find a Windows command behind a shell launcher
Git for Windows' bin\bash.exe holds the pty and starts the real bash, so the shell's direct children predate every command and Stop found nothing to end. Stop now walks the shell's descendants and takes the outermost ones created since the command was typed.
1 parent 4e03097 commit 3680790

2 files changed

Lines changed: 71 additions & 27 deletions

File tree

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,38 @@
11
import { describe, expect, it } from 'vitest'
2-
import { windowsProcessGroups } from '@/main/terminal/process-group'
2+
import { type WindowsProcess, windowsProcessGroups } from '@/main/terminal/process-group'
33

44
const SHELL_PID = 100
55
const COMMAND_TYPED_AT = 1_000_000
66

7+
async function foreground(table: WindowsProcess[]): Promise<number[]> {
8+
const ids = await windowsProcessGroups(async () => table).foreground(SHELL_PID, COMMAND_TYPED_AT)
9+
return [...ids].sort()
10+
}
11+
712
describe('the Windows foreground command', () => {
8-
it('is the children started since the command was typed, never earlier background work', async () => {
9-
const groups = windowsProcessGroups(async () => [
10-
{ pid: 201, createdAt: COMMAND_TYPED_AT - 60_000 },
11-
{ pid: 202, createdAt: COMMAND_TYPED_AT },
12-
{ pid: 203, createdAt: COMMAND_TYPED_AT + 15 },
13-
])
13+
it('is what the shell started since the command was typed, never earlier background work', async () => {
14+
await expect(
15+
foreground([
16+
{ pid: 201, parentPid: SHELL_PID, createdAt: COMMAND_TYPED_AT - 60_000 },
17+
{ pid: 202, parentPid: SHELL_PID, createdAt: COMMAND_TYPED_AT },
18+
{ pid: 203, parentPid: SHELL_PID, createdAt: COMMAND_TYPED_AT + 15 },
19+
{ pid: 300, parentPid: 1, createdAt: COMMAND_TYPED_AT + 15 },
20+
])
21+
).resolves.toEqual([202, 203])
22+
await expect(
23+
foreground([{ pid: 201, parentPid: SHELL_PID, createdAt: COMMAND_TYPED_AT - 60_000 }])
24+
).resolves.toEqual([])
25+
})
1426

15-
await expect(groups.foreground(SHELL_PID, COMMAND_TYPED_AT)).resolves.toEqual([202, 203])
27+
it('finds the command behind a launcher, and names only the top of its tree', async () => {
28+
// Git for Windows' bin\bash.exe holds the pty and starts the real usr\bin\bash.exe.
29+
await expect(
30+
foreground([
31+
{ pid: 150, parentPid: SHELL_PID, createdAt: COMMAND_TYPED_AT - 90_000 },
32+
{ pid: 201, parentPid: 150, createdAt: COMMAND_TYPED_AT - 60_000 },
33+
{ pid: 202, parentPid: 150, createdAt: COMMAND_TYPED_AT + 5 },
34+
{ pid: 210, parentPid: 202, createdAt: COMMAND_TYPED_AT + 40 },
35+
])
36+
).resolves.toEqual([202])
1637
})
1738
})

‎apps/desktop/src/main/terminal/process-group.ts‎

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
* with Ctrl-C, including its children, and never the shell itself.
44
*
55
* On macOS and Linux that is the tty's foreground process group. Windows has no process groups or
6-
* job control, so there it is the shell's child processes created since the command was typed,
7-
* each ended with its whole tree. Children from before then (a server the user started in the
8-
* background earlier) belong to someone else's work and are left running. Creation times and the
9-
* command's start come from the same system clock, and the shell only starts a program after it
10-
* reads the command, so no tolerance is needed.
6+
* job control, so there it is the processes the shell started since the command was typed, each
7+
* ended with its whole tree. They are found among the shell's descendants rather than its
8+
* children, since a launcher can stand between the pty and the real shell (Git for Windows'
9+
* `bin\bash.exe` starts `usr\bin\bash.exe`). Processes from before then (a server the user started
10+
* in the background earlier, the real shell behind a launcher) are someone else's and are left
11+
* running. Creation times and the command's start come from the same system clock, and the shell
12+
* only starts a program after it reads the command, so no tolerance is needed.
1113
*/
1214
import { spawn } from 'node:child_process'
1315
import { win32 } from 'node:path'
@@ -54,25 +56,46 @@ const POSIX_PROCESS_GROUPS: TerminalProcessGroups = {
5456
}
5557

5658
/** A Windows process, as the process table reports it. */
57-
export interface WindowsChildProcess {
59+
export interface WindowsProcess {
5860
pid: number
61+
parentPid: number
5962
/** Epoch milliseconds. */
6063
createdAt: number
6164
}
6265

6366
/**
64-
* Windows, given how to list a shell's child processes. The one escalation is a forced tree kill:
65-
* a Windows console program cannot be asked to terminate, and Ctrl-C, the one request it can trap,
67+
* The outermost of the shell's descendants created at or after `startedAt`: the processes a
68+
* command typed then started, each standing for its whole tree.
69+
*/
70+
function commandRoots(table: WindowsProcess[], shellPid: number, startedAt: number): number[] {
71+
const children = new Map<number, WindowsProcess[]>()
72+
for (const entry of table) {
73+
if (entry.pid === entry.parentPid) continue
74+
children.set(entry.parentPid, [...(children.get(entry.parentPid) ?? []), entry])
75+
}
76+
const roots: number[] = []
77+
const visited = new Set<number>([shellPid])
78+
const pending = [...(children.get(shellPid) ?? [])]
79+
for (let entry = pending.pop(); entry; entry = pending.pop()) {
80+
if (visited.has(entry.pid)) continue
81+
visited.add(entry.pid)
82+
if (entry.createdAt >= startedAt) roots.push(entry.pid)
83+
else pending.push(...(children.get(entry.pid) ?? []))
84+
}
85+
return roots
86+
}
87+
88+
/**
89+
* Windows, given how to read the process table. The one escalation is a forced tree kill: a
90+
* Windows console program cannot be asked to terminate, and Ctrl-C, the one request it can trap,
6691
* has already been sent.
6792
*/
6893
export function windowsProcessGroups(
69-
readChildren: (shellPid: number) => Promise<WindowsChildProcess[]>
94+
readProcesses: () => Promise<WindowsProcess[]>
7095
): TerminalProcessGroups {
7196
return {
7297
foreground: async (shellPid, startedAt) =>
73-
(await readChildren(shellPid))
74-
.filter((child) => child.pid !== shellPid && child.createdAt >= startedAt)
75-
.map((child) => child.pid),
98+
commandRoots(await readProcesses(), shellPid, startedAt),
7699
stillRunning: async (_shellPid, pids) => pids.filter(isRunning),
77100
signal: (pids) => {
78101
for (const pid of pids) killProcessTree(pid)
@@ -83,7 +106,7 @@ export function windowsProcessGroups(
83106

84107
/** This operating system's way of reading and stopping a terminal's foreground command. */
85108
export const osProcessGroups: TerminalProcessGroups =
86-
process.platform === 'win32' ? windowsProcessGroups(readChildProcesses) : POSIX_PROCESS_GROUPS
109+
process.platform === 'win32' ? windowsProcessGroups(readProcessTable) : POSIX_PROCESS_GROUPS
87110

88111
/**
89112
* The tty's foreground process group, read from `ps`. Null when it cannot be read, or when the
@@ -107,23 +130,23 @@ function signalProcessGroup(pgid: number, signal: NodeJS.Signals): void {
107130
}
108131
}
109132

110-
/** The shell's direct children, from the process table; each one's tree is what it runs. */
111-
async function readChildProcesses(shellPid: number): Promise<WindowsChildProcess[]> {
112-
if (!isProcessId(shellPid)) return []
133+
async function readProcessTable(): Promise<WindowsProcess[]> {
113134
const stdout = await readOutput(
114135
windowsPowerShellPath(process.env),
115136
[
116137
'-NoLogo',
117138
'-NoProfile',
118139
'-NonInteractive',
119140
'-Command',
120-
`Get-CimInstance -ClassName Win32_Process -Filter 'ParentProcessId=${shellPid}' | ForEach-Object { "$($_.ProcessId) $(([DateTimeOffset]$_.CreationDate).ToUnixTimeMilliseconds())" }`,
141+
'Get-CimInstance -ClassName Win32_Process | ForEach-Object { "$($_.ProcessId) $($_.ParentProcessId) $(([DateTimeOffset]$_.CreationDate).ToUnixTimeMilliseconds())" }',
121142
],
122143
WINDOWS_LOOKUP_TIMEOUT_MS
123144
)
124145
return (stdout ?? '').split(/\r?\n/).flatMap((line) => {
125-
const [pid, createdAt] = line.trim().split(' ').map(Number)
126-
return isProcessId(pid) && Number.isFinite(createdAt) ? [{ pid, createdAt }] : []
146+
const [pid, parentPid, createdAt] = line.trim().split(' ').map(Number)
147+
return isProcessId(pid) && Number.isInteger(parentPid) && Number.isFinite(createdAt)
148+
? [{ pid, parentPid, createdAt }]
149+
: []
127150
})
128151
}
129152

0 commit comments

Comments
 (0)