From 447393e0853c5a20d925b43ef55de39243570ebb Mon Sep 17 00:00:00 2001 From: Divya Prakash Date: Sat, 20 Dec 2025 09:49:54 +0530 Subject: [PATCH 1/3] Refactor DocumentManager to use ThreadSafeBox for document storage --- Sources/SourceKitLSP/DocumentManager.swift | 29 ++++++++++------------ 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/Sources/SourceKitLSP/DocumentManager.swift b/Sources/SourceKitLSP/DocumentManager.swift index 91055cd32..cd6767a60 100644 --- a/Sources/SourceKitLSP/DocumentManager.swift +++ b/Sources/SourceKitLSP/DocumentManager.swift @@ -10,9 +10,9 @@ // //===----------------------------------------------------------------------===// -import Dispatch import Foundation @_spi(SourceKitLSP) package import LanguageServerProtocol +import SwiftExtensions @_spi(SourceKitLSP) import SKLogging package import SKUtilities import SemanticIndex @@ -92,18 +92,15 @@ package final class DocumentManager: InMemoryDocumentManager, Sendable { case missingDocument(DocumentURI) } - // TODO: Migrate this to be an AsyncQueue (https://github.com/swiftlang/sourcekit-lsp/issues/1597) - private let queue: DispatchQueue = DispatchQueue(label: "document-manager-queue") - - // `nonisolated(unsafe)` is fine because `documents` is guarded by queue. - private nonisolated(unsafe) var documents: [DocumentURI: Document] = [:] + // Documents storage, protected by a `ThreadSafeBox` to ensure thread safety without making APIs async. + private let documents: ThreadSafeBox<[DocumentURI: Document]> = ThreadSafeBox(initialValue: [:]) package init() {} /// All currently opened documents. package var openDocuments: Set { - return queue.sync { - return Set(documents.keys) + return documents.withLock { docs in + return Set(docs.keys) } } @@ -113,9 +110,9 @@ package final class DocumentManager: InMemoryDocumentManager, Sendable { /// - throws: Error.alreadyOpen if the document is already open. @discardableResult package func open(_ uri: DocumentURI, language: Language, version: Int, text: String) throws -> DocumentSnapshot { - return try queue.sync { + return try documents.withLock { docs in let document = Document(uri: uri, language: language, version: version, text: text) - if nil != documents.updateValue(document, forKey: uri) { + if nil != docs.updateValue(document, forKey: uri) { throw Error.alreadyOpen(uri) } return document.latestSnapshot @@ -127,8 +124,8 @@ package final class DocumentManager: InMemoryDocumentManager, Sendable { /// - returns: The initial contents of the file. /// - throws: Error.missingDocument if the document is not open. package func close(_ uri: DocumentURI) throws { - try queue.sync { - if nil == documents.removeValue(forKey: uri) { + try documents.withLock { docs in + if nil == docs.removeValue(forKey: uri) { throw Error.missingDocument(uri) } } @@ -151,8 +148,8 @@ package final class DocumentManager: InMemoryDocumentManager, Sendable { newVersion: Int, edits: [TextDocumentContentChangeEvent] ) throws -> (preEditSnapshot: DocumentSnapshot, postEditSnapshot: DocumentSnapshot, edits: [SourceEdit]) { - return try queue.sync { - guard let document = documents[uri] else { + return try documents.withLock { docs in + guard let document = docs[uri] else { throw Error.missingDocument(uri) } let preEditSnapshot = document.latestSnapshot @@ -184,8 +181,8 @@ package final class DocumentManager: InMemoryDocumentManager, Sendable { } package func latestSnapshot(_ uri: DocumentURI) throws -> DocumentSnapshot { - return try queue.sync { - guard let document = documents[uri] else { + return try documents.withLock { docs in + guard let document = docs[uri] else { throw ResponseError.unknown("Failed to find snapshot for '\(uri)'") } return document.latestSnapshot From d9072850e0d10851f6135a9359a338cb318de0ce Mon Sep 17 00:00:00 2001 From: Divya Prakash Date: Sat, 20 Dec 2025 09:55:20 +0530 Subject: [PATCH 2/3] Mark Document as @unchecked Sendable to allow storage in ThreadSafeBox --- Sources/SourceKitLSP/DocumentManager.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/SourceKitLSP/DocumentManager.swift b/Sources/SourceKitLSP/DocumentManager.swift index cd6767a60..f664dc22d 100644 --- a/Sources/SourceKitLSP/DocumentManager.swift +++ b/Sources/SourceKitLSP/DocumentManager.swift @@ -12,10 +12,10 @@ import Foundation @_spi(SourceKitLSP) package import LanguageServerProtocol -import SwiftExtensions @_spi(SourceKitLSP) import SKLogging package import SKUtilities import SemanticIndex +import SwiftExtensions package import SwiftSyntax /// An immutable snapshot of a document at a given time. @@ -61,7 +61,7 @@ package struct DocumentSnapshot: Identifiable, Sendable { } } -package final class Document { +package final class Document: @unchecked Sendable { package let uri: DocumentURI package let language: Language var latestVersion: Int From 0ca0660d99896df480ba7935b73cb6be0cb2dbdf Mon Sep 17 00:00:00 2001 From: Divya Prakash Date: Sun, 21 Dec 2025 19:26:02 +0530 Subject: [PATCH 3/3] Refactor DocumentManager threading for Swift 6 Replaces DispatchQueue with ThreadSafeBox, relaxes T: Sendable constraints, and removes @unchecked Sendable from Document. --- Sources/SourceKitD/SourceKitD.swift | 4 +-- Sources/SourceKitLSP/DocumentManager.swift | 2 +- Sources/SwiftExtensions/ThreadSafeBox.swift | 28 ++++++++++++--------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/Sources/SourceKitD/SourceKitD.swift b/Sources/SourceKitD/SourceKitD.swift index f2ef17f9f..0bd434a7c 100644 --- a/Sources/SourceKitD/SourceKitD.swift +++ b/Sources/SourceKitD/SourceKitD.swift @@ -23,8 +23,8 @@ extension sourcekitd_api_values: @unchecked Sendable {} fileprivate extension ThreadSafeBox { /// If the wrapped value is `nil`, run `compute` and store the computed value. If it is not `nil`, return the stored /// value. - func computeIfNil(compute: () -> WrappedValue) -> WrappedValue where T == WrappedValue? { - return withLock { value in + func computeIfNil(compute: () -> WrappedValue) -> WrappedValue where T == WrappedValue? { + return withLock { (value: inout WrappedValue?) -> WrappedValue in if let value { return value } diff --git a/Sources/SourceKitLSP/DocumentManager.swift b/Sources/SourceKitLSP/DocumentManager.swift index f664dc22d..e92d8aa52 100644 --- a/Sources/SourceKitLSP/DocumentManager.swift +++ b/Sources/SourceKitLSP/DocumentManager.swift @@ -61,7 +61,7 @@ package struct DocumentSnapshot: Identifiable, Sendable { } } -package final class Document: @unchecked Sendable { +package final class Document { package let uri: DocumentURI package let language: Language var latestVersion: Int diff --git a/Sources/SwiftExtensions/ThreadSafeBox.swift b/Sources/SwiftExtensions/ThreadSafeBox.swift index bc0ec9be4..c5b29ca7b 100644 --- a/Sources/SwiftExtensions/ThreadSafeBox.swift +++ b/Sources/SwiftExtensions/ThreadSafeBox.swift @@ -15,12 +15,26 @@ import Foundation /// A thread safe container that contains a value of type `T`. /// /// - Note: Unchecked sendable conformance because value is guarded by a lock. -package class ThreadSafeBox: @unchecked Sendable { +package final class ThreadSafeBox: Sendable { /// Lock guarding `_value`. private let lock = NSLock() - private var _value: T + private nonisolated(unsafe) var _value: T + package init(initialValue: T) { + _value = initialValue + } + + /// Restrict the result of the body to `Sendable` so callers can safely use + /// the returned value outside the lock without requiring `T: Sendable`. + package func withLock(_ body: (inout T) throws -> Result) rethrows -> Result { + return try lock.withLock { + return try body(&_value) + } + } +} + +extension ThreadSafeBox where T: Sendable { package var value: T { get { return lock.withLock { @@ -39,16 +53,6 @@ package class ThreadSafeBox: @unchecked Sendable { } } - package init(initialValue: T) { - _value = initialValue - } - - package func withLock(_ body: (inout T) throws -> Result) rethrows -> Result { - return try lock.withLock { - return try body(&_value) - } - } - /// If the value in the box is an optional, return it and reset it to `nil` /// in an atomic operation. package func takeValue() -> T where U? == T {