From 027178acc3693d74d86711b24b63095d4df9373c Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 10 Nov 2025 16:56:13 -0500 Subject: [PATCH 1/2] Make lazy properties in UserToolchain thread safe `runtimeLibraryPaths`, `swiftCompilerVersion` and `targetInfo` were all marked as lazy on the UserToolchain class which could lead to race conditions if callers on multiple threads attempted to initialize them simultaneously. Use an `NSRecursiveLock` to guard access while initialization occurs, preventing crashes. --- Sources/PackageModel/UserToolchain.swift | 72 ++++++++++++++++++++---- 1 file changed, 61 insertions(+), 11 deletions(-) diff --git a/Sources/PackageModel/UserToolchain.swift b/Sources/PackageModel/UserToolchain.swift index 2520787428b..a85a80d57b0 100644 --- a/Sources/PackageModel/UserToolchain.swift +++ b/Sources/PackageModel/UserToolchain.swift @@ -42,11 +42,30 @@ public final class UserToolchain: Toolchain { /// An array of paths to search for libraries at link time. public let librarySearchPaths: [AbsolutePath] + /// Lock for thread-safe access to lazy properties (recursive to handle property dependencies) + private let lazyPropertiesLock = NSRecursiveLock() + + /// Cached runtime library paths + private var _runtimeLibraryPaths: [AbsolutePath]? = nil + /// An array of paths to use with binaries produced by this toolchain at run time. - public lazy var runtimeLibraryPaths: [AbsolutePath] = { - guard let targetInfo else { return [] } - return (try? Self.computeRuntimeLibraryPaths(targetInfo: targetInfo)) ?? [] - }() + public var runtimeLibraryPaths: [AbsolutePath] { + lazyPropertiesLock.lock() + defer { lazyPropertiesLock.unlock() } + + if let cached = _runtimeLibraryPaths { + return cached + } + + guard let targetInfo = self.targetInfo else { + _runtimeLibraryPaths = [] + return [] + } + + let paths = (try? Self.computeRuntimeLibraryPaths(targetInfo: targetInfo)) ?? [] + _runtimeLibraryPaths = paths + return paths + } /// Path containing Swift resources for dynamic linking. public var swiftResourcesPath: AbsolutePath? { @@ -82,16 +101,47 @@ public final class UserToolchain: Toolchain { public let targetTriple: Basics.Triple private let _targetInfo: JSON? - private lazy var targetInfo: JSON? = { + + /// Cached target info, deliberately doublely optional, one + /// for "not yet cached", one for "failed to get target info". + private var _cachedTargetInfo: Optional = nil + + private var targetInfo: JSON? { + lazyPropertiesLock.lock() + defer { lazyPropertiesLock.unlock() } + + if let cached = _cachedTargetInfo { + return cached + } + // Only call out to the swift compiler to fetch the target info when necessary - try? _targetInfo ?? Self.getTargetInfo(swiftCompiler: swiftCompilerPath) - }() + let info = try? _targetInfo ?? Self.getTargetInfo(swiftCompiler: swiftCompilerPath) + _cachedTargetInfo = info + return info + } + + /// Cached swift compiler version, deliberately doublely optional, one + /// for "not yet cached", one for "failed to get target info". + private var _swiftCompilerVersion: Optional = nil // A version string that can be used to identify the swift compiler version - public lazy var swiftCompilerVersion: String? = { - guard let targetInfo else { return nil } - return Self.computeSwiftCompilerVersion(targetInfo: targetInfo) - }() + public var swiftCompilerVersion: String? { + lazyPropertiesLock.lock() + defer { lazyPropertiesLock.unlock() } + + if let cached = _swiftCompilerVersion { + return cached + } + + guard let targetInfo = self.targetInfo else { + _swiftCompilerVersion = nil + return nil + } + + let version = Self.computeSwiftCompilerVersion(targetInfo: targetInfo) + _swiftCompilerVersion = version + return version + } /// The list of CPU architectures to build for. public let architectures: [String]? From b3f0ac660abd5743b8d3d236d85d6a7f9ae4a4ce Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 10 Nov 2025 20:10:36 -0500 Subject: [PATCH 2/2] Use ThreadSafeBox over NSRecursiveLock --- Sources/PackageModel/UserToolchain.swift | 68 +++++++----------------- 1 file changed, 19 insertions(+), 49 deletions(-) diff --git a/Sources/PackageModel/UserToolchain.swift b/Sources/PackageModel/UserToolchain.swift index a85a80d57b0..69edefd7132 100644 --- a/Sources/PackageModel/UserToolchain.swift +++ b/Sources/PackageModel/UserToolchain.swift @@ -42,29 +42,17 @@ public final class UserToolchain: Toolchain { /// An array of paths to search for libraries at link time. public let librarySearchPaths: [AbsolutePath] - /// Lock for thread-safe access to lazy properties (recursive to handle property dependencies) - private let lazyPropertiesLock = NSRecursiveLock() - - /// Cached runtime library paths - private var _runtimeLibraryPaths: [AbsolutePath]? = nil + /// Thread-safe cached runtime library paths + private let _runtimeLibraryPaths = ThreadSafeBox<[AbsolutePath]>() /// An array of paths to use with binaries produced by this toolchain at run time. public var runtimeLibraryPaths: [AbsolutePath] { - lazyPropertiesLock.lock() - defer { lazyPropertiesLock.unlock() } - - if let cached = _runtimeLibraryPaths { - return cached - } - - guard let targetInfo = self.targetInfo else { - _runtimeLibraryPaths = [] - return [] + return _runtimeLibraryPaths.memoize { + guard let targetInfo = self.targetInfo else { + return [] + } + return (try? Self.computeRuntimeLibraryPaths(targetInfo: targetInfo)) ?? [] } - - let paths = (try? Self.computeRuntimeLibraryPaths(targetInfo: targetInfo)) ?? [] - _runtimeLibraryPaths = paths - return paths } /// Path containing Swift resources for dynamic linking. @@ -102,45 +90,27 @@ public final class UserToolchain: Toolchain { private let _targetInfo: JSON? - /// Cached target info, deliberately doublely optional, one - /// for "not yet cached", one for "failed to get target info". - private var _cachedTargetInfo: Optional = nil + /// Thread-safe cached target info that allows for lazy instantiation + private let _cachedTargetInfo = ThreadSafeBox() private var targetInfo: JSON? { - lazyPropertiesLock.lock() - defer { lazyPropertiesLock.unlock() } - - if let cached = _cachedTargetInfo { - return cached + return _cachedTargetInfo.memoize { + // Only call out to the swift compiler to fetch the target info when necessary + return try? _targetInfo ?? Self.getTargetInfo(swiftCompiler: swiftCompilerPath) } - - // Only call out to the swift compiler to fetch the target info when necessary - let info = try? _targetInfo ?? Self.getTargetInfo(swiftCompiler: swiftCompilerPath) - _cachedTargetInfo = info - return info } - /// Cached swift compiler version, deliberately doublely optional, one - /// for "not yet cached", one for "failed to get target info". - private var _swiftCompilerVersion: Optional = nil + /// Thread-safe cached swift compiler version that allows for lazy instantiation + private let _swiftCompilerVersion = ThreadSafeBox() // A version string that can be used to identify the swift compiler version public var swiftCompilerVersion: String? { - lazyPropertiesLock.lock() - defer { lazyPropertiesLock.unlock() } - - if let cached = _swiftCompilerVersion { - return cached - } - - guard let targetInfo = self.targetInfo else { - _swiftCompilerVersion = nil - return nil + return _swiftCompilerVersion.memoize { + guard let targetInfo = self.targetInfo else { + return nil + } + return Self.computeSwiftCompilerVersion(targetInfo: targetInfo) } - - let version = Self.computeSwiftCompilerVersion(targetInfo: targetInfo) - _swiftCompilerVersion = version - return version } /// The list of CPU architectures to build for.