Skip to content

Commit 8976fa0

Browse files
committed
fix: terminate escaped subprocess descendants
1 parent ad14534 commit 8976fa0

3 files changed

Lines changed: 109 additions & 9 deletions

File tree

Sources/CodexBarCore/Host/PTY/TTYCommandRunner.swift

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ private enum TTYCommandRunnerActiveProcessRegistry {
105105
}
106106

107107
enum TTYProcessTreeTerminator {
108+
struct ProcessIdentity: Equatable {
109+
let pid: pid_t
110+
let startToken: UInt64
111+
}
112+
108113
static func descendantPIDs(
109114
of rootPID: pid_t,
110115
childResolver: (pid_t) -> [pid_t] = Self.currentChildPIDs(of:)) -> [pid_t]
@@ -134,10 +139,49 @@ enum TTYProcessTreeTerminator {
134139
guard childCount > 0 else { return [] }
135140
return Array(pids.prefix(min(Int(childCount), pids.count))).filter { $0 > 0 }
136141
#else
137-
return []
142+
let taskPath = "/proc/\(parentPID)/task"
143+
guard let taskIDs = try? FileManager.default.contentsOfDirectory(atPath: taskPath) else { return [] }
144+
145+
var children: Set<pid_t> = []
146+
for taskID in taskIDs {
147+
let childrenPath = "\(taskPath)/\(taskID)/children"
148+
guard let text = try? String(contentsOfFile: childrenPath, encoding: .utf8) else { continue }
149+
children.formUnion(text.split(whereSeparator: \.isWhitespace).compactMap { pid_t($0) })
150+
}
151+
return children.sorted()
152+
#endif
153+
}
154+
155+
static func processIdentity(for pid: pid_t) -> ProcessIdentity? {
156+
guard pid > 0 else { return nil }
157+
158+
#if canImport(Darwin)
159+
var info = proc_bsdinfo()
160+
let size = proc_pidinfo(
161+
pid,
162+
PROC_PIDTBSDINFO,
163+
0,
164+
&info,
165+
Int32(MemoryLayout<proc_bsdinfo>.stride))
166+
guard size == Int32(MemoryLayout<proc_bsdinfo>.stride) else { return nil }
167+
let startToken = UInt64(info.pbi_start_tvsec) * 1_000_000 + UInt64(info.pbi_start_tvusec)
168+
return ProcessIdentity(pid: pid, startToken: startToken)
169+
#else
170+
guard let stat = try? String(contentsOfFile: "/proc/\(pid)/stat", encoding: .utf8),
171+
let commandEnd = stat.lastIndex(of: ")")
172+
else {
173+
return nil
174+
}
175+
let fields = stat[stat.index(after: commandEnd)...].split(whereSeparator: \.isWhitespace)
176+
guard fields.count > 19, let startToken = UInt64(fields[19]) else { return nil }
177+
return ProcessIdentity(pid: pid, startToken: startToken)
138178
#endif
139179
}
140180

181+
static func isCurrent(_ identity: ProcessIdentity) -> Bool {
182+
self.processIdentity(for: identity.pid) == identity
183+
}
184+
141185
static func terminateProcessTree(
142186
rootPID: pid_t,
143187
processGroup: pid_t?,

Sources/CodexBarCore/Host/Process/SubprocessRunner.swift

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,23 +112,34 @@ public enum SubprocessRunner {
112112
@discardableResult
113113
package static func terminateProcess(_ process: Process, processGroup: pid_t?) -> Bool {
114114
guard process.isRunning else { return false }
115-
process.terminate()
116-
if let pgid = processGroup {
117-
kill(-pgid, SIGTERM)
118-
}
115+
let descendants = TTYProcessTreeTerminator.descendantPIDs(of: process.processIdentifier)
116+
let descendantIdentities = descendants.compactMap(TTYProcessTreeTerminator.processIdentity(for:))
117+
TTYProcessTreeTerminator.terminateProcessTree(
118+
rootPID: process.processIdentifier,
119+
processGroup: processGroup,
120+
signal: SIGTERM,
121+
knownDescendants: descendants)
119122
let killDeadline = Date().addingTimeInterval(0.4)
120123
while process.isRunning, Date() < killDeadline {
121124
usleep(50000)
122125
}
123126
if process.isRunning {
124-
if let pgid = processGroup {
125-
kill(-pgid, SIGKILL)
126-
}
127-
kill(process.processIdentifier, SIGKILL)
127+
let currentDescendants = descendantIdentities
128+
.filter(TTYProcessTreeTerminator.isCurrent(_:))
129+
.map(\.pid)
130+
TTYProcessTreeTerminator.terminateProcessTree(
131+
rootPID: process.processIdentifier,
132+
processGroup: processGroup,
133+
signal: SIGKILL,
134+
knownDescendants: currentDescendants)
128135
let reapDeadline = Date().addingTimeInterval(0.4)
129136
while process.isRunning, Date() < reapDeadline {
130137
usleep(50000)
131138
}
139+
} else {
140+
for identity in descendantIdentities where TTYProcessTreeTerminator.isCurrent(identity) {
141+
kill(identity.pid, SIGKILL)
142+
}
132143
}
133144
return true
134145
}

Tests/CodexBarTests/SubprocessRunnerTests.swift

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,51 @@ struct SubprocessRunnerTests {
9898
#expect(elapsed < 3, "Timeout should fire in ~1s, not wait for process to exit naturally")
9999
}
100100

101+
@Test
102+
func `timeout kills descendants that escape the process group`() async throws {
103+
let root = FileManager.default.temporaryDirectory
104+
.appendingPathComponent("codexbar-subprocess-tree-\(UUID().uuidString)", isDirectory: true)
105+
let childPIDFile = root.appendingPathComponent("child.pid")
106+
try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)
107+
defer { try? FileManager.default.removeItem(at: root) }
108+
109+
var environment = ProcessInfo.processInfo.environment
110+
environment["CODEXBAR_TEST_CHILD_PID_FILE"] = childPIDFile.path
111+
let script = """
112+
import os
113+
import subprocess
114+
import sys
115+
import time
116+
117+
child = subprocess.Popen(
118+
[sys.executable, "-c", "import time; time.sleep(30)"],
119+
start_new_session=True,
120+
)
121+
with open(os.environ["CODEXBAR_TEST_CHILD_PID_FILE"], "w") as handle:
122+
handle.write(str(child.pid))
123+
time.sleep(30)
124+
"""
125+
126+
await #expect(throws: SubprocessRunnerError.self) {
127+
try await SubprocessRunner.run(
128+
binary: "/usr/bin/python3",
129+
arguments: ["-c", script],
130+
environment: environment,
131+
timeout: 0.5,
132+
label: "escaped-descendant")
133+
}
134+
135+
let text = try String(contentsOf: childPIDFile, encoding: .utf8)
136+
let childPID = try #require(pid_t(text.trimmingCharacters(in: .whitespacesAndNewlines)))
137+
defer { _ = kill(childPID, SIGKILL) }
138+
139+
let deadline = Date().addingTimeInterval(1)
140+
while kill(childPID, 0) == 0, Date() < deadline {
141+
try await Task.sleep(for: .milliseconds(20))
142+
}
143+
#expect(kill(childPID, 0) == -1)
144+
}
145+
101146
/// Multiple concurrent hung subprocesses must all time out independently, proving that
102147
/// one blocked subprocess does not starve the timeout mechanism of others.
103148
/// This is the core scenario that caused the original permanent-refresh-stall bug.

0 commit comments

Comments
 (0)