From 191d366a2cede71624411d0725871c53ea472246 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Tue, 2 Jul 2024 09:27:15 +0200 Subject: [PATCH] Add a maximum duration for sourcekitd requests VS Code does not cancel semantic tokens requests. If a source file gets into a state where an AST build takes very long, this can cause us to wait for the semantic tokens from sourcekitd for a few minutes, effectively blocking all other semantic functionality in that file. To circumvent this problem (or any other problem where an editor might not be cancelling requests they are no longer interested in) add a maximum request duration for SourceKitD requests, defaulting to 2 minutes. rdar://130948453 --- .../RunSourcekitdRequestCommand.swift | 3 ++ Sources/LanguageServerProtocol/Error.swift | 2 +- Sources/SKCore/SourceKitLSPOptions.swift | 28 +++++++++-- Sources/SourceKitD/SourceKitD.swift | 39 ++++++++++----- Sources/SourceKitLSP/Rename.swift | 6 +-- .../SourceKitLSP/Swift/CodeCompletion.swift | 1 + .../Swift/CodeCompletionSession.swift | 20 ++++++-- Sources/SourceKitLSP/Swift/CursorInfo.swift | 2 +- .../Swift/DiagnosticReportManager.swift | 10 +++- .../SourceKitLSP/Swift/OpenInterface.swift | 6 +-- Sources/SourceKitLSP/Swift/Refactoring.swift | 2 +- .../Swift/RelatedIdentifiers.swift | 2 +- .../SourceKitLSP/Swift/SemanticTokens.swift | 2 +- .../Swift/SourceKitD+ResponseError.swift | 2 + .../Swift/SwiftLanguageService.swift | 40 ++++++++++----- .../SourceKitLSP/Swift/VariableTypeInfo.swift | 2 +- Tests/DiagnoseTests/DiagnoseTests.swift | 2 +- Tests/SourceKitDTests/SourceKitDTests.swift | 4 +- Tests/SourceKitLSPTests/LocalSwiftTests.swift | 50 ++++++++++++++++++- 19 files changed, 172 insertions(+), 51 deletions(-) diff --git a/Sources/Diagnose/RunSourcekitdRequestCommand.swift b/Sources/Diagnose/RunSourcekitdRequestCommand.swift index 6cfe921c0..5f0e4a36a 100644 --- a/Sources/Diagnose/RunSourcekitdRequestCommand.swift +++ b/Sources/Diagnose/RunSourcekitdRequestCommand.swift @@ -96,6 +96,9 @@ public struct RunSourceKitdRequestCommand: AsyncParsableCommand { case .requestCancelled: print("request cancelled") throw ExitCode(1) + case .timedOut: + print("request timed out") + throw ExitCode(1) case .missingRequiredSymbol: print("missing required symbol") throw ExitCode(1) diff --git a/Sources/LanguageServerProtocol/Error.swift b/Sources/LanguageServerProtocol/Error.swift index 042bed2dc..5b9dcd4c3 100644 --- a/Sources/LanguageServerProtocol/Error.swift +++ b/Sources/LanguageServerProtocol/Error.swift @@ -81,7 +81,7 @@ public struct ErrorCode: RawRepresentable, Codable, Hashable, Sendable { /// It doesn't denote a real error code. public static let lspReservedErrorRangeEnd = ErrorCode(rawValue: -32800) - // MARK: SourceKit-LSP specifiic eror codes + // MARK: SourceKit-LSP specific error codes public static let workspaceNotOpen: ErrorCode = ErrorCode(rawValue: -32003) } diff --git a/Sources/SKCore/SourceKitLSPOptions.swift b/Sources/SKCore/SourceKitLSPOptions.swift index 0f9da813d..e35aae254 100644 --- a/Sources/SKCore/SourceKitLSPOptions.swift +++ b/Sources/SKCore/SourceKitLSPOptions.swift @@ -191,6 +191,10 @@ public struct SourceKitLSPOptions: Sendable, Codable { /// Whether background indexing is enabled. public var backgroundIndexing: Bool? + public var backgroundIndexingOrDefault: Bool { + return backgroundIndexing ?? false + } + /// Experimental features that are enabled. public var experimentalFeatures: Set? = nil @@ -220,8 +224,21 @@ public struct SourceKitLSPOptions: Sendable, Codable { return .seconds(1) } - public var backgroundIndexingOrDefault: Bool { - return backgroundIndexing ?? false + /// 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. + public var sourcekitdRequestTimeout: Double? = nil + + public var sourcekitdRequestTimeoutOrDefault: Duration { + if let sourcekitdRequestTimeout { + return .seconds(sourcekitdRequestTimeout) + } + return .seconds(120) } public init( @@ -235,7 +252,8 @@ public struct SourceKitLSPOptions: Sendable, Codable { backgroundIndexing: Bool? = nil, experimentalFeatures: Set? = nil, swiftPublishDiagnosticsDebounceDuration: Double? = nil, - workDoneProgressDebounceDuration: Double? = nil + workDoneProgressDebounceDuration: Double? = nil, + sourcekitdRequestTimeout: Double? = nil ) { self.swiftPM = swiftPM self.fallbackBuildSystem = fallbackBuildSystem @@ -248,6 +266,7 @@ public struct SourceKitLSPOptions: Sendable, Codable { self.experimentalFeatures = experimentalFeatures self.swiftPublishDiagnosticsDebounceDuration = swiftPublishDiagnosticsDebounceDuration self.workDoneProgressDebounceDuration = workDoneProgressDebounceDuration + self.sourcekitdRequestTimeout = sourcekitdRequestTimeout } public init?(fromLSPAny lspAny: LSPAny?) throws { @@ -300,7 +319,8 @@ public struct SourceKitLSPOptions: Sendable, Codable { swiftPublishDiagnosticsDebounceDuration: override?.swiftPublishDiagnosticsDebounceDuration ?? base.swiftPublishDiagnosticsDebounceDuration, workDoneProgressDebounceDuration: override?.workDoneProgressDebounceDuration - ?? base.workDoneProgressDebounceDuration + ?? base.workDoneProgressDebounceDuration, + sourcekitdRequestTimeout: override?.sourcekitdRequestTimeout ?? base.sourcekitdRequestTimeout ) } diff --git a/Sources/SourceKitD/SourceKitD.swift b/Sources/SourceKitD/SourceKitD.swift index 7086c87c2..13604944b 100644 --- a/Sources/SourceKitD/SourceKitD.swift +++ b/Sources/SourceKitD/SourceKitD.swift @@ -83,31 +83,41 @@ public enum SKDError: Error, Equatable { /// The request was cancelled. case requestCancelled + /// The request exceeded the maximum allowed duration. + case timedOut + /// Loading a required symbol from the sourcekitd library failed. case missingRequiredSymbol(String) } extension SourceKitD { - // MARK: - Convenience API for requests. /// - Parameters: - /// - req: The request to send to sourcekitd. + /// - request: The request to send to sourcekitd. + /// - timeout: The maximum duration how long to wait for a response. If no response is returned within this time, + /// declare the request as having timed out. /// - fileContents: The contents of the file that the request operates on. If sourcekitd crashes, the file contents /// will be logged. - public func send(_ request: SKDRequestDictionary, fileContents: String?) async throws -> SKDResponseDictionary { + public func send( + _ request: SKDRequestDictionary, + timeout: Duration, + fileContents: String? + ) async throws -> SKDResponseDictionary { log(request: request) - let sourcekitdResponse: SKDResponse = try await withCancellableCheckedThrowingContinuation { continuation in - var handle: sourcekitd_api_request_handle_t? = nil - api.send_request(request.dict, &handle) { response in - continuation.resume(returning: SKDResponse(response!, sourcekitd: self)) - } - return handle - } cancel: { handle in - if let handle { - logRequestCancellation(request: request) - api.cancel_request(handle) + let sourcekitdResponse = try await withTimeout(timeout) { + return try await withCancellableCheckedThrowingContinuation { continuation in + var handle: sourcekitd_api_request_handle_t? = nil + self.api.send_request(request.dict, &handle) { response in + continuation.resume(returning: SKDResponse(response!, sourcekitd: self)) + } + return handle + } cancel: { handle in + if let handle { + self.logRequestCancellation(request: request) + self.api.cancel_request(handle) + } } } @@ -117,6 +127,9 @@ extension SourceKitD { if sourcekitdResponse.error == .connectionInterrupted { log(crashedRequest: request, fileContents: fileContents) } + if sourcekitdResponse.error == .requestCancelled && !Task.isCancelled { + throw SKDError.timedOut + } throw sourcekitdResponse.error! } diff --git a/Sources/SourceKitLSP/Rename.swift b/Sources/SourceKitLSP/Rename.swift index 80a446206..9e9d44b7a 100644 --- a/Sources/SourceKitLSP/Rename.swift +++ b/Sources/SourceKitLSP/Rename.swift @@ -359,7 +359,7 @@ extension SwiftLanguageService { keys.argNames: sourcekitd.array(name.parameters.map { $0.stringOrWildcard }), ]) - let response = try await sourcekitd.send(req, fileContents: snapshot.text) + let response = try await sendSourcekitdRequest(req, fileContents: snapshot.text) guard let isZeroArgSelector: Int = response[keys.isZeroArgSelector], let selectorPieces: SKDResponseArray = response[keys.selectorPieces] @@ -416,7 +416,7 @@ extension SwiftLanguageService { req.set(keys.baseName, to: name) } - let response = try await sourcekitd.send(req, fileContents: snapshot.text) + let response = try await sendSourcekitdRequest(req, fileContents: snapshot.text) guard let baseName: String = response[keys.baseName] else { throw NameTranslationError.malformedClangToSwiftTranslateNameResponse(response) @@ -914,7 +914,7 @@ extension SwiftLanguageService { keys.renameLocations: locations, ]) - let syntacticRenameRangesResponse = try await sourcekitd.send(skreq, fileContents: snapshot.text) + let syntacticRenameRangesResponse = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) guard let categorizedRanges: SKDResponseArray = syntacticRenameRangesResponse[keys.categorizedRanges] else { throw ResponseError.internalError("sourcekitd did not return categorized ranges") } diff --git a/Sources/SourceKitLSP/Swift/CodeCompletion.swift b/Sources/SourceKitLSP/Swift/CodeCompletion.swift index b33d234a2..0041d5b4a 100644 --- a/Sources/SourceKitLSP/Swift/CodeCompletion.swift +++ b/Sources/SourceKitLSP/Swift/CodeCompletion.swift @@ -33,6 +33,7 @@ extension SwiftLanguageService { return try await CodeCompletionSession.completionList( sourcekitd: sourcekitd, snapshot: snapshot, + options: options, indentationWidth: inferredIndentationWidth, completionPosition: completionPos, completionUtf8Offset: offset, diff --git a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift index 7957e6eb2..169831789 100644 --- a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift +++ b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift @@ -13,6 +13,7 @@ import Dispatch import LSPLogging import LanguageServerProtocol +import SKCore import SKSupport import SourceKitD import SwiftExtensions @@ -92,6 +93,7 @@ class CodeCompletionSession { static func completionList( sourcekitd: any SourceKitD, snapshot: DocumentSnapshot, + options: SourceKitLSPOptions, indentationWidth: Trivia?, completionPosition: Position, completionUtf8Offset: Int, @@ -122,6 +124,7 @@ class CodeCompletionSession { let session = CodeCompletionSession( sourcekitd: sourcekitd, snapshot: snapshot, + options: options, indentationWidth: indentationWidth, utf8Offset: completionUtf8Offset, position: completionPosition, @@ -139,6 +142,7 @@ class CodeCompletionSession { private let sourcekitd: any SourceKitD private let snapshot: DocumentSnapshot + private let options: SourceKitLSPOptions /// The inferred indentation width of the source file the completion is being performed in private let indentationWidth: Trivia? private let utf8StartOffset: Int @@ -158,6 +162,7 @@ class CodeCompletionSession { private init( sourcekitd: any SourceKitD, snapshot: DocumentSnapshot, + options: SourceKitLSPOptions, indentationWidth: Trivia?, utf8Offset: Int, position: Position, @@ -165,6 +170,7 @@ class CodeCompletionSession { clientSupportsSnippets: Bool ) { self.sourcekitd = sourcekitd + self.options = options self.indentationWidth = indentationWidth self.snapshot = snapshot self.utf8StartOffset = utf8Offset @@ -193,7 +199,11 @@ class CodeCompletionSession { keys.compilerArgs: compileCommand?.compilerArgs as [SKDRequestValue]?, ]) - let dict = try await sourcekitd.send(req, fileContents: snapshot.text) + let dict = try await sourcekitd.send( + req, + timeout: options.sourcekitdRequestTimeoutOrDefault, + fileContents: snapshot.text + ) self.state = .open guard let completions: SKDResponseArray = dict[keys.results] else { @@ -226,7 +236,11 @@ class CodeCompletionSession { keys.codeCompleteOptions: optionsDictionary(filterText: filterText), ]) - let dict = try await sourcekitd.send(req, fileContents: snapshot.text) + let dict = try await sourcekitd.send( + req, + timeout: options.sourcekitdRequestTimeoutOrDefault, + fileContents: snapshot.text + ) guard let completions: SKDResponseArray = dict[keys.results] else { return CompletionList(isIncomplete: false, items: []) } @@ -269,7 +283,7 @@ class CodeCompletionSession { keys.name: snapshot.uri.pseudoPath, ]) logger.info("Closing code completion session: \(self.description)") - _ = try? await sourcekitd.send(req, fileContents: nil) + _ = try? await sourcekitd.send(req, timeout: options.sourcekitdRequestTimeoutOrDefault, fileContents: nil) self.state = .closed } } diff --git a/Sources/SourceKitLSP/Swift/CursorInfo.swift b/Sources/SourceKitLSP/Swift/CursorInfo.swift index c9ead89dd..ea8249c4c 100644 --- a/Sources/SourceKitLSP/Swift/CursorInfo.swift +++ b/Sources/SourceKitLSP/Swift/CursorInfo.swift @@ -159,7 +159,7 @@ extension SwiftLanguageService { appendAdditionalParameters?(skreq) - let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text) + let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) var cursorInfoResults: [CursorInfo] = [] if let cursorInfo = CursorInfo(dict, sourcekitd: sourcekitd) { diff --git a/Sources/SourceKitLSP/Swift/DiagnosticReportManager.swift b/Sources/SourceKitLSP/Swift/DiagnosticReportManager.swift index 005fd8173..c6b22ba6f 100644 --- a/Sources/SourceKitLSP/Swift/DiagnosticReportManager.swift +++ b/Sources/SourceKitLSP/Swift/DiagnosticReportManager.swift @@ -12,6 +12,7 @@ import LSPLogging import LanguageServerProtocol +import SKCore import SKSupport import SourceKitD import SwiftExtensions @@ -22,6 +23,7 @@ actor DiagnosticReportManager { private typealias ReportTask = RefCountedCancellableTask private let sourcekitd: SourceKitD + private let options: SourceKitLSPOptions private let syntaxTreeManager: SyntaxTreeManager private let documentManager: DocumentManager private let clientHasDiagnosticsCodeDescriptionSupport: Bool @@ -48,11 +50,13 @@ actor DiagnosticReportManager { init( sourcekitd: SourceKitD, + options: SourceKitLSPOptions, syntaxTreeManager: SyntaxTreeManager, documentManager: DocumentManager, clientHasDiagnosticsCodeDescriptionSupport: Bool ) { self.sourcekitd = sourcekitd + self.options = options self.syntaxTreeManager = syntaxTreeManager self.documentManager = documentManager self.clientHasDiagnosticsCodeDescriptionSupport = clientHasDiagnosticsCodeDescriptionSupport @@ -104,7 +108,11 @@ actor DiagnosticReportManager { keys.compilerArgs: compilerArgs as [SKDRequestValue], ]) - let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text) + let dict = try await self.sourcekitd.send( + skreq, + timeout: options.sourcekitdRequestTimeoutOrDefault, + fileContents: snapshot.text + ) try Task.checkCancellation() diff --git a/Sources/SourceKitLSP/Swift/OpenInterface.swift b/Sources/SourceKitLSP/Swift/OpenInterface.swift index b81480d52..03d455e66 100644 --- a/Sources/SourceKitLSP/Swift/OpenInterface.swift +++ b/Sources/SourceKitLSP/Swift/OpenInterface.swift @@ -52,7 +52,7 @@ extension SwiftLanguageService { symbol: symbol ) _ = await orLog("Closing generated interface") { - try await self.sourcekitd.send(closeDocumentSourcekitdRequest(uri: interfaceDocURI), fileContents: nil) + try await sendSourcekitdRequest(closeDocumentSourcekitdRequest(uri: interfaceDocURI), fileContents: nil) } return result } @@ -80,7 +80,7 @@ extension SwiftLanguageService { keys.compilerArgs: await self.buildSettings(for: request.textDocument.uri)?.compilerArgs as [SKDRequestValue]?, ]) - let dict = try await self.sourcekitd.send(skreq, fileContents: nil) + let dict = try await sendSourcekitdRequest(skreq, fileContents: nil) return GeneratedInterfaceInfo(contents: dict[keys.sourceText] ?? "") } @@ -101,7 +101,7 @@ extension SwiftLanguageService { keys.usr: symbol, ]) - let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text) + let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) if let offset: Int = dict[keys.offset] { return GeneratedInterfaceDetails(uri: uri, position: snapshot.positionOf(utf8Offset: offset)) } else { diff --git a/Sources/SourceKitLSP/Swift/Refactoring.swift b/Sources/SourceKitLSP/Swift/Refactoring.swift index 0a1477df0..8c5dd9d6b 100644 --- a/Sources/SourceKitLSP/Swift/Refactoring.swift +++ b/Sources/SourceKitLSP/Swift/Refactoring.swift @@ -120,7 +120,7 @@ extension SwiftLanguageService { keys.compilerArgs: await self.buildSettings(for: snapshot.uri)?.compilerArgs as [SKDRequestValue]?, ]) - let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text) + let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) guard let refactor = T.Response(refactorCommand.title, dict, snapshot, self.keys) else { throw SemanticRefactoringError.noEditsNeeded(uri) } diff --git a/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift b/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift index 33aa32f89..555a60dd0 100644 --- a/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift +++ b/Sources/SourceKitLSP/Swift/RelatedIdentifiers.swift @@ -74,7 +74,7 @@ extension SwiftLanguageService { keys.compilerArgs: await self.buildSettings(for: snapshot.uri)?.compilerArgs as [SKDRequestValue]?, ]) - let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text) + let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) guard let results: SKDResponseArray = dict[self.keys.results] else { throw ResponseError.internalError("sourcekitd response did not contain results") diff --git a/Sources/SourceKitLSP/Swift/SemanticTokens.swift b/Sources/SourceKitLSP/Swift/SemanticTokens.swift index 2f3dae96d..86fa8f59e 100644 --- a/Sources/SourceKitLSP/Swift/SemanticTokens.swift +++ b/Sources/SourceKitLSP/Swift/SemanticTokens.swift @@ -30,7 +30,7 @@ extension SwiftLanguageService { keys.compilerArgs: buildSettings.compilerArgs as [SKDRequestValue], ]) - let dict = try await sourcekitd.send(skreq, fileContents: snapshot.text) + let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) guard let skTokens: SKDResponseArray = dict[keys.semanticTokens] else { return nil diff --git a/Sources/SourceKitLSP/Swift/SourceKitD+ResponseError.swift b/Sources/SourceKitLSP/Swift/SourceKitD+ResponseError.swift index 55d239cad..2ccc18f7d 100644 --- a/Sources/SourceKitLSP/Swift/SourceKitD+ResponseError.swift +++ b/Sources/SourceKitLSP/Swift/SourceKitD+ResponseError.swift @@ -18,6 +18,8 @@ extension ResponseError { switch value { case .requestCancelled: self = .cancelled + case .timedOut: + self = .unknown("sourcekitd request timed out") case .requestFailed(let desc): self = .unknown("sourcekitd request failed: \(desc)") case .requestInvalid(let desc): diff --git a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift index 7b36f8cb1..9bf4462fb 100644 --- a/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift +++ b/Sources/SourceKitLSP/Swift/SwiftLanguageService.swift @@ -109,17 +109,16 @@ public actor SwiftLanguageService: LanguageService, Sendable { let testHooks: TestHooks - /// Directory where generated Files will be stored. - let generatedFilesPath: URL + let options: SourceKitLSPOptions /// Directory where generated Swift interfaces will be stored. var generatedInterfacesPath: URL { - generatedFilesPath.appendingPathComponent("GeneratedInterfaces") + options.generatedFilesAbsolutePath.asURL.appendingPathComponent("GeneratedInterfaces") } /// Directory where generated Macro expansions will be stored. var generatedMacroExpansionsPath: URL { - generatedFilesPath.appendingPathComponent("GeneratedMacroExpansions") + options.generatedFilesAbsolutePath.asURL.appendingPathComponent("GeneratedMacroExpansions") } // FIXME: ideally we wouldn't need separate management from a parent server in the same process. @@ -187,6 +186,7 @@ public actor SwiftLanguageService: LanguageService, Sendable { self.documentManager = DocumentManager() self.state = .connected self.diagnosticReportManager = nil // Needed to work around rdar://116221716 + self.options = options // The debounce duration of 500ms was chosen arbitrarily without scientific research. self.refreshDiagnosticsDebouncer = Debouncer(debounceDuration: .milliseconds(500)) { [weak sourceKitLSPServer] in @@ -199,10 +199,9 @@ public actor SwiftLanguageService: LanguageService, Sendable { } } - self.generatedFilesPath = options.generatedFilesAbsolutePath.asURL - self.diagnosticReportManager = DiagnosticReportManager( sourcekitd: self.sourcekitd, + options: options, syntaxTreeManager: syntaxTreeManager, documentManager: documentManager, clientHasDiagnosticsCodeDescriptionSupport: await self.clientHasDiagnosticsCodeDescriptionSupport @@ -236,6 +235,17 @@ public actor SwiftLanguageService: LanguageService, Sendable { } } + func sendSourcekitdRequest( + _ request: SKDRequestDictionary, + fileContents: String? + ) async throws -> SKDResponseDictionary { + try await sourcekitd.send( + request, + timeout: options.sourcekitdRequestTimeoutOrDefault, + fileContents: fileContents + ) + } + public nonisolated func canHandle(workspace: Workspace) -> Bool { // We have a single sourcekitd instance for all workspaces. return true @@ -349,7 +359,7 @@ extension SwiftLanguageService { let req = sourcekitd.dictionary([ keys.request: sourcekitd.requests.crashWithExit ]) - _ = try? await sourcekitd.send(req, fileContents: nil) + _ = try? await sendSourcekitdRequest(req, fileContents: nil) } // MARK: - Build System Integration @@ -365,13 +375,17 @@ extension SwiftLanguageService { await diagnosticReportManager.removeItemsFromCache(with: snapshot.uri) let closeReq = closeDocumentSourcekitdRequest(uri: snapshot.uri) - _ = await orLog("Closing document to re-open it") { try await self.sourcekitd.send(closeReq, fileContents: nil) } + _ = await orLog("Closing document to re-open it") { + try await self.sendSourcekitdRequest(closeReq, fileContents: nil) + } let openReq = openDocumentSourcekitdRequest( snapshot: snapshot, compileCommand: await buildSettings(for: snapshot.uri) ) - _ = await orLog("Re-opening document") { try await self.sourcekitd.send(openReq, fileContents: snapshot.text) } + _ = await orLog("Re-opening document") { + try await self.sendSourcekitdRequest(openReq, fileContents: snapshot.text) + } if await capabilityRegistry.clientSupportsPullDiagnostics(for: .swift) { await self.refreshDiagnosticsDebouncer.scheduleCall() @@ -393,7 +407,7 @@ extension SwiftLanguageService { let req = sourcekitd.dictionary([ keys.request: requests.dependencyUpdated ]) - _ = try await self.sourcekitd.send(req, fileContents: nil) + _ = try await self.sendSourcekitdRequest(req, fileContents: nil) } // `documentUpdatedBuildSettings` already handles reopening the document, so we do that here as well. await self.documentUpdatedBuildSettings(uri) @@ -437,7 +451,7 @@ extension SwiftLanguageService { let buildSettings = await self.buildSettings(for: snapshot.uri) let req = openDocumentSourcekitdRequest(snapshot: snapshot, compileCommand: buildSettings) - _ = try? await self.sourcekitd.send(req, fileContents: snapshot.text) + _ = try? await self.sendSourcekitdRequest(req, fileContents: snapshot.text) await publishDiagnosticsIfNeeded(for: notification.textDocument.uri) } @@ -449,7 +463,7 @@ extension SwiftLanguageService { self.documentManager.close(notification) let req = closeDocumentSourcekitdRequest(uri: notification.textDocument.uri) - _ = try? await self.sourcekitd.send(req, fileContents: nil) + _ = try? await self.sendSourcekitdRequest(req, fileContents: nil) } /// Cancels any in-flight tasks to send a `PublishedDiagnosticsNotification` after edits. @@ -555,7 +569,7 @@ extension SwiftLanguageService { keys.sourceText: edit.replacement, ]) do { - _ = try await self.sourcekitd.send(req, fileContents: nil) + _ = try await self.sendSourcekitdRequest(req, fileContents: nil) } catch { logger.fault( """ diff --git a/Sources/SourceKitLSP/Swift/VariableTypeInfo.swift b/Sources/SourceKitLSP/Swift/VariableTypeInfo.swift index 0ed2bcdc3..81b2deb1b 100644 --- a/Sources/SourceKitLSP/Swift/VariableTypeInfo.swift +++ b/Sources/SourceKitLSP/Swift/VariableTypeInfo.swift @@ -98,7 +98,7 @@ extension SwiftLanguageService { skreq.set(keys.length, to: end - start) } - let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text) + let dict = try await sendSourcekitdRequest(skreq, fileContents: snapshot.text) guard let skVariableTypeInfos: SKDResponseArray = dict[keys.variableTypeList] else { return [] } diff --git a/Tests/DiagnoseTests/DiagnoseTests.swift b/Tests/DiagnoseTests/DiagnoseTests.swift index c1daf1d9c..4e7fdafcb 100644 --- a/Tests/DiagnoseTests/DiagnoseTests.swift +++ b/Tests/DiagnoseTests/DiagnoseTests.swift @@ -325,7 +325,7 @@ private class InProcessSourceKitRequestExecutor: SourceKitRequestExecutor { logger.info("Received response: \(response.description)") switch response.error { - case .requestFailed, .requestInvalid, .requestCancelled, .missingRequiredSymbol, .connectionInterrupted: + case .requestFailed, .requestInvalid, .requestCancelled, .timedOut, .missingRequiredSymbol, .connectionInterrupted: return .error case nil: if reproducerPredicate.evaluate(with: response.description) { diff --git a/Tests/SourceKitDTests/SourceKitDTests.swift b/Tests/SourceKitDTests/SourceKitDTests.swift index f67408d79..f781cd2aa 100644 --- a/Tests/SourceKitDTests/SourceKitDTests.swift +++ b/Tests/SourceKitDTests/SourceKitDTests.swift @@ -82,7 +82,7 @@ final class SourceKitDTests: XCTestCase { keys.compilerArgs: args, ]) - _ = try await sourcekitd.send(req, fileContents: nil) + _ = try await sourcekitd.send(req, timeout: .seconds(defaultTimeout), fileContents: nil) try await fulfillmentOfOrThrow([expectation1, expectation2]) @@ -90,7 +90,7 @@ final class SourceKitDTests: XCTestCase { keys.request: sourcekitd.requests.editorClose, keys.name: path, ]) - _ = try await sourcekitd.send(close, fileContents: nil) + _ = try await sourcekitd.send(close, timeout: .seconds(defaultTimeout), fileContents: nil) } } diff --git a/Tests/SourceKitLSPTests/LocalSwiftTests.swift b/Tests/SourceKitLSPTests/LocalSwiftTests.swift index 9de72a9e7..bd3fe9a5f 100644 --- a/Tests/SourceKitLSPTests/LocalSwiftTests.swift +++ b/Tests/SourceKitLSPTests/LocalSwiftTests.swift @@ -15,6 +15,7 @@ import LSPTestSupport import LanguageServerProtocol import SKCore import SKTestSupport +import SourceKitD @_spi(Testing) import SourceKitLSP import SwiftExtensions import SwiftParser @@ -1401,8 +1402,6 @@ final class LocalSwiftTests: XCTestCase { try SkipUnless.longTestsEnabled() let options = SourceKitLSPOptions(swiftPublishDiagnosticsDebounceDuration: 1 /* second */) - - // Construct our own `TestSourceKitLSPClient` instead of the one from set up because we want a higher debounce interval. let testClient = try await TestSourceKitLSPClient(options: options, usePullDiagnostics: false) let uri = DocumentURI(URL(fileURLWithPath: "/\(UUID())/a.swift")) @@ -1426,4 +1425,51 @@ final class LocalSwiftTests: XCTestCase { // Ensure that we don't get a second `PublishDiagnosticsNotification` await assertThrowsError(try await testClient.nextDiagnosticsNotification(timeout: .seconds(2))) } + + func testSourceKitdTimeout() async throws { + var options = SourceKitLSPOptions.testDefault() + options.sourcekitdRequestTimeout = 1 /* second */ + + let testClient = try await TestSourceKitLSPClient(options: options) + let uri = DocumentURI(for: .swift) + + let positions = testClient.openDocument( + """ + 1️⃣class Foo { + func slow(x: Invalid1, y: Invalid2) { + x / y / x / y / x / y / x / y. + } + }2️⃣ + """, + uri: uri + ) + + let responseBeforeEdit = try await testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + /// The diagnostic request times out, which causes us to return empty diagnostics. + XCTAssertEqual(responseBeforeEdit, .full(RelatedFullDocumentDiagnosticReport(items: []))) + + // Now check that sourcekitd is not blocked. + // Replacing the file and sending another diagnostic request should return proper diagnostics. + testClient.send( + DidChangeTextDocumentNotification( + textDocument: VersionedTextDocumentIdentifier(uri, version: 2), + contentChanges: [ + TextDocumentContentChangeEvent(range: positions["1️⃣"]..