Skip to content

Commit aa4a3fd

Browse files
committed
Ensure LSPs Exit In Reasonable Time On Quit
1 parent bcd5f11 commit aa4a3fd

File tree

7 files changed

+246
-34
lines changed

7 files changed

+246
-34
lines changed

CodeEdit/AppDelegate.swift

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate, ObservableObject {
121121
}
122122
}
123123

124+
// MARK: - Should Terminate
125+
124126
/// Defers the application terminate message until we've finished cleanup.
125127
///
126128
/// All paths _must_ call `NSApplication.shared.reply(toApplicationShouldTerminate: true)` as soon as possible.
@@ -255,20 +257,56 @@ final class AppDelegate: NSObject, NSApplicationDelegate, ObservableObject {
255257

256258
/// Terminates running language servers. Used during app termination to ensure resources are freed.
257259
private func terminateLanguageServers() {
258-
Task {
259-
await lspService.stopAllServers()
260-
await MainActor.run {
261-
NSApplication.shared.reply(toApplicationShouldTerminate: true)
260+
Task { @MainActor in
261+
let task = TaskNotificationModel(
262+
id: "appdelegate.terminate_language_servers",
263+
title: "Stopping Language Servers",
264+
message: "Stopping running language server processes...",
265+
isLoading: true
266+
)
267+
268+
if !lspService.languageClients.isEmpty {
269+
TaskNotificationHandler.postTask(action: .create, model: task)
262270
}
271+
272+
try await withTimeout(
273+
duration: .seconds(5.0),
274+
onTimeout: {
275+
// Stop-gap measure to ensure we don't hang on CMD-Q
276+
await self.lspService.killAllServers()
277+
},
278+
operation: {
279+
await self.lspService.stopAllServers()
280+
}
281+
)
282+
283+
TaskNotificationHandler.postTask(action: .delete, model: task)
284+
NSApplication.shared.reply(toApplicationShouldTerminate: true)
263285
}
264286
}
265287

266288
/// Terminates all running tasks. Used during app termination to ensure resources are freed.
267289
private func terminateTasks() {
268-
let documents = CodeEditDocumentController.shared.documents.compactMap({ $0 as? WorkspaceDocument })
269-
documents.forEach { workspace in
270-
workspace.taskManager?.stopAllTasks()
290+
let task = TaskNotificationModel(
291+
id: "appdelegate.terminate_tasks",
292+
title: "Terminating Tasks",
293+
message: "Interrupting all running tasks before quitting...",
294+
isLoading: true
295+
)
296+
297+
let taskManagers = CodeEditDocumentController.shared.documents
298+
.compactMap({ $0 as? WorkspaceDocument })
299+
.compactMap({ $0.taskManager })
300+
301+
if taskManagers.reduce(0, { $0 + $1.activeTasks.count }) > 0 {
302+
TaskNotificationHandler.postTask(action: .create, model: task)
271303
}
304+
305+
taskManagers.forEach { manager in
306+
manager.stopAllTasks()
307+
}
308+
309+
TaskNotificationHandler.postTask(action: .delete, model: task)
272310
}
273311
}
274312

CodeEdit/Features/ActivityViewer/Notifications/TaskNotificationHandler.swift

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,28 +25,35 @@ import Combine
2525
/// Remember to manage your task notifications appropriately. You should either delete task
2626
/// notifications manually or schedule their deletion in advance using the `deleteWithDelay` method.
2727
///
28+
/// Some tasks should be restricted to a specific workspace. To do this, specify the `workspace` attribute in the
29+
/// notification's `userInfo` dictionary as a `URL`, or use the `toWorkspace` parameter on
30+
/// ``TaskNotificationHandler/postTask(toWorkspace:action:model:)``.
31+
///
2832
/// ## Available Methods
2933
/// - `create`:
3034
/// Creates a new Task Notification.
3135
/// Required fields: `id` (String), `action` (String), `title` (String).
32-
/// Optional fields: `message` (String), `percentage` (Double), `isLoading` (Bool).
36+
/// Optional fields: `message` (String), `percentage` (Double), `isLoading` (Bool), `workspace` (URL).
3337
/// - `createWithPriority`:
3438
/// Creates a new Task Notification and inserts it at the start of the array.
3539
/// This ensures it appears in the activity viewer even if there are other task notifications before it.
3640
/// **Note:** This should only be used for important notifications!
3741
/// Required fields: `id` (String), `action` (String), `title` (String).
38-
/// Optional fields: `message` (String), `percentage` (Double), `isLoading` (Bool).
42+
/// Optional fields: `message` (String), `percentage` (Double), `isLoading` (Bool), `workspace` (URL).
3943
/// - `update`:
4044
/// Updates an existing task notification. It's important to pass the same `id` to update the correct task.
4145
/// Required fields: `id` (String), `action` (String).
42-
/// Optional fields: `title` (String), `message` (String), `percentage` (Double), `isLoading` (Bool).
46+
/// Optional fields: `title` (String), `message` (String), `percentage` (Double), `isLoading` (Bool),
47+
/// `workspace` (URL).
4348
/// - `delete`:
4449
/// Deletes an existing task notification.
4550
/// Required fields: `id` (String), `action` (String).
51+
/// Optional field: `workspace` (URL).
4652
/// - `deleteWithDelay`:
4753
/// Deletes an existing task notification after a certain `TimeInterval`.
4854
/// Required fields: `id` (String), `action` (String), `delay` (Double).
49-
/// **Important:** When specifying the delay, ensure it's a double.
55+
/// Optional field: `workspace` (URL).
56+
/// **Important:** When specifying the delay, ensure it's a double.
5057
/// For example, '2' would be invalid because it would count as an integer, use '2.0' instead.
5158
///
5259
/// ## Example Usage:
@@ -101,13 +108,46 @@ import Combine
101108
/// }
102109
/// ```
103110
///
111+
/// You can also use the static helper method instead of creating dictionaries manually:
112+
/// ```swift
113+
/// TaskNotificationHandler.postTask(action: .create, model: .init(id: "task_id", "title": "New Task"))
114+
/// ```
115+
///
104116
/// - Important: Please refer to ``CodeEdit/TaskNotificationModel`` and ensure you pass the correct values.
105117
final class TaskNotificationHandler: ObservableObject {
106118
@Published private(set) var notifications: [TaskNotificationModel] = []
119+
var workspaceURL: URL?
107120
var cancellables: Set<AnyCancellable> = []
108121

122+
enum Action: String {
123+
case create
124+
case createWithPriority
125+
case update
126+
case delete
127+
case deleteWithDelay
128+
}
129+
130+
/// Post a new task.
131+
/// - Parameters:
132+
/// - toWorkspace: The workspace to restrict the task to. Defaults to `nil`, which is received by all workspaces.
133+
/// - action: The action being taken on the task.
134+
/// - model: The task contents.
135+
static func postTask(toWorkspace: URL? = nil, action: Action, model: TaskNotificationModel) {
136+
NotificationCenter.default.post(name: .taskNotification, object: nil, userInfo: [
137+
"id": model.id,
138+
"title": model.title,
139+
"message": model.message as Any,
140+
"percentage": model.percentage as Any,
141+
"isLoading": model.isLoading,
142+
"action": action.rawValue,
143+
"workspace": toWorkspace as Any
144+
])
145+
}
146+
109147
/// Initialises a new `TaskNotificationHandler` and starts observing for task notifications.
110-
init() {
148+
init(workspaceURL: URL? = nil) {
149+
self.workspaceURL = workspaceURL
150+
111151
NotificationCenter.default
112152
.publisher(for: .taskNotification)
113153
.receive(on: DispatchQueue.main)
@@ -127,21 +167,25 @@ final class TaskNotificationHandler: ObservableObject {
127167
private func handleNotification(_ notification: Notification) {
128168
guard let userInfo = notification.userInfo,
129169
let taskID = userInfo["id"] as? String,
130-
let action = userInfo["action"] as? String else { return }
170+
let actionRaw = userInfo["action"] as? String,
171+
let action = Action(rawValue: actionRaw) else { return }
172+
173+
/// If a workspace is specified, don't do anything with this task.
174+
if let workspaceURL = userInfo["workspace"] as? URL, workspaceURL != self.workspaceURL {
175+
return
176+
}
131177

132178
switch action {
133-
case "create", "createWithPriority":
179+
case .create, .createWithPriority:
134180
createTask(task: userInfo)
135-
case "update":
181+
case .update:
136182
updateTask(task: userInfo)
137-
case "delete":
183+
case .delete:
138184
deleteTask(taskID: taskID)
139-
case "deleteWithDelay":
185+
case .deleteWithDelay:
140186
if let delay = userInfo["delay"] as? Double {
141187
deleteTaskAfterDelay(taskID: taskID, delay: delay)
142188
}
143-
default:
144-
break
145189
}
146190
}
147191

CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ final class WorkspaceDocument: NSDocument, ObservableObject, NSToolbarDelegate {
161161
workspaceURL: url
162162
)
163163
}
164+
self.taskNotificationHandler.workspaceURL = url
164165

165166
editorManager?.restoreFromState(self)
166167
utilityAreaModel?.restoreFromState(self)
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
//
2+
// DataChannel+localProcess.swift
3+
// CodeEdit
4+
//
5+
// Created by Khan Winter on 7/8/25.
6+
//
7+
8+
import Foundation
9+
import LanguageServerProtocol
10+
import JSONRPC
11+
import ProcessEnv
12+
13+
// This is almost exactly the same as the extension provided by `LanguageServerProtocol`, except it returns the
14+
// process ID as well as the channel, which we need.
15+
16+
extension DataChannel {
17+
@available(macOS 12.0, *)
18+
public static func localProcessChannel(
19+
parameters: Process.ExecutionParameters,
20+
terminationHandler: @escaping @Sendable () -> Void
21+
) throws -> (pid: pid_t, channel: DataChannel) {
22+
let process = Process()
23+
24+
let stdinPipe = Pipe()
25+
let stdoutPipe = Pipe()
26+
let stderrPipe = Pipe()
27+
28+
process.standardInput = stdinPipe
29+
process.standardOutput = stdoutPipe
30+
process.standardError = stderrPipe
31+
32+
process.parameters = parameters
33+
34+
let (stream, continuation) = DataSequence.makeStream()
35+
36+
process.terminationHandler = { _ in
37+
continuation.finish()
38+
terminationHandler()
39+
}
40+
41+
Task {
42+
let dataStream = stdoutPipe.fileHandleForReading.dataStream
43+
44+
for try await data in dataStream {
45+
continuation.yield(data)
46+
}
47+
48+
continuation.finish()
49+
}
50+
51+
Task {
52+
for try await line in stderrPipe.fileHandleForReading.bytes.lines {
53+
print("stderr: ", line)
54+
}
55+
}
56+
57+
try process.run()
58+
59+
let handler: DataChannel.WriteHandler = {
60+
// this is wacky, but we need the channel to hold a strong reference to the process
61+
// to prevent it from being deallocated
62+
_ = process
63+
64+
try stdinPipe.fileHandleForWriting.write(contentsOf: $0)
65+
}
66+
67+
return (process.processIdentifier, DataChannel(writeHandler: handler, dataSequence: stream))
68+
}
69+
}

CodeEdit/Features/LSP/LanguageServer/LanguageServer.swift

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,21 @@ class LanguageServer<DocumentType: LanguageServerDocument> {
4040
private(set) var lspInstance: InitializingServer
4141
/// The path to the root of the project
4242
private(set) var rootPath: URL
43+
/// The PID of the running language server process.
44+
private(set) var pid: pid_t
4345

4446
init(
4547
languageId: LanguageIdentifier,
4648
binary: LanguageServerBinary,
4749
lspInstance: InitializingServer,
50+
lspPid: pid_t,
4851
serverCapabilities: ServerCapabilities,
4952
rootPath: URL
5053
) {
5154
self.languageId = languageId
5255
self.binary = binary
5356
self.lspInstance = lspInstance
57+
self.pid = lspPid
5458
self.serverCapabilities = serverCapabilities
5559
self.rootPath = rootPath
5660
self.openFiles = LanguageServerFileMap()
@@ -82,15 +86,17 @@ class LanguageServer<DocumentType: LanguageServerDocument> {
8286
environment: binary.env
8387
)
8488

89+
let (pid, connection) = try makeLocalServerConnection(languageId: languageId, executionParams: executionParams)
8590
let server = InitializingServer(
86-
server: try makeLocalServerConnection(languageId: languageId, executionParams: executionParams),
91+
server: connection,
8792
initializeParamsProvider: getInitParams(workspacePath: workspacePath)
8893
)
8994
let capabilities = try await server.initializeIfNeeded()
9095
return LanguageServer(
9196
languageId: languageId,
9297
binary: binary,
9398
lspInstance: server,
99+
lspPid: pid,
94100
serverCapabilities: capabilities,
95101
rootPath: URL(filePath: workspacePath)
96102
)
@@ -106,15 +112,15 @@ class LanguageServer<DocumentType: LanguageServerDocument> {
106112
static func makeLocalServerConnection(
107113
languageId: LanguageIdentifier,
108114
executionParams: Process.ExecutionParameters
109-
) throws -> JSONRPCServerConnection {
115+
) throws -> (pid: pid_t, connection: JSONRPCServerConnection) {
110116
do {
111-
let channel = try DataChannel.localProcessChannel(
117+
let (pid, channel) = try DataChannel.localProcessChannel(
112118
parameters: executionParams,
113119
terminationHandler: {
114120
logger.debug("Terminated data channel for \(languageId.rawValue)")
115121
}
116122
)
117-
return JSONRPCServerConnection(dataChannel: channel)
123+
return (pid, JSONRPCServerConnection(dataChannel: channel))
118124
} catch {
119125
logger.warning("Failed to initialize data channel for \(languageId.rawValue)")
120126
throw error
@@ -232,10 +238,14 @@ class LanguageServer<DocumentType: LanguageServerDocument> {
232238
// swiftlint:enable function_body_length
233239
}
234240

241+
// MARK: - Shutdown
242+
235243
/// Shuts down the language server and exits it.
236244
public func shutdown() async throws {
237245
self.logger.info("Shutting down language server")
238-
try await lspInstance.shutdownAndExit()
246+
try await withTimeout(duration: .seconds(1.0)) {
247+
try await self.lspInstance.shutdownAndExit()
248+
}
239249
}
240250
}
241251

CodeEdit/Features/LSP/Service/LSPService.swift

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -300,16 +300,13 @@ final class LSPService: ObservableObject {
300300

301301
/// Goes through all active language servers and attempts to shut them down.
302302
func stopAllServers() async {
303-
await withThrowingTaskGroup(of: Void.self) { group in
304-
for (key, server) in languageClients {
305-
group.addTask {
306-
do {
307-
try await server.shutdown()
308-
} catch {
309-
self.logger.error("Shutting down \(key.languageId.rawValue): Error \(error)")
310-
throw error
311-
}
312-
}
303+
// Note: This is no longer a task group for a *REASON*
304+
// The task group for some reason would never return from the `await` suspension point.
305+
for (key, server) in languageClients {
306+
do {
307+
try await server.shutdown()
308+
} catch {
309+
self.logger.warning("Shutting down \(key.languageId.rawValue): Error \(error)")
313310
}
314311
}
315312
languageClients.removeAll()
@@ -318,4 +315,11 @@ final class LSPService: ObservableObject {
318315
}
319316
eventListeningTasks.removeAll()
320317
}
318+
319+
/// Call this when a server is refusing to terminate itself. Sends the `SIGKILL` signal to all lsp processes.
320+
func killAllServers() {
321+
for (_, server) in languageClients {
322+
kill(server.pid, SIGKILL)
323+
}
324+
}
321325
}

0 commit comments

Comments
 (0)