Refactor ThreadSafeBox to use non-optional type (#9477)

Change the underlying storage type from `Value?` to storing `Value`,
with optional-specific operations moved to constrained extensions.

This improves type safety by eliminating unnecessary optionality for
non-optional types.

This stronger type constraint makes it more difficult for the struct to
be misused. For instance in the current implementation the extension
method `increment()` will fail to increment if the user forgot to
initialize the `ThreadSafeBox` with a value. Now a value must be set
initially before increment is called.

Also this patch prevents a race condition in the `memoize` methods where
the call to the memoized `body` was not guarded by a lock which allowed
multiple threads to call memoize at once and produce inconsistent
results.

Finally, fix the subscript setter so it works properly with `structs`,
as the `if var value` and then value set will set the value on a copy,
not on the `self.underlying` struct. Now that the type is no longer
optional this unwrapping isn't necessary and we can set directly on the
value.

Author: plemarquand

Sources/Basics/Cancellator.swift

@@ -99,7 +99,7 @@ public final class Cancellator: Cancellable, Sendable {
 
     @discardableResult
     public func register(name: String, handler: @escaping CancellationHandler) -> RegistrationKey? {
-        if self.cancelling.get(default: false) {
+        if self.cancelling.get() {
             self.observabilityScope?.emit(debug: "not registering '\(name)' with terminator, termination in progress")
             return .none
         }

Sources/Basics/Concurrency/AsyncProcess.swift

@@ -806,7 +806,7 @@ package final class AsyncProcess {
     package func waitUntilExit() throws -> AsyncProcessResult {
         let group = DispatchGroup()
         group.enter()
-        let resultBox = ThreadSafeBox<Result<AsyncProcessResult, Swift.Error>>()
+        let resultBox = ThreadSafeBox<Result<AsyncProcessResult, Swift.Error>?>()
         self.waitUntilExit { result in
             resultBox.put(result)
             group.leave()

Sources/Basics/Concurrency/ConcurrencyHelpers.swift

@@ -30,7 +30,7 @@ public enum Concurrency {
 public func unsafe_await<T: Sendable>(_ body: @Sendable @escaping () async -> T) -> T {
     let semaphore = DispatchSemaphore(value: 0)
 
-    let box = ThreadSafeBox<T>()
+    let box = ThreadSafeBox<T?>()
     Task {
         let localValue: T = await body()
         box.mutate { _ in localValue }

Sources/Basics/Concurrency/ThreadSafeBox.swift

@@ -12,135 +12,211 @@
 
 import class Foundation.NSLock
 
-/// Thread-safe value boxing structure
+/// Thread-safe value boxing structure that provides synchronized access to a wrapped value.
 @dynamicMemberLookup
 public final class ThreadSafeBox<Value> {
-    private var underlying: Value?
+    private var underlying: Value
     private let lock = NSLock()
 
-    public init() {}
-
+    /// Creates a new thread-safe box with the given initial value.
+    ///
+    /// - Parameter seed: The initial value to store in the box.
     public init(_ seed: Value) {
         self.underlying = seed
     }
 
-    public func mutate(body: (Value?) throws -> Value?) rethrows {
+    /// Atomically mutates the stored value by applying a transformation function.
+    ///
+    /// The transformation function receives the current value and returns a new value
+    /// to replace it. The entire operation is performed under a lock to ensure atomicity.
+    ///
+    /// - Parameter body: A closure that takes the current value and returns a new value.
+    /// - Throws: Any error thrown by the transformation function.
+    public func mutate(body: (Value) throws -> Value) rethrows {
         try self.lock.withLock {
             let value = try body(self.underlying)
             self.underlying = value
         }
     }
-    
-    public func mutate(body: (inout Value?) throws -> ()) rethrows {
+
+    /// Atomically mutates the stored value by applying an in-place transformation.
+    ///
+    /// The transformation function receives an inout reference to the current value,
+    /// allowing direct modification. The entire operation is performed under a lock
+    /// to ensure atomicity.
+    ///
+    /// - Parameter body: A closure that receives an inout reference to the current value.
+    /// - Throws: Any error thrown by the transformation function.
+    public func mutate(body: (inout Value) throws -> Void) rethrows {
         try self.lock.withLock {
             try body(&self.underlying)
         }
     }
 
-    @discardableResult
-    public func memoize(body: () throws -> Value) rethrows -> Value {
-        if let value = self.get() {
-            return value
-        }
-        let value = try body()
+    /// Atomically retrieves the current value from the box.
+    ///
+    /// - Returns: A copy of the current value stored in the box.
+    public func get() -> Value {
         self.lock.withLock {
-            self.underlying = value
+            self.underlying
         }
-        return value
     }
 
-    @discardableResult
-    public func memoize(body: () async throws -> Value) async rethrows -> Value {
-        if let value = self.get() {
-            return value
-        }
-        let value = try await body()
+    /// Atomically replaces the current value with a new value.
+    ///
+    /// - Parameter newValue: The new value to store in the box.
+    public func put(_ newValue: Value) {
         self.lock.withLock {
-            self.underlying = value
+            self.underlying = newValue
         }
-        return value
     }
 
-    public func clear() {
+    /// Provides thread-safe read-only access to properties of the wrapped value.
+    ///
+    /// This subscript allows you to access properties of the wrapped value using
+    /// dot notation while maintaining thread safety.
+    ///
+    /// - Parameter keyPath: A key path to a property of the wrapped value.
+    /// - Returns: The value of the specified property.
+    public subscript<T>(dynamicMember keyPath: KeyPath<Value, T>) -> T {
         self.lock.withLock {
-            self.underlying = nil
+            self.underlying[keyPath: keyPath]
         }
     }
 
-    public func get() -> Value? {
-        self.lock.withLock {
-            self.underlying
+    /// Provides thread-safe read-write access to properties of the wrapped value.
+    ///
+    /// - Parameter keyPath: A writable key path to a property of the wrapped value.
+    /// - Returns: The value of the specified property when getting.
+    public subscript<T>(dynamicMember keyPath: WritableKeyPath<Value, T>) -> T {
+        get {
+            self.lock.withLock {
+                self.underlying[keyPath: keyPath]
+            }
+        }
+        set {
+            self.lock.withLock {
+                self.underlying[keyPath: keyPath] = newValue
+            }
         }
     }
+}
 
-    public func get(default: Value) -> Value {
-        self.lock.withLock {
-            self.underlying ?? `default`
-        }
+// Extension for optional values to support empty initialization
+extension ThreadSafeBox {
+    /// Creates a new thread-safe box initialized with nil for optional value types.
+    ///
+    /// This convenience initializer is only available when the wrapped value type is optional.
+    public convenience init<Wrapped>() where Value == Wrapped? {
+        self.init(nil)
     }
 
-    public func put(_ newValue: Value) {
+    /// Takes the stored optional value, setting it to nil.
+    /// - Returns: The previously stored value, or nil if none was present.
+    public func takeValue<Wrapped>() -> Value where Value == Wrapped? {
         self.lock.withLock {
-            self.underlying = newValue
+            guard let value = self.underlying else { return nil }
+            self.underlying = nil
+            return value
         }
     }
 
-    public func takeValue<U>() -> Value where U? == Value {
+    /// Atomically sets the stored optional value to nil.
+    ///
+    /// This method is only available when the wrapped value type is optional.
+    public func clear<Wrapped>() where Value == Wrapped? {
         self.lock.withLock {
-            guard let value = self.underlying else { return nil }
             self.underlying = nil
-            return value
         }
     }
 
-    public subscript<T>(dynamicMember keyPath: KeyPath<Value, T>) -> T? {
+    /// Atomically retrieves the stored value, returning a default if nil.
+    ///
+    /// This method is only available when the wrapped value type is optional.
+    ///
+    /// - Parameter defaultValue: The value to return if the stored value is nil.
+    /// - Returns: The stored value if not nil, otherwise the default value.
+    public func get<Wrapped>(default defaultValue: Wrapped) -> Wrapped where Value == Wrapped? {
         self.lock.withLock {
-            self.underlying?[keyPath: keyPath]
+            self.underlying ?? defaultValue
         }
     }
 
-    public subscript<T>(dynamicMember keyPath: WritableKeyPath<Value, T?>) -> T? {
-        get {
-            self.lock.withLock {
-                self.underlying?[keyPath: keyPath]
+    /// Atomically computes and caches a value if not already present.
+    ///
+    /// If the box already contains a non-nil value, that value is returned immediately.
+    /// Otherwise, the provided closure is executed to compute the value, which is then
+    /// stored and returned. This method is only available when the wrapped value type is optional.
+    ///
+    /// - Parameter body: A closure that computes the value to store if none exists.
+    /// - Returns: The cached value or the newly computed value.
+    /// - Throws: Any error thrown by the computation closure.
+    @discardableResult
+    public func memoize<Wrapped>(body: () throws -> Wrapped) rethrows -> Wrapped where Value == Wrapped? {
+        try self.lock.withLock {
+            if let value = self.underlying {
+                return value
             }
+            let value = try body()
+            self.underlying = value
+            return value
         }
-        set {
-            self.lock.withLock {
-                if var value = self.underlying {
-                    value[keyPath: keyPath] = newValue
-                }
+    }
+
+    /// Atomically computes and caches an optional value if not already present.
+    ///
+    /// If the box already contains a non-nil value, that value is returned immediately.
+    /// Otherwise, the provided closure is executed to compute the value, which is then
+    /// stored and returned. This method is only available when the wrapped value type is optional.
+    ///
+    /// If the returned value is `nil` subsequent calls to `memoize` or `memoizeOptional` will
+    /// re-execute the closure.
+    ///
+    /// - Parameter body: A closure that computes the optional value to store if none exists.
+    /// - Returns: The cached value or the newly computed value (which may be nil).
+    /// - Throws: Any error thrown by the computation closure.
+    @discardableResult
+    public func memoizeOptional<Wrapped>(body: () throws -> Wrapped?) rethrows -> Wrapped? where Value == Wrapped? {
+        try self.lock.withLock {
+            if let value = self.underlying {
+                return value
             }
+            let value = try body()
+            self.underlying = value
+            return value
         }
     }
 }
 
 extension ThreadSafeBox where Value == Int {
+    /// Atomically increments the stored integer value by 1.
+    ///
+    /// This method is only available when the wrapped value type is Int.
     public func increment() {
         self.lock.withLock {
-            if let value = self.underlying {
-                self.underlying = value + 1
-            }
+            self.underlying = self.underlying + 1
         }
     }
 
+    /// Atomically decrements the stored integer value by 1.
+    ///
+    /// This method is only available when the wrapped value type is Int.
     public func decrement() {
         self.lock.withLock {
-            if let value = self.underlying {
-                self.underlying = value - 1
-            }
+            self.underlying = self.underlying - 1
         }
     }
 }
 
 extension ThreadSafeBox where Value == String {
+    /// Atomically appends a string to the stored string value.
+    ///
+    /// This method is only available when the wrapped value type is String.
+    ///
+    /// - Parameter value: The string to append to the current stored value.
     public func append(_ value: String) {
         self.mutate { existingValue in
-            if let existingValue {
-                return existingValue + value
-            } else {
-                return value
-            }
+            existingValue + value
         }
     }
 }

Sources/Basics/Observability.swift

@@ -163,7 +163,7 @@ public final class ObservabilityScope: DiagnosticsEmitterProtocol, Sendable, Cus
         }
 
         var errorsReported: Bool {
-            self._errorsReported.get() ?? false
+            self._errorsReported.get()
         }
     }
 }

Sources/Build/BuildOperation.swift

@@ -171,10 +171,10 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     }
 
     /// The build description resulting from planing.
-    private let buildDescription = ThreadSafeBox<BuildDescription>()
+    private let buildDescription = AsyncThrowingValueMemoizer<BuildDescription>()
 
     /// The loaded package graph.
-    private let packageGraph = ThreadSafeBox<ModulesGraph>()
+    private let packageGraph = AsyncThrowingValueMemoizer<ModulesGraph>()
 
     /// File system to operate on.
     private var fileSystem: Basics.FileSystem {

Sources/Build/ClangSupport.swift

@@ -24,7 +24,7 @@ public enum ClangSupport {
         let features: [Feature]
     }
 
-    private static var cachedFeatures = ThreadSafeBox<Features>()
+    private static var cachedFeatures = ThreadSafeBox<Features?>()
 
     public static func supportsFeature(name: String, toolchain: PackageModel.Toolchain) throws -> Bool {
         let features = try cachedFeatures.memoize {

Sources/Build/LLBuildProgressTracker.swift

@@ -96,7 +96,7 @@ public final class BuildExecutionContext {
 
     // MARK: - Private
 
-    private var indexStoreAPICache = ThreadSafeBox<Result<IndexStoreAPI, Error>>()
+    private var indexStoreAPICache = ThreadSafeBox<Result<IndexStoreAPI, Error>?>()
 
     /// Reference to the index store API.
     var indexStoreAPI: Result<IndexStoreAPI, Error> {

Sources/Commands/SwiftTestCommand.swift

@@ -1223,7 +1223,7 @@ final class ParallelTestRunner {
                     }
                     self.finishedTests.enqueue(TestResult(
                         unitTest: test,
-                        output: output.get() ?? "",
+                        output: output.get(),
                         success: result != .failure,
                         duration: duration
                     ))

Sources/DriverSupport/DriverSupportUtils.swift

@@ -19,7 +19,7 @@ import class TSCBasic.Process
 import struct TSCBasic.ProcessResult
 
 public enum DriverSupport {
-    private static var flagsMap = ThreadSafeBox<[String: Set<String>]>()
+    private static var flagsMap = ThreadSafeBox<[String: Set<String>]>([:])
 
     /// This checks _frontend_ supported flags, which are not necessarily supported in the driver.
     public static func checkSupportedFrontendFlags(
@@ -29,8 +29,9 @@ public enum DriverSupport {
     ) -> Bool {
         let trimmedFlagSet = Set(flags.map { $0.trimmingCharacters(in: ["-"]) })
         let swiftcPathString = toolchain.swiftCompilerPath.pathString
+        let entry = flagsMap.get()
 
-        if let entry = flagsMap.get(), let cachedSupportedFlagSet = entry[swiftcPathString + "-frontend"] {
+        if let cachedSupportedFlagSet = entry[swiftcPathString + "-frontend"] {
             return cachedSupportedFlagSet.intersection(trimmedFlagSet) == trimmedFlagSet
         }
         do {
@@ -63,8 +64,9 @@ public enum DriverSupport {
     ) -> Bool {
         let trimmedFlagSet = Set(flags.map { $0.trimmingCharacters(in: ["-"]) })
         let swiftcPathString = toolchain.swiftCompilerPath.pathString
+        let entry = flagsMap.get()
 
-        if let entry = flagsMap.get(), let cachedSupportedFlagSet = entry[swiftcPathString + "-driver"] {
+        if let cachedSupportedFlagSet = entry[swiftcPathString + "-driver"] {
             return cachedSupportedFlagSet.intersection(trimmedFlagSet) == trimmedFlagSet
         }
         do {