Skip to content

Commit

Permalink
SE-0258: property wrappers, revise the Atomic example.
Browse files Browse the repository at this point in the history
TSAN will correctly report an error if you use a struct Atomic
wrapper. Some developers have been baffled by the TSAN failure, which
actually correctly identifies the bug.

Fixes rdar://79173755 (TSAN violation on standard Atomic property wrappers)
  • Loading branch information
atrick committed Apr 7, 2022
1 parent 43401a7 commit a3c902c
Showing 1 changed file with 34 additions and 58 deletions.
92 changes: 34 additions & 58 deletions proposals/0258-property-wrappers.md
Expand Up @@ -16,7 +16,6 @@
+ [Examples](#examples)
- [Delayed Initialization](#delayed-initialization)
- [`NSCopying`](#nscopying)
- [`Atomic`](#atomic)
- [Thread-specific storage](#thread-specific-storage)
- [User defaults](#user-defaults)
- [Copy-on-write](#copy-on-write)
Expand Down Expand Up @@ -51,6 +50,7 @@
- [Referencing the enclosing 'self' in a wrapper type](#referencing-the-enclosing-self-in-a-wrapper-type)
- [Delegating to an existing property](#delegating-to-an-existing-property)
+ [Revisions](#revisions)
- [Changes from the accepted proposal](#changes-from-the-accepted-proposal)
- [Changes from the third reviewed version](#changes-from-the-third-reviewed-version)
- [Changes from the second reviewed version](#changes-from-the-second-reviewed-version)
- [Changes from the first reviewed version](#changes-from-the-first-reviewed-version)
Expand Down Expand Up @@ -449,63 +449,6 @@ struct Copying<Value: NSCopying> {
This implementation would address the problem detailed in
[SE-0153](https://github.com/apple/swift-evolution/blob/master/proposals/0153-compensate-for-the-inconsistency-of-nscopyings-behaviour.md). Leaving the `copy()` out of `init(wrappedValue:)` implements the pre-SE-0153 semantics.

### `Atomic`

Support for atomic operations (load, store, increment/decrement, compare-and-exchange) is a commonly-requested Swift feature. While the implementation details for such a feature would involve compiler and standard library magic, the interface itself can be nicely expressed as a property wrapper type:


```swift
@propertyWrapper
struct Atomic<Value> {
private var _value: Value

init(wrappedValue: Value) {
self._value = wrappedValue
}

var wrappedValue: Value {
get { return load() }
set { store(newValue: newValue) }
}

func load(order: MemoryOrder = .relaxed) { ... }
mutating func store(newValue: Value, order: MemoryOrder = .relaxed) { ... }
mutating func increment() { ... }
mutating func decrement() { ... }
}

extension Atomic where Value: Equatable {
mutating func compareAndExchange(oldValue: Value, newValue: Value, order: MemoryOrder = .relaxed) -> Bool {
...
}
}

enum MemoryOrder {
case relaxed, consume, acquire, release, acquireRelease, sequentiallyConsistent
};
```

Here are some simple uses of `Atomic`. With atomic types, it's fairly common
to weave lower-level atomic operations (`increment`, `load`, `compareAndExchange`) where we need specific semantics (such as memory ordering) with simple queries, so both the property and the synthesized storage property are used often:

```swift
@Atomic var counter: Int

if thingHappened {
_counter.increment()
}
print(counter)

@Atomic var initializedOnce: Int?
if initializedOnce == nil {
let newValue: Int = /*computeNewValue*/
if !_initializedOnce.compareAndExchange(oldValue: nil, newValue: newValue) {
// okay, someone else initialized it. clean up if needed
}
}
print(initializedOnce)
```

### Thread-specific storage

Thread-specific storage (based on pthreads) can be implemented as a property wrapper, too (example courtesy of Daniel Delwood):
Expand Down Expand Up @@ -1542,6 +1485,39 @@ One could express this either by naming the property directly (as above) or, for

## Revisions

### Changes from the accepted proposal

This proposal originally presented an example of implementing atomic operations using a property wrapper interface. This example was misleading because it would require additional compiler and library features to work correctly.

Programmers looking for atomic operations can use the [Swift Atomics](https://github.com/apple/swift-atomics) package.

For those who have already attempted to implement something similar, here is the original example, and why it is incorrect:

```swift
@propertyWrapper
class Atomic<Value> {
private var _value: Value

init(wrappedValue: Value) {
self._value = wrappedValue
}

var wrappedValue: Value {
get { return load() }
set { store(newValue: newValue) }
}

func load(order: MemoryOrder = .relaxed) { ... }
func store(newValue: Value, order: MemoryOrder = .relaxed) { ... }
func increment() { ... }
func decrement() { ... }
}
```

As written, this property wrapper does not access its wrapped value atomically. `wrappedValue.getter` reads the entire `_value` property nonatomically, *before* calling the atomic `load` operation on the copied value. Similarly, `wrappedValue.setter` writes the entire `_value` property nonatomically *after* calling the atomic `store` operation. So, in fact, there is no atomic access to the shared class property, `_value`.

Even if the getter and setter could be made atomic, useful atomic operations, like increment, cannot be built from atomic load and store primitives. The property wrapper in fact encourages race conditions by allowing mutating methods, such as `atomicInt += 1`, to be directly invoked on the nonatomic copy of the wrapped value.

### Changes from the third reviewed version

* `init(initialValue:)` has been renamed to `init(wrappedValue:)` to match the name of the property.
Expand Down

0 comments on commit a3c902c

Please sign in to comment.