While using NSLock, do I need to call lock() when reading a value?

[carlhung]

as title.
Do I need to lock when reading a value?

for example:

@propertyWrapper
struct Protected<T> {
    
    private let locker: NSLock = NSLock()
    
    private var storedValue: T
    
   init(wrappedValue: T) {
        self.storedValue = wrappedValue
    }
    
    var wrappedValue: T {
        get {
            locker.lock()
            defer {
                locker.unlock()
            }
            return storedValue
        }
        set {
            locker.lock()
            defer {
                locker.unlock()
            }
            storedValue = newValue
        }
    }
}

do I need to use lock() in the getter?

[Dmitriy_Ignatyev]

Using of NSLock assumes that you have a mutable state protected by this lock. In general the answer is yes – lock() should be called before reading to prevent reading when a writing is done by another thread at the same time.

[Dmitriy_Ignatyev]

@carlhung

        get {
            locker.lock()
            defer {
                locker.unlock()
            }
            return storedValue
        }
        set {
            locker.lock()
            defer {
                locker.unlock()
            }
            storedValue = newValue
        }

This sample is technically correct, but be careful. get and set are protected separately and there is no guarantee they prevent data races even they protect from memory corruption.

@Protected var counter: Int = 0

for _ in 1...1000 {
  DispatchQueue.async { counter += 1 }
}

In the example above every time counter += 1 is executed, the following is done:

• a copy of counter is made
• this copy is mutated
• this copy is then set back to counter property

In practice, at some moment the following happens:

• 4 (or any other number) threads copy the same counter value. e.g. 0
• all of these threads increment 0 to 1
• all of these threads write 1 back to counter property
  Finally, counter property is equal to 1 while you expect 4

In some cases this solution is acceptable, in others it is not.

[carlhung]

what is the best way? I also recognise this. I think it is better use class instead of struct.

[vns]

I would also suggest to use withLock instead of lock/unlock pair in most of the cases. Much less options to make a mistake.

@carlhung

I think it is better use class instead of struct.

Yes, class is much more robust in that case.

[tera]

Great answers above, and if you are to do "counter += 1" you could perform the whole operation under lock, e.g.:

counter.locker.withLock {
    counter.storedValue += 1
}

which you could wrap a bit further in a nicer API.

As an alternative consider atomics (if T is a simple type like Int).

BTW, class instead of struct does feel safer and more correct in this case, otherwise you'd need to worry what would happen if you copy a variable and perform operations on both copies (the lock will be shared, the values will be different, the overall semantics will be non-obvious).

[Dmitriy_Ignatyev]

@tera

which you could wrap a bit further for a nicer API.

Like this e.g.:

public final class Protected<A> {  
  public var value: A {
    lock.withLock { _value }
  }
  
  public func mutate(_ transform: (inout A) -> Void) {
    lock.withLock { transform(&self._value) }
  }
}