8

Do I need some kind of explicit synchronization in this case?

class A { 
  let val: Int; 
  init(_ newVal: Int) { 
    val = newVal
   }
}

public class B {
  var a: A? = nil
  public func setA() { a = A(0) }
  public func hasA() -> Bool { return a != nil }
}

There is also another method in class B:

public func resetA() {
  guard hasA() else { return }
  a = A(1)
}

setA() and resetA() may be called from any thread, in any order.

I understand that there may be a race condition, that if concurrently one thread calls setA() and another thread calls resetA(), the result is not determined: val will either be 0, or 1, but I don't care: at any rate, hasA() will return true, won't it?

Does the answer change if A is a struct instead of class?

Alex Cohn
  • 56,089
  • 9
  • 113
  • 307
  • Actually with the given code – by the way `a` must be optional – you can replace the entire code in `B` with `lazy var a = A()`, this creates the instance (once) lazily, – vadian Jan 21 '21 at 15:39
  • @vadian I believe I cannot use `lazy` here: I want to know if `setA()` was explicitly called. – Alex Cohn Jan 21 '21 at 15:52
  • 2
    @vadian `lazy` is not thread-safe / not atomic either. – timbre timbre Jan 21 '21 at 15:59
  • 1
    `var a: A = nil` this will give you compilation error, what u meant probably is `var a: A? = nil` also I agree with Vadian that using lazy will ensure that there exists only one instance of A there by atomicity of it – Sandeep Bhandari Jan 21 '21 at 15:59
  • If you want `a` to be thread-safe / atomic, yes, you need to do some additional work. Simplest one is have a `DispatchSemaphore` control access to that var (I saw in public apple's own code they use it a lot for "simple" cases), but if you want something more sophisticated, best to do reader/writer lock, e.g. https://dmytro-anokhin.medium.com/concurrency-in-swift-reader-writer-lock-4f255ae73422 – timbre timbre Jan 21 '21 at 16:04
  • @Rob, the whole point is that may object is not mutable. Well, see the updated question, which makes it a bit more juicy. Do I need locks, dispatch queues, semaphores, or some other concurrency helpers? I understand that there may be a race condition, that if concurrently one thread calls `setA()` and another thread calls `resetA(1)`, the result is not determined: `val` will either be **0**, or **1**, but I don't care: at any rate, `hasA()` will return `true`, won't it? – Alex Cohn Jan 22 '21 at 12:46
  • First, you've only just changed your question to make `A` immutable, so it's not sporting to retroactively declare that that was “the whole point”. But let's set that aside. Second, in `B`, you're still exposing property `a`, so that's definitely not thread safe. Third, assuming you fix that, too, the bottom line is that **no**, properties are not atomic. You must synchronize it to make it thread safe. This has been asked and answered many times. See https://stackoverflow.com/search?q=%5Bswift%5D+atomic+properties – Rob Jan 22 '21 at 13:57
  • 1
    “Does the answer change if A is a struct instead of class?” ... Nope. Still not atomic. Now, if (1) both `A` and `B` were structs and (2) you ensured that each thread got its own copy, employing value semantics everywhere, then, yes, that does solve the problem. – Rob Jan 22 '21 at 14:03
  • @Rob, I have posted an *answer* that uses a lock for all operations. Is it correct in your eyes? Or it is not enough, or too much, or the choice of synchronization mechanism (`os_unfair_lock_lock`) is non-optimal? – Alex Cohn Jan 22 '21 at 14:16

1 Answers1

2

In short, no, property accessors are not atomic. See WWDC 2016 video Concurrent Programming With GCD in Swift 3, which talks about the absence of atomic/synchronization native in the language. (This is a GCD talk, so when they subsequently dive into synchronization methods, they focus on GCD methods, but any synchronization method is fine.) Apple uses a variety of different synchronization methods in their own code. E.g. in ThreadSafeArrayStore they use they use NSLock).


If synchronizing with locks, I might suggest an extension like the following:

extension NSLocking {
    func synchronized<T>(block: () throws -> T) rethrows -> T {
        lock()
        defer { unlock() }
        return try block()
    }
}

Apple uses this pattern in their own code, though they happen to call it withLock rather than synchronized. But the pattern is the same.

