Usage of NSLock in property setter

Question

Let's say, there is a variable that I want to make thread safe. One of the most common ways to do this:

var value: A {
    get { return queue.sync { self._value } }
    set { queue.sync { self._value = newValue } }
}

However, this property is not completely thread safe if we change the value as in the example below:

Class.value += 1

So my question is: Using NSLock on the same principle is also not completely thread safe?

var value: A {
    get { 
       lock.lock()
       defer { lock.unlock() }
       return self._value
    }
    set { 
       lock.lock()
       defer { lock.unlock() }
       self._value = newValue
    }
}

Answer 1

That's interesting, I'm learning about this for the first time.

The issue in the first bit of code, is that:

object.value += 1

has the same semantics as

object.value = object.value + 1

which we can further expand to:

let originalValue = queue.sync { object._value }
let newValue = origiinalValue + 1
queue.sync { self._value = newValue }

Expanding it so makes it clear that the synchronization of the getter and setter work fine, but they're not synchronized as a whole. A context switch in the middle of the code above could cause _value to be mutated by another thread, without newValue reflecting the change.

Using a lock would have the exact same problem. It would expand to:

lock.lock()
let originalValue = object._value
lock.unlock()

let newValue = originalValue + 1

lock.lock()
object._value = newValue
lock.unlock()

You can see this for yourself by instrumenting your code with some logging statements, which show that the mutation isn't fully covered by the lock:

class C {
    var lock = NSLock()

    var _value: Int
    var value: Int {
        get {
            print("value.get start")
            print("lock.lock()")
            lock.lock()
            defer {
                print("lock.unlock()")
                lock.unlock()
                print("value.get end")
            }
            print("getting self._value")
            return self._value
        }
        set { 
            print("\n\n\nvalue.set start")
            lock.lock()
            print("lock.lock()")
            defer {
                print("lock.unlock()")
                lock.unlock()
                print("value.set end")
            }
            print("setting self._value")
            self._value = newValue
        }
    }

    init(_ value: Int) { self._value = value }
}

let object = C(0)
object.value += 1

Answer 2

In answer to your question, the lock approach suffers the exact same problems that the GCD approach does. Atomic accessor methods simply are insufficient to ensure broader thread-safety.

The issue is, as discussed elsewhere, that the innocuous += operator is retrieving the value via the getter, incrementing that value, and storing that new value via the setter. To achieve thread-safety, the whole process needs to be wrapped in a single synchronization mechanism. You want an atomic increment operation, you would write a method to do that.

First, taking your NSLock example, I might use withLock:

class Foo<T> {
    private let lock = NSLock()
    private var _value: T

    init(value: T) {
        _value = value
    }

    var value: T {
        get { lock.withLock { _value } }
        set { lock.withLock { _value = newValue } }
    }
}

But if you wanted to have an operation to increment the value in a thread-safe manner, you would write a method to do that, e.g.:

class Foo<T> {
    private let lock = NSLock()
    private var _value: T

    init(value: T) {
        _value = value
    }

    var value: T {
        get { lock.withLock { _value } }
        // set { lock.withLock { _value = newValue } }
    }
}

extension Foo where T: Numeric {
    func increment(by increment: T) {
        lock.withLock {
            _value += increment
        }
    }
}

Then, rather than this non-thread-safe attempt:

foo.value += 1

You would instead employ the following thread-safe rendition:

foo.increment(by: 1)

This pattern, of wrapping the increment process in its own method that synchronizes the whole operation, would be applicable regardless of what synchronization mechanism you use (e.g., locks, GCD serial queue, reader-writer pattern, os_unfair_lock, etc.).

Note, I have removed the setter entirely in this example, too. That is a matter of personal preference, a practice that entirely prohibits some non-thread-safe external += or update operation interfering with this counter.

---

For what it is worth, the Swift 5.5 actor pattern (outlined in SE-0306) formalizes this pattern. Consider:

actor Bar<T> {
    var value: T

    init(value: T) {
        self.value = value
    }
}

extension Bar where T: Numeric {
    func increment(by increment: T) {
        value += increment
    }
}

Here, the increment method is automatically an “actor-isolated” method (i.e., it will be synchronized) but the actor will control interaction with the setter for its property, namely if you try to set value from outside this class, you will receive an error:

Actor-isolated property 'value' can only be mutated from inside the actor