Skip to content

Commit 562bf80

Browse files
committed
If sourcekitd or clangd don’t respond to a request for 5 minutes, terminate them and use crash recovery to restore behavior
This should be a last stop-gap measure in case sourcekitd or clangd get stuck, don’t respond to any requests anymore and don’t honor cancellation either. In that case we can restore SourceKit-LSP behavior by killing them and using the crash recovery logic to restore functionality. rdar://149492159
1 parent 6a8ea4c commit 562bf80

File tree

17 files changed

+371
-84
lines changed

17 files changed

+371
-84
lines changed

Sources/BuildSystemIntegration/ExternalBuildSystemAdapter.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,11 @@ actor ExternalBuildSystemAdapter {
185185
protocol: bspRegistry,
186186
stderrLoggingCategory: "bsp-server-stderr",
187187
client: messagesToSourceKitLSPHandler,
188-
terminationHandler: { [weak self] terminationStatus in
188+
terminationHandler: { [weak self] terminationReason in
189189
guard let self else {
190190
return
191191
}
192-
if terminationStatus != 0 {
192+
if terminationReason != .exited(exitCode: 0) {
193193
Task {
194194
await orLog("Restarting BSP server") {
195195
try await self.handleBspServerCrash()

Sources/LanguageServerProtocolJSONRPC/JSONRPCConnection.swift

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ import struct CDispatch.dispatch_fd_t
3131
/// For example, inside a language server, the `JSONRPCConnection` takes the language service implementation as its
3232
// `receiveHandler` and itself provides the client connection for sending notifications and callbacks.
3333
public final class JSONRPCConnection: Connection {
34+
public enum TerminationReason: Sendable, Equatable {
35+
/// The process on the other end of the `JSONRPCConnection` terminated with the given exit code.
36+
case exited(exitCode: Int32)
37+
38+
/// The process on the other end of the `JSONRPCConnection` terminated with a signal. The signal that it terminated
39+
/// with is not known.
40+
case uncaughtSignal
41+
}
3442

3543
/// A name of the endpoint for this connection, used for logging, e.g. `clangd`.
3644
private let name: String
@@ -198,7 +206,7 @@ public final class JSONRPCConnection: Connection {
198206
protocol messageRegistry: MessageRegistry,
199207
stderrLoggingCategory: String,
200208
client: MessageHandler,
201-
terminationHandler: @Sendable @escaping (_ terminationStatus: Int32) -> Void
209+
terminationHandler: @Sendable @escaping (_ terminationReason: TerminationReason) -> Void
202210
) throws -> (connection: JSONRPCConnection, process: Process) {
203211
let clientToServer = Pipe()
204212
let serverToClient = Pipe()
@@ -238,10 +246,22 @@ public final class JSONRPCConnection: Connection {
238246
process.terminationHandler = { process in
239247
logger.log(
240248
level: process.terminationReason == .exit ? .default : .error,
241-
"\(name) exited: \(String(reflecting: process.terminationReason)) \(process.terminationStatus)"
249+
"\(name) exited: \(process.terminationReason.rawValue) \(process.terminationStatus)"
242250
)
243251
connection.close()
244-
terminationHandler(process.terminationStatus)
252+
let terminationReason: TerminationReason
253+
switch process.terminationReason {
254+
case .exit:
255+
terminationReason = .exited(exitCode: process.terminationStatus)
256+
case .uncaughtSignal:
257+
terminationReason = .uncaughtSignal
258+
@unknown default:
259+
logger.fault(
260+
"Process terminated with unknown termination reason: \(process.terminationReason.rawValue, privacy: .public)"
261+
)
262+
terminationReason = .exited(exitCode: 0)
263+
}
264+
terminationHandler(terminationReason)
245265
}
246266
try process.run()
247267

Sources/SKOptions/SourceKitLSPOptions.swift

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,17 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
422422
return .seconds(120)
423423
}
424424

425+
/// If a request to sourcekitd or clangd exceeds this timeout, we assume that the semantic service provider is hanging
426+
/// for some reason and won't recover. To restore semantic functionality, we terminate and restart it.
427+
public var semanticServiceRestartTimeout: Double? = nil
428+
429+
public var semanticServiceRestartTimeoutOrDefault: Duration {
430+
if let semanticServiceRestartTimeout {
431+
return .seconds(semanticServiceRestartTimeout)
432+
}
433+
return .seconds(300)
434+
}
435+
425436
public init(
426437
swiftPM: SwiftPMOptions? = .init(),
427438
fallbackBuildSystem: FallbackBuildSystemOptions? = .init(),
@@ -439,7 +450,8 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
439450
experimentalFeatures: Set<ExperimentalFeature>? = nil,
440451
swiftPublishDiagnosticsDebounceDuration: Double? = nil,
441452
workDoneProgressDebounceDuration: Double? = nil,
442-
sourcekitdRequestTimeout: Double? = nil
453+
sourcekitdRequestTimeout: Double? = nil,
454+
semanticServiceRestartTimeout: Double? = nil
443455
) {
444456
self.swiftPM = swiftPM
445457
self.fallbackBuildSystem = fallbackBuildSystem
@@ -458,6 +470,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
458470
self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration
459471
self.workDoneProgressDebounceDuration = workDoneProgressDebounceDuration
460472
self.sourcekitdRequestTimeout = sourcekitdRequestTimeout
473+
self.semanticServiceRestartTimeout = semanticServiceRestartTimeout
461474
}
462475

463476
public init?(fromLSPAny lspAny: LSPAny?) throws {
@@ -517,7 +530,8 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
517530
?? base.swiftPublishDiagnosticsDebounceDuration,
518531
workDoneProgressDebounceDuration: override?.workDoneProgressDebounceDuration
519532
?? base.workDoneProgressDebounceDuration,
520-
sourcekitdRequestTimeout: override?.sourcekitdRequestTimeout ?? base.sourcekitdRequestTimeout
533+
sourcekitdRequestTimeout: override?.sourcekitdRequestTimeout ?? base.sourcekitdRequestTimeout,
534+
semanticServiceRestartTimeout: override?.semanticServiceRestartTimeout ?? base.semanticServiceRestartTimeout
521535
)
522536
}
523537

Sources/SKTestSupport/SkipUnless.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,7 @@ package actor SkipUnless {
265265
sourcekitd.keys.useNewAPI: 1
266266
],
267267
]),
268-
timeout: defaultTimeoutDuration,
269-
fileContents: nil
268+
timeout: defaultTimeoutDuration
270269
)
271270
return response[sourcekitd.keys.useNewAPI] == 1
272271
} catch {
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2025 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+
package import SourceKitD
14+
15+
extension SourceKitD {
16+
/// Convenience overload of the `send` function for testing that doesn't restart sourcekitd if it does not respond
17+
/// and doesn't pass any file contents.
18+
package func send(
19+
_ request: SKDRequestDictionary,
20+
timeout: Duration
21+
) async throws -> SKDResponseDictionary {
22+
return try await self.send(
23+
request,
24+
timeout: timeout,
25+
restartTimeout: .seconds(60 * 60 * 24),
26+
fileContents: nil
27+
)
28+
}
29+
}

Sources/SourceKitD/SourceKitD.swift

Lines changed: 79 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,10 @@ package actor SourceKitD {
162162
/// List of notification handlers that will be called for each notification.
163163
private var notificationHandlers: [WeakSKDNotificationHandler] = []
164164

165-
/// List of hooks that should be executed for every request sent to sourcekitd.
165+
/// List of hooks that should be executed and that need to finish executing before a request is sent to sourcekitd.
166+
private var preRequestHandlingHooks: [UUID: @Sendable (SKDRequestDictionary) async -> Void] = [:]
167+
168+
/// List of hooks that should be executed after a request sent to sourcekitd.
166169
private var requestHandlingHooks: [UUID: (SKDRequestDictionary) -> Void] = [:]
167170

168171
package static func getOrCreate(
@@ -261,6 +264,30 @@ package actor SourceKitD {
261264
notificationHandlers.removeAll(where: { $0.value == nil || $0.value === handler })
262265
}
263266

267+
/// Execute `body` and invoke `hook` for every sourcekitd request that is sent during the execution time of `body`.
268+
///
269+
/// Note that `hook` will not only be executed for requests sent *by* body but this may also include sourcekitd
270+
/// requests that were sent by other clients of the same `DynamicallyLoadedSourceKitD` instance that just happen to
271+
/// send a request during that time.
272+
///
273+
/// This is intended for testing only.
274+
package func withPreRequestHandlingHook(
275+
body: () async throws -> Void,
276+
hook: @escaping @Sendable (SKDRequestDictionary) async -> Void
277+
) async rethrows {
278+
let id = UUID()
279+
preRequestHandlingHooks[id] = hook
280+
defer { preRequestHandlingHooks[id] = nil }
281+
try await body()
282+
}
283+
284+
func willSend(request: SKDRequestDictionary) async {
285+
let request = request
286+
for hook in preRequestHandlingHooks.values {
287+
await hook(request)
288+
}
289+
}
290+
264291
/// Execute `body` and invoke `hook` for every sourcekitd request that is sent during the execution time of `body`.
265292
///
266293
/// Note that `hook` will not only be executed for requests sent *by* body but this may also include sourcekitd
@@ -293,37 +320,56 @@ package actor SourceKitD {
293320
package func send(
294321
_ request: SKDRequestDictionary,
295322
timeout: Duration,
323+
restartTimeout: Duration,
296324
fileContents: String?
297325
) async throws -> SKDResponseDictionary {
298326
let sourcekitdResponse = try await withTimeout(timeout) {
299-
return try await withCancellableCheckedThrowingContinuation { (continuation) -> SourceKitDRequestHandle? in
300-
logger.info(
301-
"""
302-
Sending sourcekitd request:
303-
\(request.forLogging)
304-
"""
305-
)
306-
var handle: sourcekitd_api_request_handle_t? = nil
307-
self.api.send_request(request.dict, &handle) { response in
308-
continuation.resume(returning: SKDResponse(response!, sourcekitd: self))
309-
}
310-
Task {
311-
await self.didSend(request: request)
312-
}
313-
if let handle {
314-
return SourceKitDRequestHandle(handle: handle)
327+
let restartTimeoutHandle = TimeoutHandle()
328+
do {
329+
return try await withTimeout(restartTimeout, handle: restartTimeoutHandle) {
330+
await self.willSend(request: request)
331+
return try await withCancellableCheckedThrowingContinuation { (continuation) -> SourceKitDRequestHandle? in
332+
logger.info(
333+
"""
334+
Sending sourcekitd request:
335+
\(request.forLogging)
336+
"""
337+
)
338+
var handle: sourcekitd_api_request_handle_t? = nil
339+
self.api.send_request(request.dict, &handle) { response in
340+
continuation.resume(returning: SKDResponse(response!, sourcekitd: self))
341+
}
342+
Task {
343+
await self.didSend(request: request)
344+
}
345+
if let handle {
346+
return SourceKitDRequestHandle(handle: handle)
347+
}
348+
return nil
349+
} cancel: { (handle: SourceKitDRequestHandle?) in
350+
if let handle {
351+
logger.info(
352+
"""
353+
Cancelling sourcekitd request:
354+
\(request.forLogging)
355+
"""
356+
)
357+
self.api.cancel_request(handle.handle)
358+
}
359+
}
315360
}
316-
return nil
317-
} cancel: { (handle: SourceKitDRequestHandle?) in
318-
if let handle {
319-
logger.info(
320-
"""
321-
Cancelling sourcekitd request:
322-
\(request.forLogging)
323-
"""
361+
} catch let error as TimeoutError where error.handle == restartTimeoutHandle {
362+
if !self.path.lastPathComponent.contains("InProc") {
363+
logger.fault(
364+
"Did not receive reply from sourcekitd after \(restartTimeout, privacy: .public). Terminating and restarting sourcekitd."
365+
)
366+
await self.crash()
367+
} else {
368+
logger.fault(
369+
"Did not receive reply from sourcekitd after \(restartTimeout, privacy: .public). Not terminating sourcekitd because it is run in-process."
324370
)
325-
self.api.cancel_request(handle.handle)
326371
}
372+
throw error
327373
}
328374
}
329375

@@ -362,6 +408,13 @@ package actor SourceKitD {
362408

363409
return dict
364410
}
411+
412+
package func crash() async {
413+
let req = dictionary([
414+
keys.request: requests.crashWithExit
415+
])
416+
_ = try? await send(req, timeout: .seconds(60), restartTimeout: .seconds(24 * 60 * 60), fileContents: nil)
417+
}
365418
}
366419

367420
/// A sourcekitd notification handler in a class to allow it to be uniquely referenced.

Sources/SourceKitD/SourceKitDRegistry.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ package actor SourceKitDRegistry<SourceKitDType: AnyObject> {
3030
private var active: [URL: (pluginPaths: PluginPaths?, sourcekitd: SourceKitDType)] = [:]
3131

3232
/// Instances that have been unregistered, but may be resurrected if accessed before destruction.
33-
private var cemetary: [URL: (pluginPaths: PluginPaths?, sourcekitd: WeakSourceKitD<SourceKitDType>)] = [:]
33+
private var cemetery: [URL: (pluginPaths: PluginPaths?, sourcekitd: WeakSourceKitD<SourceKitDType>)] = [:]
3434

3535
/// Initialize an empty registry.
3636
package init() {}
@@ -49,8 +49,8 @@ package actor SourceKitDRegistry<SourceKitDType: AnyObject> {
4949
}
5050
return existing.sourcekitd
5151
}
52-
if let resurrected = cemetary[key], let resurrectedSourcekitD = resurrected.sourcekitd.value {
53-
cemetary[key] = nil
52+
if let resurrected = cemetery[key], let resurrectedSourcekitD = resurrected.sourcekitd.value {
53+
cemetery[key] = nil
5454
if resurrected.pluginPaths != pluginPaths {
5555
logger.fault(
5656
"Already created SourceKitD with plugin paths \(resurrected.pluginPaths?.forLogging), now requesting incompatible plugin paths \(pluginPaths.forLogging)"
@@ -74,8 +74,8 @@ package actor SourceKitDRegistry<SourceKitDType: AnyObject> {
7474
package func remove(_ key: URL) -> SourceKitDType? {
7575
let existing = active.removeValue(forKey: key)
7676
if let existing = existing {
77-
assert(self.cemetary[key]?.sourcekitd.value == nil)
78-
cemetary[key] = (existing.pluginPaths, WeakSourceKitD(value: existing.sourcekitd))
77+
assert(self.cemetery[key]?.sourcekitd.value == nil)
78+
cemetery[key] = (existing.pluginPaths, WeakSourceKitD(value: existing.sourcekitd))
7979
}
8080
return existing?.sourcekitd
8181
}

Sources/SourceKitLSP/Clang/ClangLanguageService.swift

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ actor ClangLanguageService: LanguageService, MessageHandler {
6262
/// Path to the `clangd` binary.
6363
let clangdPath: URL
6464

65-
let clangdOptions: [String]
65+
let options: SourceKitLSPOptions
6666

6767
/// The current state of the `clangd` language server.
6868
/// Changing the property automatically notified the state change handlers.
@@ -117,7 +117,7 @@ actor ClangLanguageService: LanguageService, MessageHandler {
117117
}
118118
self.clangPath = toolchain.clang
119119
self.clangdPath = clangdPath
120-
self.clangdOptions = options.clangdOptions ?? []
120+
self.options = options
121121
self.workspace = WeakWorkspace(workspace)
122122
self.state = .connected
123123
self.sourceKitLSPServer = sourceKitLSPServer
@@ -154,10 +154,11 @@ actor ClangLanguageService: LanguageService, MessageHandler {
154154
/// Restarts `clangd` if it has crashed.
155155
///
156156
/// - Parameter terminationStatus: The exit code of `clangd`.
157-
private func handleClangdTermination(terminationStatus: Int32) {
157+
private func handleClangdTermination(terminationReason: JSONRPCConnection.TerminationReason) {
158158
self.clangdProcess = nil
159-
if terminationStatus != 0 {
159+
if terminationReason != .exited(exitCode: 0) {
160160
self.state = .connectionInterrupted
161+
logger.info("clangd crashed. Restarting it.")
161162
self.restartClangd()
162163
}
163164
}
@@ -173,15 +174,15 @@ actor ClangLanguageService: LanguageService, MessageHandler {
173174
"-compile_args_from=lsp", // Provide compiler args programmatically.
174175
"-background-index=false", // Disable clangd indexing, we use the build
175176
"-index=false", // system index store instead.
176-
] + clangdOptions,
177+
] + (options.clangdOptions ?? []),
177178
name: "clangd",
178179
protocol: MessageRegistry.lspProtocol,
179180
stderrLoggingCategory: "clangd-stderr",
180181
client: self,
181-
terminationHandler: { [weak self] terminationStatus in
182+
terminationHandler: { [weak self] terminationReason in
182183
guard let self = self else { return }
183184
Task {
184-
await self.handleClangdTermination(terminationStatus: terminationStatus)
185+
await self.handleClangdTermination(terminationReason: terminationReason)
185186
}
186187

187188
}
@@ -291,14 +292,20 @@ actor ClangLanguageService: LanguageService, MessageHandler {
291292
}
292293

293294
/// Forward the given request to `clangd`.
294-
///
295-
/// This method calls `readyToHandleNextRequest` once the request has been
296-
/// transmitted to `clangd` and another request can be safely transmitted to
297-
/// `clangd` while guaranteeing ordering.
298-
///
299-
/// The response of the request is returned asynchronously as the return value.
300295
func forwardRequestToClangd<R: RequestType>(_ request: R) async throws -> R.Response {
301-
return try await clangd.send(request)
296+
let timeoutHandle = TimeoutHandle()
297+
do {
298+
return try await withTimeout(options.semanticServiceRestartTimeoutOrDefault, handle: timeoutHandle) {
299+
await self.sourceKitLSPServer?.hooks.preForwardRequestToClangd?(request)
300+
return try await self.clangd.send(request)
301+
}
302+
} catch let error as TimeoutError where error.handle == timeoutHandle {
303+
logger.fault(
304+
"Did not receive reply from clangd after \(options.semanticServiceRestartTimeoutOrDefault, privacy: .public). Terminating and restarting clangd."
305+
)
306+
self.crash()
307+
throw error
308+
}
302309
}
303310

304311
package func canonicalDeclarationPosition(of position: Position, in uri: DocumentURI) async -> Position? {

0 commit comments

Comments
 (0)