Hello, I am wondering how to properly handle some of the new strict concurrency warnings introduced in Swift 5.10 around global state.
I have the following property wrapper:
import os
@propertyWrapper
struct Foo<Value: Sendable>: Sendable {
let value: OSAllocatedUnfairLock<Value>
init(wrappedValue: Value) {
self.value = .init(initialState: wrappedValue)
}
@available(*, unavailable, message: "property wrappers cannot be instance members")
public static subscript(
_enclosingInstance object: Never,
wrapped wrappedKeyPath: ReferenceWritableKeyPath<Never, Value>,
storage storageKeyPath: ReferenceWritableKeyPath<Never, Foo<Value>>
) -> Value {
get {
}
}
var wrappedValue: Value {
value.withLock {
$0
}
}
var projectedValue: Self {
self
}
func set(_ newValue: Value) {
value.withLock {
$0 = newValue
}
}
}
And I get this warning when creating one:
enum Bar {
// Static property 'x' is not concurrency-safe because it is
//non-isolated global shared mutable state; this is an error in Swift 6
@Foo static var x = 0
@TaskLocal static var y = 0
}
I believe this is safe but I'm not sure how to convince the compiler of that. I can not apply nonisolated(unsafe) here because that is not supported on property wrappers but I'm also hoping it wouldn't be necessary even if it were allowed. What can I do to squash this warning?
I've also shown a TaskLocal var to demonstrate that it does not produce the same warning. I took a peek at the swift implementation of TaskLocal and I did not see any interesting attributes here so I'm guessing maybe the compiler has some special knowledge around TaskLocal?
I have also tried to make my mutex a property wrapper, hoping that the language would show some awareness for well-behaved programs. But it does not work very well:
@Mutex var value = 1
run { // an @escaping @Sendable closure
// Error: Mutation of captured var 'value'
// in concurrently-executing code
value = 2
// OK
$value.withLock { $0 = 2 }
}
run {
print(
}
I use this a lot in tests of asynchronous-but-not-async methods (similar to DispatchQueue.async):
@MainActor // for waitForExpectations
func test_that_value_is_2() {
let expectation = self.expectation(description: "")
@Mutex var value: Int? = nil
giveMeTwoEventually { n in // an @escaping @Sendable closure
$value.withLock { $0 = n }
expectation.fulfill()
}
waitForExpectations(timeout: 1)
XCTAssertEqual(value, 2)
}
Without strict concurrency checks, the tests is easier to write, obviously:
// Without strict checks
func test_that_value_is_2() {
let expectation = self.expectation(description: "")
var value: Int?
giveMeTwoEventually { n in
value = n
expectation.fulfill()
}
waitForExpectations(timeout: 1)
XCTAssertEqual(value, 2)
}
That's a good point, thanks for reminding me! It's not just an issue in for static property wrappers and I've run in to the same problem that you've outlined as well with exactly the same sort of wrapper for doing the same kind of thing.
Yeah, unfortunately, despite being a key example in the property wrapper proposal, I've found that lock wrappers aren't safe, regardless of language awareness. This is due to the fact that wrappedValue, by definition, only wraps the base value in the wrapper. This makes any chained property or other access unsafe. I've tried enforcing usage of $<property> to ensure the access always goes through the wrapper itself, but that's easy to miss. In the end I stopped using them as @propertyWrappers and just have a Protected(<value>) directly which is then used to access the underlying value. More verbose but I don't have to worry about safety.
@hborla are there any anticipated language feature (or even just theoretical features) that could enable this pattern safely? I think people are going to run into this more and more as locks become more important in a Sendable world. Perhaps something to extend the scope of wrappedValue or a way to indicate a getter should have expanded scope?
And from the PR the implication seems to be that static property wrappers like this will never jive with strict concurrency checking. Is that really true?
Unless you're within a global-actor-isolated context, the property wrapper transform for static variables is fundamentally prone to data races because the backing storage variable is always mutable.
I agree with you that that property wrappers are not suitable for mutex and atomic APIs; SE-0258 has since been amended to remove the Atomic example and explain why it is both incorrect and prone to race conditions. I think that building out the standard APIs in the Synchronization library is the right approach for encouraging these patterns safely, and I think that ~Escapable types along with lifetime dependence annotations may give us ways to make those APIs more ergonomic. I'm not sure a safe mutex API will ever be as ergonomic as directly operating on the protected value, e.g. value = 2, but I'm also not sure that's even desirable.
Just double checking: by backing storage variable do you mean this?
enum Example {
@TaskLocal static var y = 0
static func bar() {
// Reference to static property '_y' is not concurrency-safe because it
// involves shared mutable state; this is an error in Swift 6
_y = .init(wrappedValue: 1)
}
}
If so that makes complete sense.
I still really think there ought to be a way to define a Sendable property wrapper that Swift 6 (and 5.10 with strict concurrency checking) is content with and that can be used safely from a static context for all normal usages going through its wrappedValue or projectedValue.
My Mutex is a class, so its wrappedValue setter is already nonmutating.
I could also make a Mutex2 struct with a nonmutating setter (by wrapping the above class).
Both won't mutate the variable in async contexts:
func f() async {
@Mutex var value = 1 // the class
@Mutex2 var value2 = 1 // the struct
await MainActor.run {
value = 2 // Mutation of captured var 'value' in concurrently-executing code
value2 = 2 // Mutation of captured var 'value2' in concurrently-executing code
}
}
EDIT: I replaced value += 1 with value = 2 so that Holly does not spot the data race
My implementation of both types
@propertyWrapper
final class Mutex<T> {
private var _value: T
private var lock = NSLock()
var wrappedValue: T {
get { withLock { $0 } }
set { withLock { $0 = newValue } }
}
var projectedValue: Mutex<T> { self }
init(wrappedValue: T) {
_value = wrappedValue
}
init(_ value: T) {
_value = value
}
func withLock<U>(_ body: (inout T) throws -> U) rethrows -> U {
lock.lock()
defer { lock.unlock() }
return try body(&_value)
}
}
@propertyWrapper
struct Mutex2<T> {
private let mutex: Mutex<T>
var wrappedValue: T {
get {
mutex.wrappedValue
}
nonmutating set {
mutex.wrappedValue = newValue
}
}
var projectedValue: Mutex2<T> { self }
init(wrappedValue: T) {
mutex = Mutex(wrappedValue: wrappedValue)
}
init(_ value: T) {
mutex = Mutex(wrappedValue: value)
}
func withLock<U>(_ body: (inout T) throws -> U) rethrows -> U {
try mutex.withLock {
try body(&$0)
}
}
}
Could the recent discussions about Atomic variables being declared as let without being constant help revisiting the mandatory var declaration of wrapped properties? Maybe a let could help in our use case?
func f() async {
@Mutex let value = 1
await MainActor.run {
value = 2 // Sure!
}
}
It seems to that correct solution for this would be to declare static property as let, and make all API of the property wrapper nonmutating. This should convince compiler that there no issues with accessing global mutable memory. But, IIRC, property wrappers cannot be used with let declarations.
Additionally we need to show that it is ok to use the methods and properties of the wrapper concurrently. For that it needs to conform to Sendable.
“Why not let”: you also can’t use let for computed properties, and that’s because it’s “supposed” to be a stronger condition: not only is there no setter, but the value you get will be the same each time (i.e. it’s guaranteed to be locally cacheable, and eliding gets is a valid optimization). Of course, this applies to some computed properties too, and we don’t have a way to expose that. But property wrappers force computed-ness upon a variable by virtue of being defined in terms of _storage.wrappedValue, so
Unfortunately, all of that theory goes out the window with property wrappers, where you can assign over _storage in the same file and it doesn’t even matter what type you used. So I guess something needs tweaking.
In at least some cases, it seems like it would be useful for a property wrapper to synthesize its storage as a let instead of a var, and allowing that wouldn't stretch the meaning of let anymore than it already has. (Although whether we want to invite people to write @Atomic and @Mutex property wrappers specifically is questionable.)
enum Example {
static let _x = Foo(wrappedValue: 0)
static var x: Int {
_x.wrappedValue
}
}
This compiles cleanly in Swift 5.10 with strict concurrency checking enabled but it has big "we have property wrappers at home" energy. I think this is what anyone who wants to use a static property wrapper in Swift 6 would have to do unless there's a change. This is my current plan unless there's a different way to go about it.
I am hoping that the following normal usage can be made legal again in Swift 6, and that assigning directly to the _x backing storage would still remain an error.
enum Example {
@Foo static var x = 0
}
And not to get too wrapped up in atomics, locking, etc: this property wrapper, useless as it is, of course produces the same warning when used statically.
@propertyWrapper
public struct Simple: Sendable {
public init(wrappedValue: Bool) {
self.wrappedValue = wrappedValue
}
public let wrappedValue: Bool
}
Yeah, Atomic behaves like any other type with reference semantics combined with let, like UnsafePointer or Box<T>. As noted manually “expanding” the property wrapper into a let stored property and a read-only or nonmutating-ly settable var computed property would indeed sidestep the diagnostic, so having a way to write that somehow while still using wrapper syntax isn’t unreasonable.
We can already express this with macros. We could provide a standard library macro that does this transformation so that all you need to do is declare a new macro in terms of the standard library one, giving it your wrapper type.
I haven't yet used macros much but that sounds great!
And on the topic of potentially defining the backing storage with let I wonder if something like this would make sense:
Swift 5.10
Suppress the current diagnostic for get only or nonmutating set property wrappers defined statically.
get only or nonmutating set property wrappers would have their backing storage continue to be defined as a var as they are today.
Continue to warn about access to the backing storage.
Continue to warn when it has a mutating wrapped value.
Swift 6
Defining a static property wrapper with a mutating set would now be an error (maybe with a way to mark it nonisolated(unsafe) if you really truly want it).
get only or nonmutating set property wrappers would have their backing storage defined with a let.
And property wrappers declared at the instance/function level would continue to be stored as var no matter what as I assume doing otherwise would be a breaking change.
Would this help here? If so, would it be possible to (eventually) update strict concurrency diagnostics of Swift 5.10 to match whatever this ends up allowing in the static use case?
