Skip to content

If sourcekitd or clangd don’t respond to a request for 5 minutes, terminate them and use crash recovery to restore behavior #2137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Documentation/Configuration File.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,4 @@ The structure of the file is currently not guaranteed to be stable. Options may
- `swiftPublishDiagnosticsDebounceDuration: number`: The time that `SwiftLanguageService` should wait after an edit before starting to compute diagnostics and sending a `PublishDiagnosticsNotification`.
- `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.
- `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.
- `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.
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,11 @@ actor ExternalBuildSystemAdapter {
protocol: bspRegistry,
stderrLoggingCategory: "bsp-server-stderr",
client: messagesToSourceKitLSPHandler,
terminationHandler: { [weak self] terminationStatus in
terminationHandler: { [weak self] terminationReason in
guard let self else {
return
}
if terminationStatus != 0 {
if terminationReason != .exited(exitCode: 0) {
Task {
await orLog("Restarting BSP server") {
try await self.handleBspServerCrash()
Expand Down
26 changes: 23 additions & 3 deletions Sources/LanguageServerProtocolJSONRPC/JSONRPCConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ import struct CDispatch.dispatch_fd_t
/// For example, inside a language server, the `JSONRPCConnection` takes the language service implementation as its
// `receiveHandler` and itself provides the client connection for sending notifications and callbacks.
public final class JSONRPCConnection: Connection {
public enum TerminationReason: Sendable, Equatable {
/// The process on the other end of the `JSONRPCConnection` terminated with the given exit code.
case exited(exitCode: Int32)

/// The process on the other end of the `JSONRPCConnection` terminated with a signal. The signal that it terminated
/// with is not known.
case uncaughtSignal
}

/// A name of the endpoint for this connection, used for logging, e.g. `clangd`.
private let name: String
Expand Down Expand Up @@ -198,7 +206,7 @@ public final class JSONRPCConnection: Connection {
protocol messageRegistry: MessageRegistry,
stderrLoggingCategory: String,
client: MessageHandler,
terminationHandler: @Sendable @escaping (_ terminationStatus: Int32) -> Void
terminationHandler: @Sendable @escaping (_ terminationReason: TerminationReason) -> Void
) throws -> (connection: JSONRPCConnection, process: Process) {
let clientToServer = Pipe()
let serverToClient = Pipe()
Expand Down Expand Up @@ -238,10 +246,22 @@ public final class JSONRPCConnection: Connection {
process.terminationHandler = { process in
logger.log(
level: process.terminationReason == .exit ? .default : .error,
"\(name) exited: \(String(reflecting: process.terminationReason)) \(process.terminationStatus)"
"\(name) exited: \(process.terminationReason.rawValue) \(process.terminationStatus)"
)
connection.close()
terminationHandler(process.terminationStatus)
let terminationReason: TerminationReason
switch process.terminationReason {
case .exit:
terminationReason = .exited(exitCode: process.terminationStatus)
case .uncaughtSignal:
terminationReason = .uncaughtSignal
@unknown default:
logger.fault(
"Process terminated with unknown termination reason: \(process.terminationReason.rawValue, privacy: .public)"
)
terminationReason = .exited(exitCode: 0)
}
terminationHandler(terminationReason)
}
try process.run()

Expand Down
18 changes: 16 additions & 2 deletions Sources/SKOptions/SourceKitLSPOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,17 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
return .seconds(120)
}

/// 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.
public var semanticServiceRestartTimeout: Double? = nil

public var semanticServiceRestartTimeoutOrDefault: Duration {
if let semanticServiceRestartTimeout {
return .seconds(semanticServiceRestartTimeout)
}
return .seconds(300)
}

public init(
swiftPM: SwiftPMOptions? = .init(),
fallbackBuildSystem: FallbackBuildSystemOptions? = .init(),
Expand All @@ -439,7 +450,8 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
experimentalFeatures: Set<ExperimentalFeature>? = nil,
swiftPublishDiagnosticsDebounceDuration: Double? = nil,
workDoneProgressDebounceDuration: Double? = nil,
sourcekitdRequestTimeout: Double? = nil
sourcekitdRequestTimeout: Double? = nil,
semanticServiceRestartTimeout: Double? = nil
) {
self.swiftPM = swiftPM
self.fallbackBuildSystem = fallbackBuildSystem
Expand All @@ -458,6 +470,7 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration
self.workDoneProgressDebounceDuration = workDoneProgressDebounceDuration
self.sourcekitdRequestTimeout = sourcekitdRequestTimeout
self.semanticServiceRestartTimeout = semanticServiceRestartTimeout
}

public init?(fromLSPAny lspAny: LSPAny?) throws {
Expand Down Expand Up @@ -517,7 +530,8 @@ public struct SourceKitLSPOptions: Sendable, Codable, Equatable {
?? base.swiftPublishDiagnosticsDebounceDuration,
workDoneProgressDebounceDuration: override?.workDoneProgressDebounceDuration
?? base.workDoneProgressDebounceDuration,
sourcekitdRequestTimeout: override?.sourcekitdRequestTimeout ?? base.sourcekitdRequestTimeout
sourcekitdRequestTimeout: override?.sourcekitdRequestTimeout ?? base.sourcekitdRequestTimeout,
semanticServiceRestartTimeout: override?.semanticServiceRestartTimeout ?? base.semanticServiceRestartTimeout
)
}

Expand Down
3 changes: 1 addition & 2 deletions Sources/SKTestSupport/SkipUnless.swift
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,7 @@ package actor SkipUnless {
sourcekitd.keys.useNewAPI: 1
],
]),
timeout: defaultTimeoutDuration,
fileContents: nil
timeout: defaultTimeoutDuration
)
return response[sourcekitd.keys.useNewAPI] == 1
} catch {
Expand Down
29 changes: 29 additions & 0 deletions Sources/SKTestSupport/SourceKitD+send.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

package import SourceKitD

extension SourceKitD {
/// Convenience overload of the `send` function for testing that doesn't restart sourcekitd if it does not respond
/// and doesn't pass any file contents.
package func send(
_ request: SKDRequestDictionary,
timeout: Duration
) async throws -> SKDResponseDictionary {
return try await self.send(
request,
timeout: timeout,
restartTimeout: .seconds(60 * 60 * 24),
fileContents: nil
)
}
}
105 changes: 79 additions & 26 deletions Sources/SourceKitD/SourceKitD.swift
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ package actor SourceKitD {
/// List of notification handlers that will be called for each notification.
private var notificationHandlers: [WeakSKDNotificationHandler] = []

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

/// List of hooks that should be executed after a request sent to sourcekitd.
private var requestHandlingHooks: [UUID: (SKDRequestDictionary) -> Void] = [:]

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

/// Execute `body` and invoke `hook` for every sourcekitd request that is sent during the execution time of `body`.
///
/// Note that `hook` will not only be executed for requests sent *by* body but this may also include sourcekitd
/// requests that were sent by other clients of the same `DynamicallyLoadedSourceKitD` instance that just happen to
/// send a request during that time.
///
/// This is intended for testing only.
package func withPreRequestHandlingHook(
body: () async throws -> Void,
hook: @escaping @Sendable (SKDRequestDictionary) async -> Void
) async rethrows {
let id = UUID()
preRequestHandlingHooks[id] = hook
defer { preRequestHandlingHooks[id] = nil }
try await body()
}

func willSend(request: SKDRequestDictionary) async {
let request = request
for hook in preRequestHandlingHooks.values {
await hook(request)
}
}

/// Execute `body` and invoke `hook` for every sourcekitd request that is sent during the execution time of `body`.
///
/// Note that `hook` will not only be executed for requests sent *by* body but this may also include sourcekitd
Expand Down Expand Up @@ -293,37 +320,56 @@ package actor SourceKitD {
package func send(
_ request: SKDRequestDictionary,
timeout: Duration,
restartTimeout: Duration,
fileContents: String?
) async throws -> SKDResponseDictionary {
let sourcekitdResponse = try await withTimeout(timeout) {
return try await withCancellableCheckedThrowingContinuation { (continuation) -> SourceKitDRequestHandle? in
logger.info(
"""
Sending sourcekitd request:
\(request.forLogging)
"""
)
var handle: sourcekitd_api_request_handle_t? = nil
self.api.send_request(request.dict, &handle) { response in
continuation.resume(returning: SKDResponse(response!, sourcekitd: self))
}
Task {
await self.didSend(request: request)
}
if let handle {
return SourceKitDRequestHandle(handle: handle)
let restartTimeoutHandle = TimeoutHandle()
do {
return try await withTimeout(restartTimeout, handle: restartTimeoutHandle) {
await self.willSend(request: request)
return try await withCancellableCheckedThrowingContinuation { (continuation) -> SourceKitDRequestHandle? in
logger.info(
"""
Sending sourcekitd request:
\(request.forLogging)
"""
)
var handle: sourcekitd_api_request_handle_t? = nil
self.api.send_request(request.dict, &handle) { response in
continuation.resume(returning: SKDResponse(response!, sourcekitd: self))
}
Task {
await self.didSend(request: request)
}
if let handle {
return SourceKitDRequestHandle(handle: handle)
}
return nil
} cancel: { (handle: SourceKitDRequestHandle?) in
if let handle {
logger.info(
"""
Cancelling sourcekitd request:
\(request.forLogging)
"""
)
self.api.cancel_request(handle.handle)
}
}
}
return nil
} cancel: { (handle: SourceKitDRequestHandle?) in
if let handle {
logger.info(
"""
Cancelling sourcekitd request:
\(request.forLogging)
"""
} catch let error as TimeoutError where error.handle == restartTimeoutHandle {
if !self.path.lastPathComponent.contains("InProc") {
logger.fault(
"Did not receive reply from sourcekitd after \(restartTimeout, privacy: .public). Terminating and restarting sourcekitd."
)
await self.crash()
} else {
logger.fault(
"Did not receive reply from sourcekitd after \(restartTimeout, privacy: .public). Not terminating sourcekitd because it is run in-process."
)
self.api.cancel_request(handle.handle)
}
throw error
}
}

Expand Down Expand Up @@ -362,6 +408,13 @@ package actor SourceKitD {

return dict
}

package func crash() async {
let req = dictionary([
keys.request: requests.crashWithExit
])
_ = try? await send(req, timeout: .seconds(60), restartTimeout: .seconds(24 * 60 * 60), fileContents: nil)
}
}

/// A sourcekitd notification handler in a class to allow it to be uniquely referenced.
Expand Down
10 changes: 5 additions & 5 deletions Sources/SourceKitD/SourceKitDRegistry.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ package actor SourceKitDRegistry<SourceKitDType: AnyObject> {
private var active: [URL: (pluginPaths: PluginPaths?, sourcekitd: SourceKitDType)] = [:]

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

/// Initialize an empty registry.
package init() {}
Expand All @@ -49,8 +49,8 @@ package actor SourceKitDRegistry<SourceKitDType: AnyObject> {
}
return existing.sourcekitd
}
if let resurrected = cemetary[key], let resurrectedSourcekitD = resurrected.sourcekitd.value {
cemetary[key] = nil
if let resurrected = cemetery[key], let resurrectedSourcekitD = resurrected.sourcekitd.value {
cemetery[key] = nil
if resurrected.pluginPaths != pluginPaths {
logger.fault(
"Already created SourceKitD with plugin paths \(resurrected.pluginPaths?.forLogging), now requesting incompatible plugin paths \(pluginPaths.forLogging)"
Expand All @@ -74,8 +74,8 @@ package actor SourceKitDRegistry<SourceKitDType: AnyObject> {
package func remove(_ key: URL) -> SourceKitDType? {
let existing = active.removeValue(forKey: key)
if let existing = existing {
assert(self.cemetary[key]?.sourcekitd.value == nil)
cemetary[key] = (existing.pluginPaths, WeakSourceKitD(value: existing.sourcekitd))
assert(self.cemetery[key]?.sourcekitd.value == nil)
cemetery[key] = (existing.pluginPaths, WeakSourceKitD(value: existing.sourcekitd))
}
return existing?.sourcekitd
}
Expand Down
Loading