Skip to content

Commit 3a522a9

Browse files
committed
SE-0258: property wrappers, revise the Atomic example.
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)
1 parent 43401a7 commit 3a522a9

File tree

1 file changed

+33
-58
lines changed

1 file changed

+33
-58
lines changed

proposals/0258-property-wrappers.md

Lines changed: 33 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
+ [Examples](#examples)
1717
- [Delayed Initialization](#delayed-initialization)
1818
- [`NSCopying`](#nscopying)
19-
- [`Atomic`](#atomic)
2019
- [Thread-specific storage](#thread-specific-storage)
2120
- [User defaults](#user-defaults)
2221
- [Copy-on-write](#copy-on-write)
@@ -449,63 +448,6 @@ struct Copying<Value: NSCopying> {
449448
This implementation would address the problem detailed in
450449
[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.
451450

452-
### `Atomic`
453-
454-
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:
455-
456-
457-
```swift
458-
@propertyWrapper
459-
struct Atomic<Value> {
460-
private var _value: Value
461-
462-
init(wrappedValue: Value) {
463-
self._value = wrappedValue
464-
}
465-
466-
var wrappedValue: Value {
467-
get { return load() }
468-
set { store(newValue: newValue) }
469-
}
470-
471-
func load(order: MemoryOrder = .relaxed) { ... }
472-
mutating func store(newValue: Value, order: MemoryOrder = .relaxed) { ... }
473-
mutating func increment() { ... }
474-
mutating func decrement() { ... }
475-
}
476-
477-
extension Atomic where Value: Equatable {
478-
mutating func compareAndExchange(oldValue: Value, newValue: Value, order: MemoryOrder = .relaxed) -> Bool {
479-
...
480-
}
481-
}
482-
483-
enum MemoryOrder {
484-
case relaxed, consume, acquire, release, acquireRelease, sequentiallyConsistent
485-
};
486-
```
487-
488-
Here are some simple uses of `Atomic`. With atomic types, it's fairly common
489-
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:
490-
491-
```swift
492-
@Atomic var counter: Int
493-
494-
if thingHappened {
495-
_counter.increment()
496-
}
497-
print(counter)
498-
499-
@Atomic var initializedOnce: Int?
500-
if initializedOnce == nil {
501-
let newValue: Int = /*computeNewValue*/
502-
if !_initializedOnce.compareAndExchange(oldValue: nil, newValue: newValue) {
503-
// okay, someone else initialized it. clean up if needed
504-
}
505-
}
506-
print(initializedOnce)
507-
```
508-
509451
### Thread-specific storage
510452

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

15431485
## Revisions
15441486

1487+
### Changes from the accepted proposal
1488+
1489+
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.
1490+
1491+
Programmers looking for atomic operations can use the [Swift Atomics](https://github.com/apple/swift-atomics) package.
1492+
1493+
For those who have already attempted to implement something similar, here is the original example, and why it is incorrect:
1494+
1495+
```swift
1496+
@propertyWrapper
1497+
class Atomic<Value> {
1498+
private var _value: Value
1499+
1500+
init(wrappedValue: Value) {
1501+
self._value = wrappedValue
1502+
}
1503+
1504+
var wrappedValue: Value {
1505+
get { return load() }
1506+
set { store(newValue: newValue) }
1507+
}
1508+
1509+
func load(order: MemoryOrder = .relaxed) { ... }
1510+
func store(newValue: Value, order: MemoryOrder = .relaxed) { ... }
1511+
func increment() { ... }
1512+
func decrement() { ... }
1513+
}
1514+
```
1515+
1516+
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`.
1517+
1518+
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.
1519+
15451520
### Changes from the third reviewed version
15461521

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

0 commit comments

Comments
 (0)