Repository navigation
Fix main-thread freeze during SSH paste detection #15113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
a8030e2
test: cover stalled SSH paste target detection
austinywang fd47d6a
fix: keep SSH paste detection off the main actor
austinywang d839b3d
test: make stalled detector regression deterministic
austinywang 7e61675
fix: restore composer attachment result
austinywang 7fc5e47
fix: revalidate paste surface after detection
austinywang 3438d7f
fix: keep paste task captures valid for Swift
austinywang 00377d4
fix: bound concurrent SSH target lookups
austinywang 8d01fb1
fix: validate paste target generations
austinywang 247170d
fix: import Foundation for detection timeout
austinywang 3f60016
fix: clear Swift warning budget findings
austinywang 34b95f5
fix: address async detection review findings
austinywang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import Foundation | ||
|
|
||
| /// Owns the one-shot result of a bounded SSH process lookup. | ||
| actor TerminalSSHSessionDetectionTimeoutGate { | ||
| private var completed = false | ||
| private var hasPendingResult = false | ||
| private var pendingResult: DetectedSSHSession? | ||
| private var continuation: CheckedContinuation<DetectedSSHSession?, Never>? | ||
| private var worker: Task<Void, Never>? | ||
| private var timeoutTask: Task<Void, Never>? | ||
|
|
||
| func install( | ||
| continuation: CheckedContinuation<DetectedSSHSession?, Never>, | ||
| worker: Task<Void, Never>, | ||
| timeoutTask: Task<Void, Never> | ||
| ) { | ||
| guard !completed else { | ||
| worker.cancel() | ||
| timeoutTask.cancel() | ||
| continuation.resume( | ||
| returning: hasPendingResult ? pendingResult : nil | ||
| ) | ||
| return | ||
| } | ||
| self.continuation = continuation | ||
| self.worker = worker | ||
| self.timeoutTask = timeoutTask | ||
| } | ||
|
|
||
| func finish(_ result: DetectedSSHSession?) { | ||
| guard !completed else { return } | ||
| completed = true | ||
| worker?.cancel() | ||
| timeoutTask?.cancel() | ||
| if let continuation { | ||
| continuation.resume(returning: result) | ||
| } else { | ||
| hasPendingResult = true | ||
| pendingResult = result | ||
| } | ||
| self.continuation = nil | ||
| worker = nil | ||
| timeoutTask = nil | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import Foundation | ||
|
|
||
| extension TerminalSSHSessionDetector { | ||
| static let defaultDetectionTimeout: TimeInterval = 0.25 | ||
|
|
||
| static func defaultDetectionTimeoutSleep(_ timeout: TimeInterval) async { | ||
| let milliseconds = Int64(max(0, timeout) * 1_000) | ||
| try? await ContinuousClock().sleep( | ||
| for: .milliseconds(milliseconds) | ||
| ) | ||
| } | ||
|
|
||
| /// Resolves an ad-hoc SSH session without making the caller wait for a | ||
| /// process that may be stuck reading another process's memory. | ||
| /// | ||
| /// The synchronous detector runs on a detached worker. The deadline task | ||
| /// completes the request independently; a worker that is wedged in | ||
| /// `KERN_PROCARGS2` is intentionally abandoned and its late result is | ||
| /// ignored. | ||
| #if compiler(>=6.2) | ||
| @concurrent | ||
| #else | ||
| @Sendable | ||
| #endif | ||
| static func detectAsync( | ||
| forTTY ttyName: String, | ||
| timeout: TimeInterval = defaultDetectionTimeout, | ||
| detector: @escaping @Sendable (String) -> DetectedSSHSession? = { tty in | ||
| detect(forTTY: tty) | ||
| }, | ||
| timeoutSleep: @escaping @Sendable (TimeInterval) async -> Void = { | ||
| timeout in | ||
| await TerminalSSHSessionDetector | ||
| .defaultDetectionTimeoutSleep(timeout) | ||
| } | ||
| ) async -> DetectedSSHSession? { | ||
| let gate = TerminalSSHSessionDetectionTimeoutGate() | ||
| return await withTaskCancellationHandler { | ||
| await withCheckedContinuation { continuation in | ||
| let worker = Task.detached(priority: .utility) { | ||
| await gate.finish(detector(ttyName)) | ||
| } | ||
| let timeoutTask = Task.detached(priority: .utility) { | ||
| await timeoutSleep(timeout) | ||
| await gate.finish(nil) | ||
| } | ||
| Task { | ||
| await gate.install( | ||
| continuation: continuation, | ||
| worker: worker, | ||
| timeoutTask: timeoutTask | ||
| ) | ||
| } | ||
| } | ||
| } onCancel: { | ||
| Task { await gate.finish(nil) } | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import Darwin | ||
|
|
||
| extension TerminalSSHSessionDetector { | ||
| static func processSnapshots(forTTY ttyName: String) -> [ProcessSnapshot] { | ||
| guard let ttyDevice = CmuxTopProcessSnapshot.deviceIdentifier(forTTYName: ttyName), | ||
| ttyDevice <= Int64(UInt32.max) else { | ||
| return [] | ||
| } | ||
|
|
||
| var capacity = 64 | ||
| while capacity <= 4096 { | ||
| var pids = [pid_t](repeating: 0, count: capacity) | ||
| let byteCount = pids.withUnsafeMutableBytes { rawBuffer in | ||
| proc_listpids( | ||
| UInt32(PROC_TTY_ONLY), | ||
| UInt32(ttyDevice), | ||
| rawBuffer.baseAddress, | ||
| Int32(rawBuffer.count) | ||
| ) | ||
| } | ||
| guard byteCount > 0 else { return [] } | ||
| let count = min(Int(byteCount) / MemoryLayout<pid_t>.stride, pids.count) | ||
| if byteCount < Int32(pids.count * MemoryLayout<pid_t>.stride) { | ||
| return pids.prefix(count).compactMap { pid in | ||
| processSnapshot( | ||
| for: pid, | ||
| ttyName: ttyName, | ||
| ttyDevice: UInt32(ttyDevice) | ||
| ) | ||
| } | ||
| } | ||
| if capacity == 4096 { | ||
| return pids.compactMap { pid in | ||
| processSnapshot( | ||
| for: pid, | ||
| ttyName: ttyName, | ||
| ttyDevice: UInt32(ttyDevice) | ||
| ) | ||
| } | ||
| } | ||
| capacity *= 2 | ||
| } | ||
| return [] | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| private static func processSnapshot( | ||
| for pid: pid_t, | ||
| ttyName: String, | ||
| ttyDevice: UInt32 | ||
| ) -> ProcessSnapshot? { | ||
| guard pid > 0 else { return nil } | ||
| var info = proc_bsdinfo() | ||
| let expectedSize = MemoryLayout<proc_bsdinfo>.stride | ||
| let size = proc_pidinfo( | ||
| pid, | ||
| PROC_PIDTBSDINFO, | ||
| 0, | ||
| &info, | ||
| Int32(expectedSize) | ||
| ) | ||
| guard size == expectedSize, | ||
| info.e_tdev == ttyDevice, | ||
| info.e_tpgid > 0, | ||
| info.pbi_pgid > 0 else { | ||
| return nil | ||
| } | ||
| return ProcessSnapshot( | ||
| pid: pid, | ||
| pgid: Int32(info.pbi_pgid), | ||
| tpgid: Int32(info.e_tpgid), | ||
| tty: ttyName, | ||
| executableName: CmuxTopProcessSnapshot | ||
| .fixedString(info.pbi_comm) | ||
| .lowercased() | ||
| ) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.