Skip to content

Commit ff32570

Browse files
committed
Show an indicator in the client if sourcekitd has crashed
This allows us to prompt the user to file an issue. It’s also useful for developers of sourcekit-lsp to indicate why semantic functionality is limited. rdar://123391260
1 parent 884b1f5 commit ff32570

File tree

4 files changed

+104
-28
lines changed

4 files changed

+104
-28
lines changed

Sources/SourceKitLSP/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ add_library(SourceKitLSP STATIC
1212
SourceKitServer+Options.swift
1313
TestDiscovery.swift
1414
ToolchainLanguageServer.swift
15+
WorkDoneProgressManager.swift
1516
Workspace.swift
1617
)
1718
target_sources(SourceKitLSP PRIVATE

Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,31 @@ public actor SwiftLanguageServer: ToolchainLanguageServer {
127127
nonisolated var requests: sourcekitd_api_requests { return sourcekitd.requests }
128128
nonisolated var values: sourcekitd_api_values { return sourcekitd.values }
129129

130+
/// When sourcekitd is crashed, a `WorkDoneProgressManager` that display the sourcekitd crash status in the client.
131+
private var sourcekitdCrashedWorkDoneProgress: WorkDoneProgressManager?
132+
130133
private var state: LanguageServerState {
131134
didSet {
132135
for handler in stateChangeHandlers {
133136
handler(oldValue, state)
134137
}
138+
139+
guard let sourceKitServer else {
140+
sourcekitdCrashedWorkDoneProgress = nil
141+
return
142+
}
143+
switch state {
144+
case .connected:
145+
sourcekitdCrashedWorkDoneProgress = nil
146+
case .connectionInterrupted, .semanticFunctionalityDisabled:
147+
if sourcekitdCrashedWorkDoneProgress == nil {
148+
sourcekitdCrashedWorkDoneProgress = WorkDoneProgressManager(
149+
server: sourceKitServer,
150+
title: "SourceKit-LSP: Restoring functionality",
151+
message: "Please run 'sourcekit-lsp diagnose' to file an issue"
152+
)
153+
}
154+
}
135155
}
136156
}
137157

@@ -149,7 +169,7 @@ public actor SwiftLanguageServer: ToolchainLanguageServer {
149169

150170
/// Creates a language server for the given client using the sourcekitd dylib specified in `toolchain`.
151171
/// `reopenDocuments` is a closure that will be called if sourcekitd crashes and the `SwiftLanguageServer` asks its parent server to reopen all of its documents.
152-
/// Returns `nil` if `sourcektid` couldn't be found.
172+
/// Returns `nil` if `sourcekitd` couldn't be found.
153173
public init?(
154174
sourceKitServer: SourceKitServer,
155175
toolchain: Toolchain,
@@ -885,6 +905,12 @@ extension SwiftLanguageServer: SKDNotificationHandler {
885905
}
886906

887907
private func notificationImpl(_ notification: SKDResponse) async {
908+
logger.debug(
909+
"""
910+
Received notification from sourcekitd
911+
\(notification.forLogging)
912+
"""
913+
)
888914
// Check if we need to update our `state` based on the contents of the notification.
889915
if notification.value?[self.keys.notification] == self.values.semaEnabledNotification {
890916
self.state = .connected
@@ -910,13 +936,6 @@ extension SwiftLanguageServer: SKDNotificationHandler {
910936
// Reset the document manager to reflect that.
911937
self.documentManager = DocumentManager()
912938
}
913-
914-
logger.debug(
915-
"""
916-
Received notification from sourcekitd
917-
\(notification.forLogging)
918-
"""
919-
)
920939
}
921940
}
922941

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import Foundation
14+
import LanguageServerProtocol
15+
import SKSupport
16+
17+
/// Represents a single `WorkDoneProgress` task that gets communicated with the client.
18+
///
19+
/// The work done progress is started when the object is created and ended when the object is destroyed.
20+
/// In between, updates can be sent to the client.
21+
final class WorkDoneProgressManager {
22+
private let token: ProgressToken
23+
private let queue = AsyncQueue<Serial>()
24+
private let server: SourceKitServer
25+
26+
init(server: SourceKitServer, title: String, message: String? = nil) {
27+
self.token = .string("WorkDoneProgress-\(UUID())")
28+
self.server = server
29+
queue.async { [server, token] in
30+
await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
31+
_ = server.client.send(CreateWorkDoneProgressRequest(token: token)) { result in
32+
continuation.resume()
33+
}
34+
}
35+
await server.sendNotificationToClient(
36+
WorkDoneProgress(token: token, value: .begin(WorkDoneProgressBegin(title: title, message: message)))
37+
)
38+
}
39+
}
40+
41+
func update(message: String? = nil, percentage: Int? = nil) {
42+
queue.async { [server, token] in
43+
await server.sendNotificationToClient(
44+
WorkDoneProgress(
45+
token: token,
46+
value: .report(WorkDoneProgressReport(cancellable: false, message: message, percentage: percentage))
47+
)
48+
)
49+
}
50+
}
51+
52+
deinit {
53+
queue.async { [server, token] in
54+
await server.sendNotificationToClient(WorkDoneProgress(token: token, value: .end(WorkDoneProgressEnd())))
55+
}
56+
}
57+
}

Tests/SourceKitDTests/CrashRecoveryTests.swift

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ final class CrashRecoveryTests: XCTestCase {
7272
"Sanity check failed. The Hover response did not contain foo(), even before crashing sourcekitd. Received response: \(String(describing: preCrashHoverResponse))"
7373
)
7474

75+
testClient.handleNextRequest { (request: CreateWorkDoneProgressRequest) -> VoidResponse in
76+
return VoidResponse()
77+
}
78+
7579
// Crash sourcekitd
7680

7781
let sourcekitdServer =
@@ -81,33 +85,28 @@ final class CrashRecoveryTests: XCTestCase {
8185
in: testClient.server.workspaceForDocument(uri: uri)!
8286
) as! SwiftLanguageServer
8387

84-
let sourcekitdCrashed = expectation(description: "sourcekitd has crashed")
85-
let sourcekitdRestarted = expectation(description: "sourcekitd has been restarted (syntactic only)")
86-
let semanticFunctionalityRestored = expectation(
87-
description: "sourcekitd has restored semantic language functionality"
88-
)
89-
90-
await sourcekitdServer.addStateChangeHandler { (oldState, newState) in
91-
switch newState {
92-
case .connectionInterrupted:
93-
sourcekitdCrashed.fulfill()
94-
case .semanticFunctionalityDisabled:
95-
sourcekitdRestarted.fulfill()
96-
case .connected:
97-
semanticFunctionalityRestored.fulfill()
98-
}
99-
}
100-
10188
await sourcekitdServer._crash()
10289

103-
try await fulfillmentOfOrThrow([sourcekitdCrashed], timeout: 5)
104-
try await fulfillmentOfOrThrow([sourcekitdRestarted], timeout: 30)
90+
let crashedNotification = try await testClient.nextNotification(ofType: WorkDoneProgress.self, timeout: 5)
91+
XCTAssertEqual(
92+
crashedNotification.value,
93+
.begin(
94+
WorkDoneProgressBegin(
95+
title: "SourceKit-LSP: Restoring functionality",
96+
message: "Please run 'sourcekit-lsp diagnose' to file an issue"
97+
)
98+
)
99+
)
105100

106101
// sourcekitd's semantic request timer is only started when the first semantic request comes in.
107102
// Send a hover request (which will fail) to trigger that timer.
108103
// Afterwards wait for semantic functionality to be restored.
109104
_ = try? await testClient.send(hoverRequest)
110-
try await fulfillmentOfOrThrow([semanticFunctionalityRestored], timeout: 30)
105+
let semanticFunctionalityRestoredNotification = try await testClient.nextNotification(
106+
ofType: WorkDoneProgress.self,
107+
timeout: 30
108+
)
109+
XCTAssertEqual(semanticFunctionalityRestoredNotification.value, .end(WorkDoneProgressEnd()))
111110

112111
// Check that we get the same hover response from the restored in-memory state
113112

0 commit comments

Comments
 (0)