From d788e449cd940e28225be1832b780d53d9a67e06 Mon Sep 17 00:00:00 2001 From: mustiikhalil <26250654+mustiikhalil@users.noreply.github.com> Date: Wed, 5 Feb 2025 19:12:39 +0100 Subject: [PATCH] Uses Native swift Atomics Replaces the usage of CAtomics with the native Atomics from the swift library and bumps the development platform to macOS 15 Removes all references to CAtomics from the library Removes atomic wrappers infavor of using them directly Replacing the usage of Int32, and UInt32 with Int since the previous was only used because of c bridging Uses .sequentiallyConsistent for all loading and storing methods --- Contributor Documentation/Modules.md | 4 - Package.swift | 11 +- .../SwiftPMBuildSystem.swift | 7 +- Sources/CAtomics/CAtomics.c | 0 Sources/CAtomics/CMakeLists.txt | 2 - Sources/CAtomics/include/CAtomics.h | 73 ------------ Sources/CAtomics/include/module.modulemap | 4 - Sources/CMakeLists.txt | 1 - .../TestExtensions.swift | 7 +- Sources/Diagnose/DiagnoseCommand.swift | 14 ++- .../InProcessSourceKitLSPClient.swift | 9 +- .../QueueBasedMessageHandler.swift | 5 +- .../RequestAndReply.swift | 10 +- Sources/SKLogging/NonDarwinLogging.swift | 8 +- .../TestSourceKitLSPClient.swift | 8 +- .../SemanticIndex/IndexTaskDescription.swift | 2 +- .../PreparationTaskDescription.swift | 6 +- Sources/SemanticIndex/TaskScheduler.swift | 24 ++-- .../UpdateIndexStoreTaskDescription.swift | 6 +- .../SourceKitLSP/SourceKitIndexDelegate.swift | 17 +-- .../Swift/CodeCompletionSession.swift | 11 +- Sources/SwiftExtensions/AsyncUtils.swift | 15 ++- Sources/SwiftExtensions/Atomics.swift | 106 ------------------ Sources/SwiftExtensions/CMakeLists.txt | 4 - Sources/TSCExtensions/Process+Run.swift | 7 +- .../SourceKitDRegistryTests.swift | 9 +- .../BackgroundIndexingTests.swift | 7 +- .../PullDiagnosticsTests.swift | 7 +- 28 files changed, 111 insertions(+), 273 deletions(-) delete mode 100644 Sources/CAtomics/CAtomics.c delete mode 100644 Sources/CAtomics/CMakeLists.txt delete mode 100644 Sources/CAtomics/include/CAtomics.h delete mode 100644 Sources/CAtomics/include/module.modulemap delete mode 100644 Sources/SwiftExtensions/Atomics.swift diff --git a/Contributor Documentation/Modules.md b/Contributor Documentation/Modules.md index dfaf54ee6..aefeef73b 100644 --- a/Contributor Documentation/Modules.md +++ b/Contributor Documentation/Modules.md @@ -10,10 +10,6 @@ Swift types to represent the [Build Server Protocol (BSP) specification](https:/ Defines the queries SourceKit-LSP can ask of a build system, like getting compiler arguments for a file, finding a target’s dependencies or preparing a target. -### CAtomics - -Implementation of atomics for Swift using C. Once we can raise our deployment target to use the `Atomic` type from the Swift standard library, this module should be removed. - ### CSKTestSupport For testing, overrides `__cxa_atexit` to prevent registration of static destructors due to work around https://github.com/swiftlang/swift/issues/55112. diff --git a/Package.swift b/Package.swift index 7909805f1..d3e6f9a5a 100644 --- a/Package.swift +++ b/Package.swift @@ -112,13 +112,6 @@ var targets: [Target] = [ swiftSettings: globalSwiftSettings ), - // MARK: CAtomics - - .target( - name: "CAtomics", - dependencies: [] - ), - .target( name: "CCompletionScoring", dependencies: [] @@ -526,14 +519,12 @@ var targets: [Target] = [ .target( name: "SwiftExtensions", - dependencies: ["CAtomics"], exclude: ["CMakeLists.txt"], swiftSettings: globalSwiftSettings ), .target( name: "SwiftExtensionsForPlugin", - dependencies: ["CAtomics"], exclude: ["CMakeLists.txt"], swiftSettings: globalSwiftSettings ), @@ -694,7 +685,7 @@ if buildOnlyTests { let package = Package( name: "SourceKitLSP", - platforms: [.macOS(.v13)], + platforms: [.macOS("15")], products: products, dependencies: dependencies, targets: targets, diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index 7d8f6dd14..abf088de6 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -22,6 +22,7 @@ import SKLogging @preconcurrency package import SPMBuildCore import SourceControl import SwiftExtensions +import Synchronization import TSCExtensions @preconcurrency import Workspace @@ -88,7 +89,7 @@ fileprivate extension TSCBasic.AbsolutePath { } } -fileprivate let preparationTaskID: AtomicUInt32 = AtomicUInt32(initialValue: 0) +fileprivate let preparationTaskID: Atomic = Atomic(0) /// Swift Package Manager build system and workspace support. /// @@ -613,7 +614,9 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem { } let start = ContinuousClock.now - let taskID: TaskId = TaskId(id: "preparation-\(preparationTaskID.fetchAndIncrement())") + let taskID: TaskId = TaskId( + id: "preparation-\(preparationTaskID.add(1, ordering: .sequentiallyConsistent).oldValue)" + ) logMessageToIndexLog( taskID, """ diff --git a/Sources/CAtomics/CAtomics.c b/Sources/CAtomics/CAtomics.c deleted file mode 100644 index e69de29bb..000000000 diff --git a/Sources/CAtomics/CMakeLists.txt b/Sources/CAtomics/CMakeLists.txt deleted file mode 100644 index 9ed253763..000000000 --- a/Sources/CAtomics/CMakeLists.txt +++ /dev/null @@ -1,2 +0,0 @@ -add_library(CAtomics INTERFACE) -target_include_directories(CAtomics INTERFACE "include") diff --git a/Sources/CAtomics/include/CAtomics.h b/Sources/CAtomics/include/CAtomics.h deleted file mode 100644 index fc659a311..000000000 --- a/Sources/CAtomics/include/CAtomics.h +++ /dev/null @@ -1,73 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2014 - 2024 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 -// -//===----------------------------------------------------------------------===// - -#ifndef SOURCEKITLSP_CATOMICS_H -#define SOURCEKITLSP_CATOMICS_H - -#include -#include -#include -#include - -typedef struct { - _Atomic(uint32_t) value; -} CAtomicUInt32; - -static inline CAtomicUInt32 *_Nonnull atomic_uint32_create(uint32_t initialValue) { - CAtomicUInt32 *atomic = malloc(sizeof(CAtomicUInt32)); - atomic->value = initialValue; - return atomic; -} - -static inline uint32_t atomic_uint32_get(CAtomicUInt32 *_Nonnull atomic) { - return atomic->value; -} - -static inline void atomic_uint32_set(CAtomicUInt32 *_Nonnull atomic, uint32_t newValue) { - atomic->value = newValue; -} - -static inline uint32_t atomic_uint32_fetch_and_increment(CAtomicUInt32 *_Nonnull atomic) { - return atomic->value++; -} - -static inline void atomic_uint32_destroy(CAtomicUInt32 *_Nonnull atomic) { - free(atomic); -} - -typedef struct { - _Atomic(int32_t) value; -} CAtomicInt32; - -static inline CAtomicInt32 *_Nonnull atomic_int32_create(int32_t initialValue) { - CAtomicInt32 *atomic = malloc(sizeof(CAtomicInt32)); - atomic->value = initialValue; - return atomic; -} - -static inline int32_t atomic_int32_get(CAtomicInt32 *_Nonnull atomic) { - return atomic->value; -} - -static inline void atomic_int32_set(CAtomicInt32 *_Nonnull atomic, int32_t newValue) { - atomic->value = newValue; -} - -static inline int32_t atomic_int32_fetch_and_increment(CAtomicInt32 *_Nonnull atomic) { - return atomic->value++; -} - -static inline void atomic_int32_destroy(CAtomicInt32 *_Nonnull atomic) { - free(atomic); -} - -#endif // SOURCEKITLSP_CATOMICS_H diff --git a/Sources/CAtomics/include/module.modulemap b/Sources/CAtomics/include/module.modulemap deleted file mode 100644 index 0e9d1d3ed..000000000 --- a/Sources/CAtomics/include/module.modulemap +++ /dev/null @@ -1,4 +0,0 @@ -module CAtomics { - header "CAtomics.h" - export * -} diff --git a/Sources/CMakeLists.txt b/Sources/CMakeLists.txt index 926ea6265..1eefc8239 100644 --- a/Sources/CMakeLists.txt +++ b/Sources/CMakeLists.txt @@ -3,7 +3,6 @@ add_compile_options("$<$:SHELL:-DRESILIENT_LIBRARIES>") add_compile_options("$<$:SHELL:-swift-version 6>") add_subdirectory(BuildServerProtocol) add_subdirectory(BuildSystemIntegration) -add_subdirectory(CAtomics) add_subdirectory(CCompletionScoring) add_subdirectory(CompletionScoring) add_subdirectory(Csourcekitd) diff --git a/Sources/CompletionScoringTestSupport/TestExtensions.swift b/Sources/CompletionScoringTestSupport/TestExtensions.swift index e18ec7f9c..cdca749b3 100644 --- a/Sources/CompletionScoringTestSupport/TestExtensions.swift +++ b/Sources/CompletionScoringTestSupport/TestExtensions.swift @@ -12,6 +12,7 @@ import Foundation import SwiftExtensions +import Synchronization import XCTest #if compiler(>=6) @@ -106,7 +107,7 @@ extension XCTestCase { UserDefaults.standard.string(forKey: "TestMeasurementLogPath") }() - static let printBeginingOfLog = AtomicBool(initialValue: true) + static let printBeginingOfLog: Atomic = Atomic(true) private static func openPerformanceLog() throws -> FileHandle? { try measurementsLogFile.map { path in @@ -119,9 +120,9 @@ extension XCTestCase { } let logFD = try FileHandle(forWritingAtPath: path).unwrap(orThrow: "Opening \(path) failed") try logFD.seekToEnd() - if printBeginingOfLog.value { + if printBeginingOfLog.load(ordering: .sequentiallyConsistent) { try logFD.print("========= \(Date().description(with: .current)) =========") - printBeginingOfLog.value = false + printBeginingOfLog.store(false, ordering: .sequentiallyConsistent) } return logFD } diff --git a/Sources/Diagnose/DiagnoseCommand.swift b/Sources/Diagnose/DiagnoseCommand.swift index 605ee5f72..ab09e6867 100644 --- a/Sources/Diagnose/DiagnoseCommand.swift +++ b/Sources/Diagnose/DiagnoseCommand.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import Synchronization + #if compiler(>=6) package import ArgumentParser import Foundation @@ -233,8 +235,8 @@ package struct DiagnoseCommand: AsyncParsableCommand { throw GenericError("Failed to create log.txt") } let fileHandle = try FileHandle(forWritingTo: outputFileUrl) - let bytesCollected = AtomicInt32(initialValue: 0) - let processExited = AtomicBool(initialValue: false) + let bytesCollected: Atomic = Atomic(0) + let processExited: Atomic = Atomic(false) // 50 MB is an average log size collected by sourcekit-lsp diagnose. // It's a good proxy to show some progress indication for the majority of the time. let expectedLogSize = 50_000_000 @@ -250,8 +252,8 @@ package struct DiagnoseCommand: AsyncParsableCommand { outputRedirection: .stream( stdout: { @Sendable bytes in try? fileHandle.write(contentsOf: bytes) - bytesCollected.value += Int32(bytes.count) - var progress = Double(bytesCollected.value) / Double(expectedLogSize) + let count = bytesCollected.add(bytes.count, ordering: .sequentiallyConsistent).newValue + var progress = Double(count) / Double(expectedLogSize) if progress > 1 { // The log is larger than we expected. Halt at 100% progress = 1 @@ -259,7 +261,7 @@ package struct DiagnoseCommand: AsyncParsableCommand { Task(priority: .high) { // We have launched an async task to call `reportProgress`, which means that the process might have exited // before we execute this task. To avoid overriding a more recent progress, add a guard. - if !processExited.value { + if !processExited.load(ordering: .sequentiallyConsistent) { await reportProgress(.collectingLogMessages(progress: progress), message: "Collecting log messages") } } @@ -269,7 +271,7 @@ package struct DiagnoseCommand: AsyncParsableCommand { ) try process.launch() try await process.waitUntilExit() - processExited.value = true + processExited.store(true, ordering: .sequentiallyConsistent) #endif } diff --git a/Sources/InProcessClient/InProcessSourceKitLSPClient.swift b/Sources/InProcessClient/InProcessSourceKitLSPClient.swift index dbcd25d6b..4a070e78e 100644 --- a/Sources/InProcessClient/InProcessSourceKitLSPClient.swift +++ b/Sources/InProcessClient/InProcessSourceKitLSPClient.swift @@ -15,6 +15,7 @@ public import Foundation public import LanguageServerProtocol import LanguageServerProtocolExtensions import SwiftExtensions +import Synchronization import TSCExtensions import struct TSCBasic.AbsolutePath @@ -33,7 +34,7 @@ import ToolchainRegistry public final class InProcessSourceKitLSPClient: Sendable { private let server: SourceKitLSPServer - private let nextRequestID = AtomicUInt32(initialValue: 0) + private let nextRequestID: Atomic = Atomic(0) public convenience init( toolchainPath: URL?, @@ -101,7 +102,11 @@ public final class InProcessSourceKitLSPClient: Sendable { /// Send the request to `server` and return the request result via a completion handler. public func send(_ request: R, reply: @Sendable @escaping (LSPResult) -> Void) { - server.handle(request, id: .number(Int(nextRequestID.fetchAndIncrement())), reply: reply) + server.handle( + request, + id: .number(Int(nextRequestID.add(1, ordering: .sequentiallyConsistent).oldValue)), + reply: reply + ) } /// Send the notification to `server`. diff --git a/Sources/LanguageServerProtocolExtensions/QueueBasedMessageHandler.swift b/Sources/LanguageServerProtocolExtensions/QueueBasedMessageHandler.swift index 13c41cc07..4d5506ba9 100644 --- a/Sources/LanguageServerProtocolExtensions/QueueBasedMessageHandler.swift +++ b/Sources/LanguageServerProtocolExtensions/QueueBasedMessageHandler.swift @@ -13,6 +13,7 @@ import Foundation import LanguageServerProtocolJSONRPC import SKLogging +import Synchronization #if compiler(>=6) package import LanguageServerProtocol @@ -41,7 +42,7 @@ package actor QueueBasedMessageHandlerHelper { private let cancellationMessageHandlingQueue = AsyncQueue() /// Notifications don't have an ID. This represents the next ID we can use to identify a notification. - private let notificationIDForLogging = AtomicUInt32(initialValue: 1) + private let notificationIDForLogging: Atomic = Atomic(1) /// The requests that we are currently handling. /// @@ -103,7 +104,7 @@ package actor QueueBasedMessageHandlerHelper { // Only use the last two digits of the notification ID for the logging scope to avoid creating too many scopes. // See comment in `withLoggingScope`. // The last 2 digits should be sufficient to differentiate between multiple concurrently running notifications. - let notificationID = notificationIDForLogging.fetchAndIncrement() + let notificationID = notificationIDForLogging.add(1, ordering: .sequentiallyConsistent).oldValue withLoggingScope("notification-\(notificationID % 100)") { body() } diff --git a/Sources/LanguageServerProtocolExtensions/RequestAndReply.swift b/Sources/LanguageServerProtocolExtensions/RequestAndReply.swift index 42ad7db41..d36123cea 100644 --- a/Sources/LanguageServerProtocolExtensions/RequestAndReply.swift +++ b/Sources/LanguageServerProtocolExtensions/RequestAndReply.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import Synchronization + #if compiler(>=6) package import LanguageServerProtocol import SwiftExtensions @@ -24,7 +26,7 @@ package final class RequestAndReply: Sendable { private let replyBlock: @Sendable (LSPResult) -> Void /// Whether a reply has been made. Every request must reply exactly once. - private let replied: AtomicBool = AtomicBool(initialValue: false) + private let replied: Atomic = Atomic(false) package init(_ request: Params, reply: @escaping @Sendable (LSPResult) -> Void) { self.params = request @@ -32,13 +34,13 @@ package final class RequestAndReply: Sendable { } deinit { - precondition(replied.value, "request never received a reply") + precondition(replied.load(ordering: .sequentiallyConsistent), "request never received a reply") } /// Call the `replyBlock` with the result produced by the given closure. package func reply(_ body: @Sendable () async throws -> Params.Response) async { - precondition(!replied.value, "replied to request more than once") - replied.value = true + precondition(!replied.load(ordering: .sequentiallyConsistent), "replied to request more than once") + replied.store(true, ordering: .sequentiallyConsistent) do { replyBlock(.success(try await body())) } catch { diff --git a/Sources/SKLogging/NonDarwinLogging.swift b/Sources/SKLogging/NonDarwinLogging.swift index e156b571c..9e25414c3 100644 --- a/Sources/SKLogging/NonDarwinLogging.swift +++ b/Sources/SKLogging/NonDarwinLogging.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import Synchronization + #if compiler(>=6) package import SwiftExtensions #else @@ -407,14 +409,14 @@ package struct NonDarwinLogger: Sendable { // MARK: - Signposter package struct NonDarwinSignpostID: Sendable { - fileprivate let id: UInt32 + fileprivate let id: Int } package struct NonDarwinSignpostIntervalState: Sendable { fileprivate let id: NonDarwinSignpostID } -private let nextSignpostID = AtomicUInt32(initialValue: 0) +private let nextSignpostID: Atomic = Atomic(0) /// A type that is API-compatible to `OSLogMessage` for all uses within sourcekit-lsp. /// @@ -427,7 +429,7 @@ package struct NonDarwinSignposter: Sendable { } package func makeSignpostID() -> NonDarwinSignpostID { - return NonDarwinSignpostID(id: nextSignpostID.fetchAndIncrement()) + return NonDarwinSignpostID(id: nextSignpostID.add(1, ordering: .sequentiallyConsistent).oldValue) } package func beginInterval( diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index df6d671cd..9ac9317a7 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -18,6 +18,7 @@ import SKUtilities import SourceKitD import SwiftExtensions import SwiftSyntax +import Synchronization import XCTest #if compiler(>=6) @@ -88,7 +89,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable { package typealias RequestHandler = @Sendable (Request) -> Request.Response /// The ID that should be assigned to the next request sent to the `server`. - private let nextRequestID = AtomicUInt32(initialValue: 0) + private let nextRequestID: Atomic = Atomic(0) /// The server that handles the requests. package let server: SourceKitLSPServer @@ -222,7 +223,8 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable { // It's really unfortunate that there are no async deinits. If we had async // deinits, we could await the sending of a ShutdownRequest. let sema = DispatchSemaphore(value: 0) - server.handle(ShutdownRequest(), id: .number(Int(nextRequestID.fetchAndIncrement()))) { result in + server.handle(ShutdownRequest(), id: .number(Int(nextRequestID.add(1, ordering: .sequentiallyConsistent).oldValue))) + { result in sema.signal() } sema.wait() @@ -257,7 +259,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable { _ request: R, completionHandler: @Sendable @escaping (LSPResult) -> Void ) -> RequestID { - let requestID = RequestID.number(Int(nextRequestID.fetchAndIncrement())) + let requestID = RequestID.number(Int(nextRequestID.add(1, ordering: .sequentiallyConsistent).oldValue)) server.handle(request, id: requestID) { result in completionHandler(result) } diff --git a/Sources/SemanticIndex/IndexTaskDescription.swift b/Sources/SemanticIndex/IndexTaskDescription.swift index d687561ec..bbcbfddc8 100644 --- a/Sources/SemanticIndex/IndexTaskDescription.swift +++ b/Sources/SemanticIndex/IndexTaskDescription.swift @@ -20,7 +20,7 @@ protocol IndexTaskDescription: TaskDescriptionProtocol { /// different types in `AnyIndexTaskDescription` static var idPrefix: String { get } - var id: UInt32 { get } + var id: Int { get } } extension IndexTaskDescription { diff --git a/Sources/SemanticIndex/PreparationTaskDescription.swift b/Sources/SemanticIndex/PreparationTaskDescription.swift index 397ff3e09..13a1822af 100644 --- a/Sources/SemanticIndex/PreparationTaskDescription.swift +++ b/Sources/SemanticIndex/PreparationTaskDescription.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import Synchronization + #if compiler(>=6) package import BuildServerProtocol import BuildSystemIntegration @@ -34,7 +36,7 @@ import struct TSCBasic.AbsolutePath import class TSCBasic.Process #endif -private let preparationIDForLogging = AtomicUInt32(initialValue: 1) +private let preparationIDForLogging: Atomic = Atomic(1) /// Describes a task to prepare a set of targets. /// @@ -42,7 +44,7 @@ private let preparationIDForLogging = AtomicUInt32(initialValue: 1) package struct PreparationTaskDescription: IndexTaskDescription { package static let idPrefix = "prepare" - package let id = preparationIDForLogging.fetchAndIncrement() + package let id = preparationIDForLogging.add(1, ordering: .sequentiallyConsistent).oldValue /// The targets that should be prepared. package let targetsToPrepare: [BuildTargetIdentifier] diff --git a/Sources/SemanticIndex/TaskScheduler.swift b/Sources/SemanticIndex/TaskScheduler.swift index 33e269ae6..8c88c13f3 100644 --- a/Sources/SemanticIndex/TaskScheduler.swift +++ b/Sources/SemanticIndex/TaskScheduler.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import Synchronization + #if compiler(>=6) import Foundation import LanguageServerProtocolExtensions @@ -133,7 +135,7 @@ package actor QueuedTask { /// Every time `execute` gets called, a new task is placed in this continuation. See comment on `executionTask`. private let executionTaskCreatedContinuation: AsyncStream>.Continuation - private let _priority: AtomicUInt8 + private let _priority: Atomic /// The latest known priority of the task. /// @@ -141,10 +143,10 @@ package actor QueuedTask { /// it, the priority may get elevated. nonisolated var priority: TaskPriority { get { - TaskPriority(rawValue: _priority.value) + TaskPriority(rawValue: _priority.load(ordering: .sequentiallyConsistent)) } set { - _priority.value = newValue.rawValue + _priority.store(newValue.rawValue, ordering: .sequentiallyConsistent) } } @@ -154,13 +156,13 @@ package actor QueuedTask { private var cancelledToBeRescheduled: Bool = false /// Whether `resultTask` has been cancelled. - private let resultTaskCancelled: AtomicBool = .init(initialValue: false) + private let resultTaskCancelled: Atomic = Atomic(false) - private let _isExecuting: AtomicBool = .init(initialValue: false) + private let _isExecuting: Atomic = Atomic(false) /// Whether the task is currently executing or still queued to be executed later. package nonisolated var isExecuting: Bool { - return _isExecuting.value + return _isExecuting.load(ordering: .sequentiallyConsistent) } package nonisolated func cancel() { @@ -193,7 +195,7 @@ package actor QueuedTask { description: TaskDescription, executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)? ) async { - self._priority = AtomicUInt8(initialValue: priority.rawValue) + self._priority = Atomic(priority.rawValue) self.description = description self.executionStateChangedCallback = executionStateChangedCallback @@ -225,7 +227,7 @@ package actor QueuedTask { self.priority = Task.currentPriority } } onCancel: { - self.resultTaskCancelled.value = true + self.resultTaskCancelled.store(true, ordering: .sequentiallyConsistent) } } } @@ -246,12 +248,12 @@ package actor QueuedTask { } precondition(executionTask == nil, "Task started twice") let task = Task.detached(priority: self.priority) { - if !Task.isCancelled && !self.resultTaskCancelled.value { + if !Task.isCancelled && !self.resultTaskCancelled.load(ordering: .sequentiallyConsistent) { await self.description.execute() } return await self.finalizeExecution() } - _isExecuting.value = true + _isExecuting.store(true, ordering: .sequentiallyConsistent) executionTask = task executionTaskCreatedContinuation.yield(task) await executionStateChangedCallback?(self, .executing) @@ -261,7 +263,7 @@ package actor QueuedTask { /// Implementation detail of `execute` that is called after `self.description.execute()` finishes. private func finalizeExecution() async -> ExecutionTaskFinishStatus { self.executionTask = nil - _isExecuting.value = false + _isExecuting.store(false, ordering: .sequentiallyConsistent) if Task.isCancelled && self.cancelledToBeRescheduled { await executionStateChangedCallback?(self, .cancelledToBeRescheduled) self.cancelledToBeRescheduled = false diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index 0bfc9bcea..35a0b1588 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import Synchronization + #if compiler(>=6) package import BuildServerProtocol import BuildSystemIntegration @@ -40,7 +42,7 @@ import class TSCBasic.Process import struct TSCBasic.ProcessResult #endif -private let updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1) +private let updateIndexStoreIDForLogging: Atomic = Atomic(1) package enum FileToIndex: CustomLogStringConvertible { /// A non-header file @@ -117,7 +119,7 @@ private enum IndexKind { /// This task description can be scheduled in a `TaskScheduler`. package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { package static let idPrefix = "update-indexstore" - package let id = updateIndexStoreIDForLogging.fetchAndIncrement() + package let id = updateIndexStoreIDForLogging.add(1, ordering: .sequentiallyConsistent).oldValue /// The files that should be indexed. package let filesToIndex: [FileAndTarget] diff --git a/Sources/SourceKitLSP/SourceKitIndexDelegate.swift b/Sources/SourceKitLSP/SourceKitIndexDelegate.swift index c1e57e277..ffd795d64 100644 --- a/Sources/SourceKitLSP/SourceKitIndexDelegate.swift +++ b/Sources/SourceKitLSP/SourceKitIndexDelegate.swift @@ -15,6 +15,7 @@ import IndexStoreDB import LanguageServerProtocolExtensions import SKLogging import SwiftExtensions +import Synchronization /// `IndexDelegate` for the SourceKit workspace. actor SourceKitIndexDelegate: IndexDelegate { @@ -24,27 +25,29 @@ actor SourceKitIndexDelegate: IndexDelegate { /// The count of pending unit events. Whenever this transitions to 0, it represents a time where /// the index finished processing known events. Of course, that may have already changed by the /// time we are notified. - let pendingUnitCount = AtomicInt32(initialValue: 0) + let pendingUnitCount: Atomic = Atomic(0) package init() {} nonisolated package func processingAddedPending(_ count: Int) { - pendingUnitCount.value += Int32(count) + pendingUnitCount.add(count, ordering: .sequentiallyConsistent) } nonisolated package func processingCompleted(_ count: Int) { - pendingUnitCount.value -= Int32(count) - if pendingUnitCount.value == 0 { + let count = pendingUnitCount.subtract(count, ordering: .sequentiallyConsistent).newValue + if count == 0 { Task { await indexChanged() } } - if pendingUnitCount.value < 0 { + if count < 0 { // Technically this is not data race safe because `pendingUnitCount` might change between the check and us setting // it to 0. But then, this should never happen anyway, so it's fine. - logger.fault("pendingUnitCount dropped below zero: \(self.pendingUnitCount.value)") - pendingUnitCount.value = 0 + logger.fault( + "pendingUnitCount dropped below zero: \(self.pendingUnitCount.load(ordering: .sequentiallyConsistent))" + ) + pendingUnitCount.store(0, ordering: .sequentiallyConsistent) Task { await indexChanged() } diff --git a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift index f8edfac81..1c0ab28ba 100644 --- a/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift +++ b/Sources/SourceKitLSP/Swift/CodeCompletionSession.swift @@ -22,20 +22,21 @@ import SwiftExtensions import SwiftParser @_spi(SourceKitLSP) import SwiftRefactor import SwiftSyntax +import Synchronization /// Uniquely identifies a code completion session. We need this so that when resolving a code completion item, we can /// verify that the item to resolve belongs to the code completion session that is currently open. struct CompletionSessionID: Equatable { - private static let nextSessionID = AtomicUInt32(initialValue: 0) + private static let nextSessionID: Atomic = Atomic(0) - let value: UInt32 + let value: Int - init(value: UInt32) { + init(value: Int) { self.value = value } static func next() -> CompletionSessionID { - return CompletionSessionID(value: nextSessionID.fetchAndIncrement()) + return CompletionSessionID(value: nextSessionID.add(1, ordering: .sequentiallyConsistent).oldValue) } } @@ -60,7 +61,7 @@ struct CompletionItemData: LSPAnyCodable { return nil } self.uri = uri - self.sessionId = CompletionSessionID(value: UInt32(sessionId)) + self.sessionId = CompletionSessionID(value: sessionId) self.itemId = itemId } diff --git a/Sources/SwiftExtensions/AsyncUtils.swift b/Sources/SwiftExtensions/AsyncUtils.swift index 9530b91d2..b23fa7cfc 100644 --- a/Sources/SwiftExtensions/AsyncUtils.swift +++ b/Sources/SwiftExtensions/AsyncUtils.swift @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// import Foundation +import Synchronization /// Wrapper around a task that allows multiple clients to depend on the task's value. /// @@ -227,6 +228,14 @@ package func withTimeout( } } +private final class Wrapped: Sendable { + package let value: T + + package init(_ value: consuming T) { + self.value = value + } +} + /// Executes `body`. If it doesn't finish after `duration`, return `nil` and continue running body. When `body` returns /// a value after the timeout, `resultReceivedAfterTimeout` is called. /// @@ -237,19 +246,19 @@ package func withTimeout( body: @escaping @Sendable () async throws -> T?, resultReceivedAfterTimeout: @escaping @Sendable () async -> Void ) async throws -> T? { - let didHitTimeout = AtomicBool(initialValue: false) + let didHitTimeout: Wrapped> = Wrapped(Atomic(false)) let stream = AsyncThrowingStream { continuation in Task { try await Task.sleep(for: timeout) - didHitTimeout.value = true + didHitTimeout.value.store(true, ordering: .sequentiallyConsistent) continuation.yield(nil) } Task { do { let result = try await body() - if didHitTimeout.value { + if didHitTimeout.value.load(ordering: .sequentiallyConsistent) { await resultReceivedAfterTimeout() } continuation.yield(result) diff --git a/Sources/SwiftExtensions/Atomics.swift b/Sources/SwiftExtensions/Atomics.swift deleted file mode 100644 index f930704fa..000000000 --- a/Sources/SwiftExtensions/Atomics.swift +++ /dev/null @@ -1,106 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2014 - 2024 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 -// -//===----------------------------------------------------------------------===// - -import CAtomics - -// TODO: Use atomic types from the standard library (https://github.com/swiftlang/sourcekit-lsp/issues/1949) -package final class AtomicBool: Sendable { - private nonisolated(unsafe) let atomic: UnsafeMutablePointer - - package init(initialValue: Bool) { - self.atomic = atomic_uint32_create(initialValue ? 1 : 0) - } - - deinit { - atomic_uint32_destroy(atomic) - } - - package var value: Bool { - get { - atomic_uint32_get(atomic) != 0 - } - set { - atomic_uint32_set(atomic, newValue ? 1 : 0) - } - } -} - -package final class AtomicUInt8: Sendable { - private nonisolated(unsafe) let atomic: UnsafeMutablePointer - - package init(initialValue: UInt8) { - self.atomic = atomic_uint32_create(UInt32(initialValue)) - } - - deinit { - atomic_uint32_destroy(atomic) - } - - package var value: UInt8 { - get { - UInt8(atomic_uint32_get(atomic)) - } - set { - atomic_uint32_set(atomic, UInt32(newValue)) - } - } -} - -package final class AtomicUInt32: Sendable { - private nonisolated(unsafe) let atomic: UnsafeMutablePointer - - package init(initialValue: UInt32) { - self.atomic = atomic_uint32_create(initialValue) - } - - package var value: UInt32 { - get { - atomic_uint32_get(atomic) - } - set { - atomic_uint32_set(atomic, newValue) - } - } - - deinit { - atomic_uint32_destroy(atomic) - } - - package func fetchAndIncrement() -> UInt32 { - return atomic_uint32_fetch_and_increment(atomic) - } -} - -package final class AtomicInt32: Sendable { - private nonisolated(unsafe) let atomic: UnsafeMutablePointer - - package init(initialValue: Int32) { - self.atomic = atomic_int32_create(initialValue) - } - - package var value: Int32 { - get { - atomic_int32_get(atomic) - } - set { - atomic_int32_set(atomic, newValue) - } - } - - deinit { - atomic_int32_destroy(atomic) - } - - package func fetchAndIncrement() -> Int32 { - return atomic_int32_fetch_and_increment(atomic) - } -} diff --git a/Sources/SwiftExtensions/CMakeLists.txt b/Sources/SwiftExtensions/CMakeLists.txt index 12e12acfb..0ce602e32 100644 --- a/Sources/SwiftExtensions/CMakeLists.txt +++ b/Sources/SwiftExtensions/CMakeLists.txt @@ -25,15 +25,11 @@ set(sources add_library(SwiftExtensions STATIC ${sources}) set_target_properties(SwiftExtensions PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY}) -target_link_libraries(SwiftExtensions PUBLIC - CAtomics) target_link_libraries(SwiftExtensions PRIVATE $<$>:Foundation>) add_library(SwiftExtensionsForPlugin STATIC ${sources}) set_target_properties(SwiftExtensionsForPlugin PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY}) -target_link_libraries(SwiftExtensionsForPlugin PUBLIC - CAtomics) target_link_libraries(SwiftExtensionsForPlugin PRIVATE $<$>:Foundation>) diff --git a/Sources/TSCExtensions/Process+Run.swift b/Sources/TSCExtensions/Process+Run.swift index ec1b638e7..486cf3808 100644 --- a/Sources/TSCExtensions/Process+Run.swift +++ b/Sources/TSCExtensions/Process+Run.swift @@ -13,6 +13,7 @@ import Foundation import SKLogging import SwiftExtensions +import Synchronization #if compiler(>=6) package import struct TSCBasic.AbsolutePath @@ -39,10 +40,10 @@ extension Process { /// Should the process not terminate on SIGINT after 2 seconds, it is killed using `SIGKILL`. @discardableResult package func waitUntilExitStoppingProcessOnTaskCancellation() async throws -> ProcessResult { - let hasExited = AtomicBool(initialValue: false) + let hasExited = Atomic(false) return try await withTaskCancellationHandler { defer { - hasExited.value = true + hasExited.store(true, ordering: .sequentiallyConsistent) } return try await waitUntilExit() } onCancel: { @@ -50,7 +51,7 @@ extension Process { Task { // Give the process 2 seconds to react to a SIGINT. If that doesn't work, kill the process. try await Task.sleep(for: .seconds(2)) - if !hasExited.value { + if !hasExited.load(ordering: .sequentiallyConsistent) { #if os(Windows) // Windows does not define SIGKILL. Process.signal sends a `terminate` to the underlying Foundation process // for any signal that is not SIGINT. Use `SIGABRT` to terminate the process. diff --git a/Tests/SourceKitDTests/SourceKitDRegistryTests.swift b/Tests/SourceKitDTests/SourceKitDRegistryTests.swift index c98f4a9b6..f9f6517e8 100644 --- a/Tests/SourceKitDTests/SourceKitDRegistryTests.swift +++ b/Tests/SourceKitDTests/SourceKitDRegistryTests.swift @@ -15,6 +15,7 @@ import LanguageServerProtocolExtensions import SKTestSupport import SourceKitD import SwiftExtensions +import Synchronization import TSCBasic import XCTest @@ -43,7 +44,7 @@ final class SourceKitDRegistryTests: XCTestCase { let registry = SourceKitDRegistry() @inline(never) - func scope(registry: SourceKitDRegistry) async throws -> UInt32 { + func scope(registry: SourceKitDRegistry) async throws -> Int { let a = await FakeSourceKitD.getOrCreate(URL(fileURLWithPath: "/a"), in: registry) await assertTrue(a === FakeSourceKitD.getOrCreate(URL(fileURLWithPath: "/a"), in: registry)) @@ -61,10 +62,10 @@ final class SourceKitDRegistryTests: XCTestCase { } } -private let nextToken = AtomicUInt32(initialValue: 0) +private let nextToken: Atomic = Atomic(0) final class FakeSourceKitD: SourceKitD { - let token: UInt32 + let token: Int var api: sourcekitd_api_functions_t { fatalError() } var ideApi: sourcekitd_ide_api_functions_t { fatalError() } var pluginApi: sourcekitd_plugin_api_functions_t { fatalError() } @@ -75,7 +76,7 @@ final class FakeSourceKitD: SourceKitD { func addNotificationHandler(_ handler: SKDNotificationHandler) { fatalError() } func removeNotificationHandler(_ handler: SKDNotificationHandler) { fatalError() } private init() { - token = nextToken.fetchAndIncrement() + token = nextToken.add(1, ordering: .sequentiallyConsistent).oldValue } static func getOrCreate(_ url: URL, in registry: SourceKitDRegistry) async -> SourceKitD { diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index 534890de9..fa3c1d141 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -19,6 +19,7 @@ import SKTestSupport import SemanticIndex import SourceKitLSP import SwiftExtensions +import Synchronization import TSCExtensions import ToolchainRegistry import XCTest @@ -1307,11 +1308,11 @@ final class BackgroundIndexingTests: XCTestCase { } func testAddingRandomSwiftFileDoesNotTriggerPackageReload() async throws { - let packageInitialized = AtomicBool(initialValue: false) + let packageInitialized: Atomic = Atomic(false) var testHooks = Hooks() testHooks.buildSystemHooks.swiftPMTestHooks.reloadPackageDidStart = { - if packageInitialized.value { + if packageInitialized.load(ordering: .sequentiallyConsistent) { XCTFail("Build graph should not get reloaded when random file gets added") } } @@ -1320,7 +1321,7 @@ final class BackgroundIndexingTests: XCTestCase { hooks: testHooks, enableBackgroundIndexing: true ) - packageInitialized.value = true + packageInitialized.store(true, ordering: .sequentiallyConsistent) project.testClient.send( DidChangeWatchedFilesNotification(changes: [ FileEvent(uri: DocumentURI(project.scratchDirectory.appendingPathComponent("random.swift")), type: .created) diff --git a/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift b/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift index 42236a676..fa39526a8 100644 --- a/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift +++ b/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift @@ -19,6 +19,7 @@ import SKTestSupport import SemanticIndex import SourceKitLSP import SwiftExtensions +import Synchronization import XCTest #if os(Windows) @@ -279,13 +280,13 @@ final class PullDiagnosticsTests: XCTestCase { } func testDiagnosticsWaitForDocumentToBePrepared() async throws { - let diagnosticRequestSent = AtomicBool(initialValue: false) + let diagnosticRequestSent: Atomic = Atomic(false) var testHooks = Hooks() testHooks.indexHooks.preparationTaskDidStart = { @Sendable taskDescription in // Only start preparation after we sent the diagnostic request. In almost all cases, this should not give // preparation enough time to finish before the diagnostic request is handled unless we wait for preparation in // the diagnostic request. - while diagnosticRequestSent.value == false { + while diagnosticRequestSent.load(ordering: .sequentiallyConsistent) == false { do { try await Task.sleep(for: .seconds(0.01)) } catch { @@ -331,7 +332,7 @@ final class PullDiagnosticsTests: XCTestCase { XCTAssertEqual(diagnostics.success?.fullReport?.items, []) receivedDiagnostics.fulfill() } - diagnosticRequestSent.value = true + diagnosticRequestSent.store(true, ordering: .sequentiallyConsistent) try await fulfillmentOfOrThrow([receivedDiagnostics]) }