diff --git a/CHANGELOG.md b/CHANGELOG.md index f3fac62..a751244 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ ### JSON-RPC - fix: let non-interactive RPC startup proceed without a Contacts prompt while rejecting ambiguous name targets when Contacts is unavailable (#186, #187, thanks @SebTardif). +### Reliability +- fix: bound osascript send, reaction, and helper-process waits with process-tree cleanup so stalled subprocesses cannot hang CLI or RPC work (#197, thanks @SebTardif). + ## 0.13.4 - 2026-07-27 ### Highlights diff --git a/Sources/IMsgCore/AttachmentResolver.swift b/Sources/IMsgCore/AttachmentResolver.swift index c033f88..5f5717d 100644 --- a/Sources/IMsgCore/AttachmentResolver.swift +++ b/Sources/IMsgCore/AttachmentResolver.swift @@ -126,7 +126,7 @@ enum AttachmentResolver { /// Default bound for external converters (ffmpeg). Hung converters must not /// block attachment metadata resolution indefinitely. - static let conversionProcessTimeout: TimeInterval = 60 + static let conversionProcessTimeout: TimeInterval = ProcessTimeout.defaultTimeout static func runConversionProcess( executableURL: URL, @@ -140,47 +140,12 @@ enum AttachmentResolver { process.standardError = FileHandle(forWritingAtPath: "/dev/null") try process.run() - - // Monotonic deadline so wall-clock jumps cannot stretch the bound. - let clock = ContinuousClock() - let bound = Duration.seconds(max(0.05, timeout)) - let deadline = clock.now + bound - while process.isRunning { - if clock.now >= deadline { - terminateConversionProcess(process) - process.waitUntilExit() - return 128 + SIGTERM - } - Thread.sleep(forTimeInterval: 0.05) + if ProcessTimeout.waitUntilExit(process, timeout: timeout) { + return 128 + SIGTERM } return process.terminationStatus } - /// SIGTERM the process, then SIGKILL process and process-group after grace. - private static func terminateConversionProcess(_ process: Process) { - let pid = process.processIdentifier - guard pid > 0 else { return } - let ownsProcessGroup = getpgid(pid) == pid - process.terminate() - if ownsProcessGroup { - kill(-pid, SIGTERM) - } - - let clock = ContinuousClock() - let killDeadline = clock.now + .milliseconds(500) - while process.isRunning, clock.now < killDeadline { - Thread.sleep(forTimeInterval: 0.02) - } - if process.isRunning { - kill(pid, SIGKILL) - } - // The leader may exit on SIGTERM while a descendant ignores it. Escalate - // the captured group independently of the leader's state. - if ownsProcessGroup { - kill(-pid, SIGKILL) - } - } - private static func conversionPlan( path: String, uti: String, diff --git a/Sources/IMsgCore/MessageSender.swift b/Sources/IMsgCore/MessageSender.swift index 29eb703..6903e53 100644 --- a/Sources/IMsgCore/MessageSender.swift +++ b/Sources/IMsgCore/MessageSender.swift @@ -319,6 +319,11 @@ public struct MessageSender { #endif } + /// Bound for osascript fallback when NSAppleScript is unauthorized. + /// Align with the send-style bridge deadline (150s): Messages can stall + /// longer than the short helper default (60s). + static let osascriptTimeout: TimeInterval = IMsgBridgeProtocol.defaultSendResponseTimeout + private static func runOsascript(source: String, arguments: [String]) throws { let process = Process() process.executableURL = URL(fileURLWithPath: "/usr/bin/osascript") @@ -332,7 +337,10 @@ public struct MessageSender { stdinPipe.fileHandleForWriting.write(data) } stdinPipe.fileHandleForWriting.closeFile() - process.waitUntilExit() + if ProcessTimeout.waitUntilExit(process, timeout: osascriptTimeout) { + throw IMsgError.appleScriptFailure( + "osascript timed out after \(Int(osascriptTimeout))s") + } if process.terminationStatus != 0 { let data = stderrPipe.fileHandleForReading.readDataToEndOfFile() let message = String(data: data, encoding: .utf8) ?? "Unknown osascript error" diff --git a/Sources/IMsgCore/MessagesLauncher.swift b/Sources/IMsgCore/MessagesLauncher.swift index beb11bb..4a5fc54 100644 --- a/Sources/IMsgCore/MessagesLauncher.swift +++ b/Sources/IMsgCore/MessagesLauncher.swift @@ -161,6 +161,10 @@ import Foundation } } + /// Bound for short helper processes (`killall`, `csrutil`). Must not hang + /// launcher/RPC setup if the helper stalls. + static let helperProcessTimeout: TimeInterval = 15 + /// Kill Messages.app if running. public func killMessages() { let task = Process() @@ -169,7 +173,7 @@ import Foundation task.standardOutput = FileHandle.nullDevice task.standardError = FileHandle.nullDevice try? task.run() - task.waitUntilExit() + _ = ProcessTimeout.waitUntilExit(task, timeout: Self.helperProcessTimeout) } /// Send a command asynchronously. @@ -224,7 +228,9 @@ import Foundation } catch { return nil } - task.waitUntilExit() + if ProcessTimeout.waitUntilExit(task, timeout: helperProcessTimeout) { + return nil + } let data = output.fileHandleForReading.readDataToEndOfFile() guard let text = String(data: data, encoding: .utf8) else { return nil } return text.trimmingCharacters(in: .whitespacesAndNewlines) diff --git a/Sources/IMsgCore/ProcessTimeout.swift b/Sources/IMsgCore/ProcessTimeout.swift new file mode 100644 index 0000000..15c3ecc --- /dev/null +++ b/Sources/IMsgCore/ProcessTimeout.swift @@ -0,0 +1,61 @@ +import Foundation + +#if canImport(Darwin) + import Darwin +#elseif canImport(Glibc) + import Glibc +#endif + +/// Bounded waits for external processes (ffmpeg, osascript, …). +/// Hung children must not block CLI/RPC work indefinitely. +public enum ProcessTimeout { + /// Default bound for short external helpers (converters, osascript). + public static let defaultTimeout: TimeInterval = 60 + + /// Wait for `process` to exit, or terminate it when `timeout` elapses. + /// - Returns: `true` if the process was killed because the deadline was reached. + @discardableResult + public static func waitUntilExit( + _ process: Process, + timeout: TimeInterval = defaultTimeout + ) -> Bool { + // Monotonic deadline so wall-clock jumps cannot stretch the bound. + let clock = ContinuousClock() + let bound = Duration.seconds(max(0.05, timeout)) + let deadline = clock.now + bound + while process.isRunning { + if clock.now >= deadline { + terminate(process) + process.waitUntilExit() + return true + } + Thread.sleep(forTimeInterval: 0.05) + } + return false + } + + /// SIGTERM the process, then SIGKILL process and process-group after grace. + public static func terminate(_ process: Process) { + let pid = process.processIdentifier + guard pid > 0 else { return } + let ownsProcessGroup = getpgid(pid) == pid + process.terminate() + if ownsProcessGroup { + kill(-pid, SIGTERM) + } + + let clock = ContinuousClock() + let killDeadline = clock.now + .milliseconds(500) + while process.isRunning, clock.now < killDeadline { + Thread.sleep(forTimeInterval: 0.02) + } + if process.isRunning { + kill(pid, SIGKILL) + } + // The leader may exit on SIGTERM while a descendant ignores it. Escalate + // the captured group independently of the leader's state. + if ownsProcessGroup { + kill(-pid, SIGKILL) + } + } +} diff --git a/Sources/imsg/Commands/ReactCommand.swift b/Sources/imsg/Commands/ReactCommand.swift index 0765e43..d192c83 100644 --- a/Sources/imsg/Commands/ReactCommand.swift +++ b/Sources/imsg/Commands/ReactCommand.swift @@ -170,6 +170,10 @@ enum ReactCommand { return scalar.properties.isEmoji || scalar.properties.isEmojiPresentation } + /// Bound for react UI automation. Align with the send-style deadline (150s) + /// used on main for reaction waits; hung osascript still cannot block forever. + static let osascriptTimeout: TimeInterval = IMsgBridgeProtocol.defaultSendResponseTimeout + private static func runAppleScript(_ source: String, arguments: [String]) throws { let process = Process() process.executableURL = URL(fileURLWithPath: "/usr/bin/osascript") @@ -185,7 +189,10 @@ enum ReactCommand { stdinPipe.fileHandleForWriting.write(data) } stdinPipe.fileHandleForWriting.closeFile() - process.waitUntilExit() + if ProcessTimeout.waitUntilExit(process, timeout: osascriptTimeout) { + throw IMsgError.appleScriptFailure( + "osascript timed out after \(Int(osascriptTimeout))s") + } if process.terminationStatus != 0 { let data = stderrPipe.fileHandleForReading.readDataToEndOfFile() diff --git a/Tests/IMsgCoreTests/ProcessTimeoutTests.swift b/Tests/IMsgCoreTests/ProcessTimeoutTests.swift new file mode 100644 index 0000000..05fea37 --- /dev/null +++ b/Tests/IMsgCoreTests/ProcessTimeoutTests.swift @@ -0,0 +1,95 @@ +import Darwin +import Foundation +import Testing + +@testable import IMsgCore + +@Test +func processTimeoutKillsHungProcess() throws { + let dir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + + // `exec` replaces the shell with sleep so the Process PID is the sleeper + // itself (no orphan child after we kill the converter PID). + let hung = dir.appendingPathComponent("sleeper") + try """ + #!/bin/sh + exec sleep 30 + """.write(to: hung, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: hung.path) + + let process = Process() + process.executableURL = hung + process.arguments = [] + process.standardOutput = FileHandle(forWritingAtPath: "/dev/null") + process.standardError = FileHandle(forWritingAtPath: "/dev/null") + + try process.run() + let clock = ContinuousClock() + let start = clock.now + let timedOut = ProcessTimeout.waitUntilExit(process, timeout: 0.4) + let elapsed = start.duration(to: clock.now) + + #expect(timedOut) + #expect(!process.isRunning) + #expect(elapsed < .seconds(5)) + #expect(elapsed >= .milliseconds(300)) +} + +@Test +func processTimeoutAllowsQuickExit() throws { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/true") + process.arguments = [] + process.standardOutput = FileHandle(forWritingAtPath: "/dev/null") + process.standardError = FileHandle(forWritingAtPath: "/dev/null") + + try process.run() + let timedOut = ProcessTimeout.waitUntilExit(process, timeout: 5) + #expect(!timedOut) + #expect(process.terminationStatus == 0) +} + +@Test +func processTimeoutReapsHungOsascript() throws { + // Same launch shape as MessageSender.runOsascript / ReactCommand.runAppleScript: + // /usr/bin/osascript -l AppleScript - with source on stdin. + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/osascript") + process.arguments = ["-l", "AppleScript", "-"] + let stdinPipe = Pipe() + process.standardInput = stdinPipe + process.standardOutput = FileHandle(forWritingAtPath: "/dev/null") + process.standardError = FileHandle(forWritingAtPath: "/dev/null") + + try process.run() + if let data = "delay 30\n".data(using: .utf8) { + stdinPipe.fileHandleForWriting.write(data) + } + stdinPipe.fileHandleForWriting.closeFile() + + let clock = ContinuousClock() + let start = clock.now + let timedOut = ProcessTimeout.waitUntilExit(process, timeout: 0.6) + let elapsed = start.duration(to: clock.now) + + #expect(timedOut) + #expect(!process.isRunning) + #expect(elapsed < .seconds(5)) + #expect(elapsed >= .milliseconds(400)) +} + +@Test +func processTimeoutAllowsCsrutilStatus() throws { + let task = Process() + let output = Pipe() + task.executableURL = URL(fileURLWithPath: "/usr/bin/csrutil") + task.arguments = ["status"] + task.standardOutput = output + task.standardError = output + try task.run() + let timedOut = ProcessTimeout.waitUntilExit(task, timeout: MessagesLauncher.helperProcessTimeout) + #expect(!timedOut) + #expect(!task.isRunning) +}