hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(desktop): kill timed git commands at app quit, and skip taskkill on an exited root

Browse Source 
Timed execGit commands lead their own POSIX process group, so nothing
reached a probe still running when the app quit: the 5 s timer died with
the process and the group was left orphaned under PID 1 (#125243's
"survives restart"). execGit now tracks timed children, and will-quit
kills each tree (group SIGKILL on POSIX, taskkill /T /F on Windows).

On Windows a root that already exited has no tree left for taskkill /T to
walk and its pid may be reused, so the timeout path skips taskkill there.
The error/close handlers no longer repeat finish()'s clearTimeout.

Quit-time cleanup was first proposed in #125861.

Co-authored-by: chelsealong <chelsealong@126.com>
This commit is contained in:
kshitijk4poor
2026-09-29 19:47:57 +05:30
committed by kshitij
parent 3a21c2a75a
commit 4daeec7f2c
3 changed files with 135 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
apps/desktop/electron/main.ts
View File

@@ -368,7 +368,7 @@ import {
} from './native-oauth'
import { runNativeLogin } from './native-oauth-login'
import { loadNativeTokenSet, type NativeTokenStoreIo, persistNativeTokenSet } from './native-token-store'
import { execGit, setNoConsoleGitRoots } from './no-console-git'
import { execGit, killTimedGitChildren, setNoConsoleGitRoots } from './no-console-git'
import { registerNativeNotifications } from './notification-ipc'
import { isExpectedOauthNavigationAbort } from './oauth-navigation'
import { serializeJsonBody, setJsonRequestHeaders } from './oauth-net-request'
@@ -17767,6 +17767,7 @@ app.on('before-quit', () => {
// Close the pooled keep-alive sockets on quit so lingering connections can't
// hold the event loop open or leak FDs past app teardown.
app.on('will-quit', () => {
killTimedGitChildren()
sshIsolatedKeepalives.stopAll()
destroyKeepaliveAgents()
nativeNotifications.dispose()

88
apps/desktop/electron/no-console-git.test.ts
View File

@@ -10,6 +10,7 @@ import { expect, test, vi } from 'vitest'
import {
CREATE_NO_WINDOW,
execGit,
killTimedGitChildren,
NO_CONSOLE_GIT_SCRIPT,
planNoConsoleGitSpawn,
resolveNoConsolePython,
@@ -19,11 +20,31 @@ import {
const gitArgs = ['-c', 'windows.appendAtomically=false', 'merge-base', '--is-ancestor', 'origin/main', 'HEAD']
const gitBin = 'C:\\Program Files\\Git\\cmd\\git.exe'
test('a timed-out git command reaps descendants, including the Windows Python host', async () => {
// A node "git" that spawns a grandchild owning a TCP listener, then hangs:
// the listener only goes away if the whole tree is killed.
function hangingTreeFixture() {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'git-timeout-'))
const pids = path.join(dir, 'pids.json')
const fixture = path.join(dir, 'hang.cjs')
fs.writeFileSync(
fixture,
`
const { spawn } = require('node:child_process');
const worker = String.raw\`const fs = require('node:fs');
const server = require('node:net').createServer(socket => socket.end());
server.listen(0, '127.0.0.1', () => fs.writeFileSync(process.argv[1],
JSON.stringify({ pids: [Number(process.argv[2]), process.pid], port: server.address().port })));\`;
spawn(process.execPath, ['-e', worker, process.argv[2], String(process.pid)], { stdio: 'inherit' });
setInterval(() => {}, 1000);
`
)
if (process.platform === 'win32') {
const python = execFileSync('python3', ['-c', 'import sys; print(sys.executable)'], { encoding: 'utf8' }).trim()
vi.stubEnv('HERMES_DESKTOP_PYTHON', python)
}
const listening = (port: number): Promise<boolean> =>
new Promise(resolve => {
const socket = connect(port, '127.0.0.1')
@@ -38,33 +59,13 @@ test('a timed-out git command reaps descendants, including the Windows Python ho
socket.setTimeout(2000, () => done(true))
})
fs.writeFileSync(
fixture,
`
const { spawn } = require('node:child_process');
const worker = String.raw\`const fs = require('node:fs');
const server = require('node:net').createServer(socket => socket.end());
server.listen(0, '127.0.0.1', () => fs.writeFileSync(process.argv[1],
JSON.stringify({ pids: [Number(process.argv[2]), process.pid], port: server.address().port })));\`;
spawn(process.execPath, ['-e', worker, process.argv[2], String(process.pid)], { stdio: 'inherit' });
setInterval(() => {}, 1000);
`
)
const port = async (): Promise<number> => {
await vi.waitFor(() => expect(fs.existsSync(pids)).toBe(true), { timeout: 5000 })
try {
if (process.platform === 'win32') {
const python = execFileSync('python3', ['-c', 'import sys; print(sys.executable)'], { encoding: 'utf8' }).trim()
vi.stubEnv('HERMES_DESKTOP_PYTHON', python)
}
return JSON.parse(fs.readFileSync(pids, 'utf8')).port
}
const success = await execGit(process.execPath, ['-e', 'process.stdout.write("ok")'], { timeoutMs: 5000 })
expect(success).toMatchObject({ code: 0, stdout: 'ok' })
const error = await execGit(process.execPath, [fixture, pids], { timeoutMs: 5000 }).catch(error => error)
const { port } = JSON.parse(fs.readFileSync(pids, 'utf8'))
await vi.waitFor(async () => expect(await listening(port)).toBe(false), { timeout: 3000 })
expect(error).toMatchObject({ code: 'ETIMEDOUT' })
} finally {
const cleanup = () => {
if (fs.existsSync(pids)) {
for (const pid of JSON.parse(fs.readFileSync(pids, 'utf8')).pids) {
try {
@@ -78,6 +79,41 @@ test('a timed-out git command reaps descendants, including the Windows Python ho
vi.unstubAllEnvs()
fs.rmSync(dir, { recursive: true, force: true })
}
return { args: [fixture, pids], cleanup, listening, port }
}
test('a timed-out git command reaps descendants, including the Windows Python host', async () => {
const tree = hangingTreeFixture()
try {
const success = await execGit(process.execPath, ['-e', 'process.stdout.write("ok")'], { timeoutMs: 5000 })
expect(success).toMatchObject({ code: 0, stdout: 'ok' })
const error = await execGit(process.execPath, tree.args, { timeoutMs: 5000 }).catch(error => error)
const port = await tree.port()
await vi.waitFor(async () => expect(await tree.listening(port)).toBe(false), { timeout: 3000 })
expect(error).toMatchObject({ code: 'ETIMEDOUT' })
} finally {
tree.cleanup()
}
}, 20_000)
test('app quit kills a timed git command still running, descendants included', async () => {
const tree = hangingTreeFixture()
try {
const pending = execGit(process.execPath, tree.args, { timeoutMs: 60_000 }).catch(error => error)
const port = await tree.port()
expect(await tree.listening(port)).toBe(true)
killTimedGitChildren()
await vi.waitFor(async () => expect(await tree.listening(port)).toBe(false), { timeout: 3000 })
await pending
} finally {
tree.cleanup()
}
}, 20_000)
test('windows git spawn uses a CREATE_NO_WINDOW host and does not rewrite git argv', () => {

100
apps/desktop/electron/no-console-git.ts
View File

@@ -8,7 +8,7 @@
// unchanged. simple-git cannot take a creation flag, so its binary tuple is
// [python, this host script].
import { execFile, spawn, type SpawnOptions } from 'node:child_process'
import { type ChildProcess, execFile, execFileSync, spawn, type SpawnOptions } from 'node:child_process'
import fs from 'node:fs'
import os from 'node:os'
import path from 'node:path'
@@ -244,6 +244,58 @@ export function hiddenGitSpawnSpec(
}
}
// Timed commands still running. On POSIX they lead their own process group,
// so nothing reaches them when the app exits unless we do it here (#125243:
// probes and their promisor fetches outliving a restart, reparented to PID 1).
const timedChildren = new Set<ChildProcess>()
const TASKKILL_OPTIONS = { windowsHide: true, timeout: 5000 }
// On Windows a root that already exited has no tree left for taskkill /T to
// walk, and its pid may already belong to someone else. A POSIX group
// outlives its leader, so it is always signaled.
function hasTreeToKill(child: ChildProcess): child is ChildProcess & { pid: number } {
if (!child.pid) {
return false
}
return process.platform !== 'win32' || (child.exitCode === null && child.signalCode === null)
}
function killGroup(child: ChildProcess & { pid: number }): void {
try {
process.kill(-child.pid, 'SIGKILL')
} catch {
child.kill('SIGKILL')
}
}
/** Kill every timed git command still running, with its descendants. For app quit. */
export function killTimedGitChildren(): void {
const live = [...timedChildren].filter(hasTreeToKill)
timedChildren.clear()
if (process.platform !== 'win32') {
live.forEach(killGroup)
return
}
if (live.length === 0) {
return
}
// One synchronous call for every tree: will-quit cannot wait on N serial ones.
try {
execFileSync('taskkill', [...live.flatMap(child => ['/PID', String(child.pid)]), '/T', '/F'], {
...TASKKILL_OPTIONS,
stdio: 'ignore'
})
} catch {
live.forEach(child => child.kill('SIGKILL'))
}
}
export function execGit(
gitBin: string,
args: string[],
@@ -294,31 +346,22 @@ export function execGit(
// failed tree kill. The timeout remains a failure, never exit 0.
child.stdout?.destroy()
child.stderr?.destroy()
timedChildren.delete(child)
finish(timeoutError)
}
if (process.platform === 'win32' && child.pid) {
execFile(
'taskkill',
['/PID', String(child.pid), '/T', '/F'],
{ windowsHide: true, timeout: 5000 },
error => {
if (error) {
child.kill('SIGKILL')
}
done()
if (!hasTreeToKill(child)) {
done()
} else if (process.platform === 'win32') {
execFile('taskkill', ['/PID', String(child.pid), '/T', '/F'], TASKKILL_OPTIONS, error => {
if (error) {
child.kill('SIGKILL')
}
)
done()
})
} else {
try {
if (child.pid) {
process.kill(-child.pid, 'SIGKILL')
}
} catch {
child.kill('SIGKILL')
}
killGroup(child)
done()
}
}, options.timeoutMs)
@@ -330,18 +373,17 @@ export function execGit(
child.stderr?.on('data', chunk => {
stderr += chunk.toString()
})
child.once('error', error => {
if (timer) {
clearTimeout(timer)
}
if (timer) {
timedChildren.add(child)
child.once('close', () => timedChildren.delete(child))
}
child.once('error', error => {
timedChildren.delete(child)
finish(error)
})
child.once('close', code => {
if (timer) {
clearTimeout(timer)
}
// On Windows wait for taskkill's completion, not just the root's exit.
if (!timeoutError) {
finish(undefined, code)