Skip to content

Commit 334357f

Browse files
Ensure LSPs Exit, Add Exit Activities (#2080)
### Description LSPs may refuse to exit (due to a bug or whatnot) but we need to ensure that CodeEdit can still close when quit. This adds both a timeout when language servers are stopped, and a method to send `SIGKILL` to language servers if necessary. This can only cause a delay while quitting CodeEdit. To help with the UX here, I've added some activity notifications that tell the user why we're delaying quitting CodeEdit. Both task termination and language server stopping are displayed while CodeEdit attempts to quit. #### Detailed Changes - Updated the `LanguageClient` package to use a new method that lets us grab the started language server process. - The `pid_t` of the process is stored on `LanguageServer` for future use. - Updated `LSPService.stopServer` to not use a throwing task group. For some reason the task group refused to continue even if the language server did close correctly. A non-throwing task group works correctly. - Added `LSPService.killAllServers` to send `SIGKILL` to all language servers if necessary. - Updated `AppDelegate` to send activity notifications when delaying quitting the app. - Updated `AppDelegate` to timeout language server stopping and kill them if necessary. - Added a little helper to `TaskNotificationHandler` to help with sending tasks. Updated docs to match. - Added a new parameter to `TaskNotificationHandler` to limit activities to a single workspace. If left out, does not limit the workspace. - Because of the new workspace parameter, I quickly limited tasks to only notify their workspaces. - I also added a second ID to `CEActiveTask` to differentiate between task runs (see screen recording). ### Related Issues * related #1976 ### Checklist - [x] I read and understood the [contributing guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md) as well as the [code of conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md) - [x] The issues this PR addresses are related to each other - [x] My changes generate no new warnings - [x] My code builds and runs on my machine - [x] My changes are all related to the related issue above - [x] I documented my code ### Screenshots Before change. Tasks show up in all workspaces, cancelled tasks remove new task activities. https://github.com/user-attachments/assets/7a4f390d-5918-450c-afc7-ec164d7bb1e3 After the change. Active tasks have their own ID and cancelled tasks don't remove new task activities. https://github.com/user-attachments/assets/5502f0b6-b34c-4c24-9ae4-f6eab3fc7100
1 parent 4ce9be0 commit 334357f

File tree

11 files changed

+215
-49
lines changed

11 files changed

+215
-49
lines changed

CodeEdit.xcodeproj/project.pbxproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1662,7 +1662,7 @@
16621662
repositoryURL = "https://github.com/ChimeHQ/LanguageClient";
16631663
requirement = {
16641664
kind = upToNextMajorVersion;
1665-
minimumVersion = 0.8.0;
1665+
minimumVersion = 0.8.2;
16661666
};
16671667
};
16681668
303E88462C276FD600EEA8D9 /* XCRemoteSwiftPackageReference "LanguageServerProtocol" */ = {

CodeEdit.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

Lines changed: 13 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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 and doesn't match, 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
@@ -163,6 +163,7 @@ final class WorkspaceDocument: NSDocument, ObservableObject, NSToolbarDelegate {
163163
workspaceURL: url
164164
)
165165
}
166+
self.taskNotificationHandler.workspaceURL = url
166167

167168
editorManager?.restoreFromState(self)
168169
utilityAreaModel?.restoreFromState(self)

CodeEdit/Features/LSP/LanguageServer/LanguageServer.swift

Lines changed: 21 additions & 7 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,16 +86,22 @@ class LanguageServer<DocumentType: LanguageServerDocument> {
8286
environment: binary.env
8387
)
8488

89+
let (connection, process) = try makeLocalServerConnection(
90+
languageId: languageId,
91+
executionParams: executionParams
92+
)
8593
let server = InitializingServer(
86-
server: try makeLocalServerConnection(languageId: languageId, executionParams: executionParams),
94+
server: connection,
8795
initializeParamsProvider: getInitParams(workspacePath: workspacePath)
8896
)
89-
let capabilities = try await server.initializeIfNeeded()
97+
let initializationResponse = try await server.initializeIfNeeded()
98+
9099
return LanguageServer(
91100
languageId: languageId,
92101
binary: binary,
93102
lspInstance: server,
94-
serverCapabilities: capabilities,
103+
lspPid: process.processIdentifier,
104+
serverCapabilities: initializationResponse.capabilities,
95105
rootPath: URL(filePath: workspacePath)
96106
)
97107
}
@@ -106,15 +116,15 @@ class LanguageServer<DocumentType: LanguageServerDocument> {
106116
static func makeLocalServerConnection(
107117
languageId: LanguageIdentifier,
108118
executionParams: Process.ExecutionParameters
109-
) throws -> JSONRPCServerConnection {
119+
) throws -> (connection: JSONRPCServerConnection, process: Process) {
110120
do {
111-
let channel = try DataChannel.localProcessChannel(
121+
let (channel, process) = try DataChannel.localProcessChannel(
112122
parameters: executionParams,
113123
terminationHandler: {
114124
logger.debug("Terminated data channel for \(languageId.rawValue)")
115125
}
116126
)
117-
return JSONRPCServerConnection(dataChannel: channel)
127+
return (JSONRPCServerConnection(dataChannel: channel), process)
118128
} catch {
119129
logger.warning("Failed to initialize data channel for \(languageId.rawValue)")
120130
throw error
@@ -232,10 +242,14 @@ class LanguageServer<DocumentType: LanguageServerDocument> {
232242
// swiftlint:enable function_body_length
233243
}
234244

245+
// MARK: - Shutdown
246+
235247
/// Shuts down the language server and exits it.
236248
public func shutdown() async throws {
237249
self.logger.info("Shutting down language server")
238-
try await lspInstance.shutdownAndExit()
250+
try await withTimeout(duration: .seconds(1.0)) {
251+
try await self.lspInstance.shutdownAndExit()
252+
}
239253
}
240254
}
241255

CodeEdit/Features/LSP/Service/LSPService.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,14 +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
303+
await withTaskGroup(of: Void.self) { group in
304304
for (key, server) in languageClients {
305305
group.addTask {
306306
do {
307307
try await server.shutdown()
308308
} catch {
309-
self.logger.error("Shutting down \(key.languageId.rawValue): Error \(error)")
310-
throw error
309+
self.logger.warning("Shutting down \(key.languageId.rawValue): Error \(error)")
311310
}
312311
}
313312
}
@@ -318,4 +317,11 @@ final class LSPService: ObservableObject {
318317
}
319318
eventListeningTasks.removeAll()
320319
}
320+
321+
/// Call this when a server is refusing to terminate itself. Sends the `SIGKILL` signal to all lsp processes.
322+
func killAllServers() {
323+
for (_, server) in languageClients {
324+
kill(server.pid, SIGKILL)
325+
}
326+
}
321327
}

0 commit comments

Comments
 (0)