Make lazy properties in UserToolchain thread safe #9356
Conversation
plemarquand
requested review from
bkhouri,
bripeticca,
cmcgee1024,
daveyc123,
dschaefer2,
jakepetroules,
owenv and
rconnell9
as code owners
plemarquand
force-pushed
the
thread-safe-lazy-usertoolchain
branch
from
b9374ff to
753bcfc
Compare
`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.
plemarquand
force-pushed
the
thread-safe-lazy-usertoolchain
branch
from
753bcfc to
274f115
Compare
|
@swift-ci please test |
| public let librarySearchPaths: [AbsolutePath] | ||
|
|
||
| /// Lock for thread-safe access to lazy properties (recursive to handle property dependencies) | ||
| private let lazyPropertiesLock = NSRecursiveLock() |
There was a problem hiding this comment.
Directly using NSLock/NSRecursiveLock is not the recommended pattern in modern Swift, instead you'll want to use an abstraction like Swift Build's LockedValue. In SwiftPM you can use ThreadSafeBox from TSCBasic.
If you want to use a single lock for multiple values to avoid a proliferation of fine-grained locks, you can define a struct to hold the values, and then guard access to the overall struct through the wrapper, rather than using a separate lock for each value.
Later, once we can increase our deployment target to macOS 15, we can replace LockedValue/ThreadSafeBox with Mutex from the Swift Standard Library.
There was a problem hiding this comment.
Thanks, I figured these could be replaced with Mutex once we're ready to adopt but didn't know about ThreadSafeBox's used in SwiftPM. I've updated the implementation accordingly.
|
@swift-ci please test |
2 participants
runtimeLibraryPaths,swiftCompilerVersionandtargetInfowere 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
NSLockto guard access while initialization occurs, preventing crashes.