Then you can do:

public class B {
    private var lock = NSLock()
    private var a: A?             // make this private to prevent unsynchronized direct access to this property

    public func setA() {
        lock.synchronized {
            a = A(0)
        }
    }

    public func hasA() -> Bool {
        lock.synchronized {
            a != nil
        }
    }

    public func resetA() {
        lock.synchronized {
            guard a != nil else { return }
            a = A(1)
        }
    }
}

Or perhaps

public class B {
    private var lock = NSLock()
    private var _a: A?

    public var a: A? {
        get { lock.synchronized { _a } }
        set { lock.synchronized { _a = newValue } }
    }

    public var hasA: Bool {
        lock.synchronized { _a != nil }
    }

    public func resetA() {
        lock.synchronized {
            guard _a != nil else { return }
            _a = A(1)
        }
    }
}

I confess to some uneasiness in exposing hasA, because it practically invites the application developer to write like:

if !b.hasA {
    b.a = ...
}

That is fine in terms of preventing simultaneous access to memory, but it introduced a logical race if two threads are doing it at the same time, where both happen to pass the !hasA test, and they both replace the value, the last one wins.

Instead, I might write a method to do this for us:

public class B {
    private var lock = NSLock() // replacing os_unfair_lock_s()
    private var _a: A? = nil // fixed, thanks to Rob

    var a: A? {
        get { lock.synchronized { _a } }
        set { lock.synchronized { _a = newValue } }
    }

    public func withA(block: (inout A?) throws -> T) rethrows -> T {
        try lock.synchronized {
            try block(&_a)
        }
    }
}

That way you can do:

b.withA { a in
    if a == nil {
        a = ...
    }
}

That is thread safe, because we are letting the caller wrap all the logical tasks (checking to see if a is nil and, if so, the initialization of a) all in one single synchronized step. It is a nice generalized solution to the problem. And it prevents logic races.


Now the above example is so abstract that it is hard to follow. So let's consider a practical example, a variation on Apple’s ThreadSafeArrayStore:

public class ThreadSafeArrayStore<Value> {
    private var underlying: [Value]
    private let lock = NSLock()

    public init(_ seed: [Value] = []) {
        underlying = seed
    }

    public subscript(index: Int) -> Value {
        get { lock.synchronized { underlying[index] } }
        set { lock.synchronized { underlying[index] = newValue } }
    }

    public func get() -> [Value] {
        lock.synchronized {
            underlying
        }
    }

    public func clear() {
        lock.synchronized {
            underlying = []
        }
    }

    public func append(_ item: Value) {
        lock.synchronized {
            underlying.append(item)
        }
    }

    public var count: Int {
        lock.synchronized {
            underlying.count
        }
    }

    public var isEmpty: Bool {
        lock.synchronized {
            underlying.isEmpty
        }
    }

    public func map<NewValue>(_ transform: (Value) throws -> NewValue) rethrows -> [NewValue] {
        try lock.synchronized {
            try underlying.map(transform)
        }
    }

    public func compactMap<NewValue>(_ transform: (Value) throws -> NewValue?) rethrows -> [NewValue] {
        try lock.synchronized {
            try underlying.compactMap(transform)
        }
    }
}

Here we have a synchronized array, where we define an interface to interact with the underlying array in a thread-safe manner.


Or, if you want an even more trivial example, consider an thread-safe object to keep track of what the tallest item was. We would not have a hasValue boolean, but rather we would incorporate that right into our synchronized updateIfTaller method:

public class Tallest {
    private var _height: Float?
    private let lock = NSLock()

    var height: Float? {
        lock.synchronized { _height }
    }

    func updateIfTaller(_ candidate: Float) {
        lock.synchronized {
            guard let tallest = _height else {
                _height = candidate
                return
            }

            if candidate > tallest {
                _height = candidate
            }
        }
    }
}

Just a few examples. Hopefully it illustrates the idea.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • Thanks a lot! These examples will definitely help in broader context. My usecase is different. – Alex Cohn Jan 22 '21 at 18:40
  • OK, now I bang my head on another atomicity issue: https://stackoverflow.com/questions/66224497/how-do-i-capture-a-property-in-thread-safe-way – Alex Cohn Feb 16 '21 at 14:11