Skip to content

Commit 21e3bc3

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 21e3bc3

File tree

19 files changed

+377
-84
lines changed

19 files changed

+377
-84
lines changed

Documentation/Configuration File.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,4 @@ The structure of the file is currently not guaranteed to be stable. Options may
5858
- `swiftPublishDiagnosticsDebounceDuration: number`: The time that `SwiftLanguageService` should wait after an edit before starting to compute diagnostics and sending a `PublishDiagnosticsNotification`.
5959
- `workDoneProgressDebounceDuration: number`: When a task is started that should be displayed to the client as a work done progress, how many milliseconds to wait before actually starting the work done progress. This prevents flickering of the work done progress in the client for short-lived index tasks which end within this duration.
6060
- `sourcekitdRequestTimeout: number`: The maximum duration that a sourcekitd request should be allowed to execute before being declared as timed out. In general, editors should cancel requests that they are no longer interested in, but in case editors don't cancel requests, this ensures that a long-running non-cancelled request is not blocking sourcekitd and thus most semantic functionality. In particular, VS Code does not cancel the semantic tokens request, which can cause a long-running AST build that blocks sourcekitd.
61+
- `semanticServiceRestartTimeout: number`: If a request to sourcekitd or clangd exceeds this timeout, we assume that the semantic service provider is hanging for some reason and won't recover. To restore semantic functionality, we terminate and restart it.

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
}

0 commit comments

Comments
 (0)