Skip to content

Commit 9fcf364

Browse files
committed
Fix race condition in rename
We were computing the edits for the rename locations concurrently, which means that we were executing `languageServerType(of:)` concurrently, which could incur concurrent accesses to the `languageServerTypesCache` dictionary. Introduce an actor `LanguageServerTypesCache` that manages the cache, eliminating the race.
1 parent 8ea9c53 commit 9fcf364

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

Sources/SourceKitLSP/Rename.swift

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -701,16 +701,26 @@ extension SourceKitServer {
701701
// First, group all occurrences of that USR by the files they occur in.
702702
var locationsByFile: [URL: [RenameLocation]] = [:]
703703

704-
var languageServerTypesCache: [URL: LanguageServerType?] = [:]
705-
func languageServerType(of url: URL) -> LanguageServerType? {
706-
if let cachedValue = languageServerTypesCache[url] {
707-
return cachedValue
704+
actor LanguageServerTypesCache {
705+
let index: IndexStoreDB
706+
var languageServerTypesCache: [URL: LanguageServerType?] = [:]
707+
708+
init(index: IndexStoreDB) {
709+
self.index = index
710+
}
711+
712+
func languageServerType(for url: URL) -> LanguageServerType? {
713+
if let cachedValue = languageServerTypesCache[url] {
714+
return cachedValue
715+
}
716+
let serverType = LanguageServerType(symbolProvider: index.symbolProvider(for: url.path))
717+
languageServerTypesCache[url] = serverType
718+
return serverType
708719
}
709-
let serverType = LanguageServerType(symbolProvider: index.symbolProvider(for: url.path))
710-
languageServerTypesCache[url] = serverType
711-
return serverType
712720
}
713721

722+
let languageServerTypesCache = LanguageServerTypesCache(index: index)
723+
714724
let usrsToRename = overridingAndOverriddenUsrs(of: usr, index: index)
715725
let occurrencesToRename = usrsToRename.flatMap { index.occurrences(ofUSR: $0, roles: renameRoles) }
716726
for occurrence in occurrencesToRename {
@@ -724,7 +734,7 @@ extension SourceKitServer {
724734
// perform an indexed rename for it.
725735
continue
726736
}
727-
switch languageServerType(of: url) {
737+
switch await languageServerTypesCache.languageServerType(for: url) {
728738
case .swift:
729739
// sourcekitd only produces AST-based results for the direct calls to this USR. This is because the Swift
730740
// AST only has upwards references to superclasses and overridden methods, not the other way round. It is
@@ -755,7 +765,7 @@ extension SourceKitServer {
755765
.concurrentMap { (url: URL, renameLocations: [RenameLocation]) -> (DocumentURI, [TextEdit])? in
756766
let uri = DocumentURI(url)
757767
let language: Language
758-
switch languageServerType(of: url) {
768+
switch await languageServerTypesCache.languageServerType(for: url) {
759769
case .clangd:
760770
// Technically, we still don't know the language of the source file but defaulting to C is sufficient to
761771
// ensure we get the clang toolchain language server, which is all we care about.

Sources/SourceKitLSP/SourceKitServer.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1363,7 +1363,10 @@ extension SourceKitServer {
13631363
cancellationMessageHandlingQueue.async(priority: .high) {
13641364
guard let task = await self.inProgressRequests[notification.id] else {
13651365
logger.error(
1366-
"Cannot cancel request \(notification.id, privacy: .public) because it hasn't been scheduled for execution yet"
1366+
"""
1367+
Cannot cancel request \(notification.id, privacy: .public) because it hasn't been scheduled for execution \
1368+
yet or because the request already returned a response
1369+
"""
13671370
)
13681371
return
13691372
}

0 commit comments

Comments
 (0)