how do I capture a property in thread-safe way

Question

My class A has a property of class B which can be reset:

class A1 {
    private var b = B(0)

    func changeB(i : Int) {
        b = B(i)
    }

    func testB(k : Int) -> Bool {
        return b.test(k)
    }
}

class B {
    private let b : Int;

    init(_ i : Int) {
        b = i
    }

    func test(_ k : Int) -> Bool {
        return b == k
    }
}

So far so good. If I want to use class A in multithreading scenario, I must add some synchronization mechanism, because properties in Swift are not atomic by themselves:

class A2 {
    private var b = B(0)
    private let lock = NSLock()

    func changeB(i : Int) {
        lock.lock()
        defer { lock.unlock() }
        b = B(i)
    }
    
    func testB(k : Int) -> Bool {
        lock.lock()
        defer { lock.unlock() }
        return b.test(k)
    }

}

But now I want to introduce a closure:

class A3 {
    func listenToB() {
        NotificationCenter.default.addObserver(forName: Notification.Name("B"), object: nil, queue: nil) { 
        [b] (notification) in
            let k = notification.userInfo!["k"] as! Int
            print(b.test(k))
        }
    }
}

Do I understand correctly that this is not thread-safe? Will this get fixed if I capture lock as well, as below?

class A4 {
    func listenToB() {
        NotificationCenter.default.addObserver(forName: Notification.Name("B"), object: nil, queue: nil) { 
        [lock, b] (notification) in
            let k = notification.userInfo!["k"] as! Int
            lock.lock()
            defer { lock.unlock() }
            print(b.test(k))
        }
    }
}

Rob · Accepted Answer · 2021-02-16T16:25:23.507

1

Yes, using the captured lock ensures that the observer’s closure is synchronized with other tasks using the same lock. You can use this capturing pattern because lock happens to be a constant.

That raises the more fundamental problem, namely the capturing of the b reference, which is not constant. That means that if you call changeB at some intervening point in time, your notification block will still reference the original captured B, not the new one.

So, you really want to fall back to the weak self pattern if you want this to reference the current B:

class A {
    func listenToB() {
        NotificationCenter.default.addObserver(forName: Notification.Name("B"), object: nil, queue: nil) { [weak self] notification in
            guard let self = self else { return }

            let k = notification.userInfo!["k"] as! Int
            self.lock.lock()
            defer { self.lock.unlock() }
            print(self.b.test(k))
        }
    }
}

edited Feb 16 '21 at 16:25

answered Feb 16 '21 at 16:15

Rob

415,655
72
787
1,044

Do I understand you correctly: the captured `b` reference is resolved at the time the closure is called, not at the time the closure is executed. But then, this is not protected by the lock! For me, either reference – before change, or after change – is OK. I only introduced the lock here to protect from a disaster when the reference to `b` will be invalid (i.e. `b.test()` will crash). This would mean, I can safely use the first, non-locked version of `listenToB()`. – Alex Cohn Feb 16 '21 at 16:49
And yes, I fully understand the advantages of capturing `self` (or `weak self`), but that's exactly what I wanted to avoid here! I wanted to make this granular, because there actually are many properties in **A**, and all follow the same pattern, and I don't want them to compete for the same `lock`. – Alex Cohn Feb 16 '21 at 16:53
When you use the capture list `[b]`, the `B` reference is captured when you add the observer, not when the closure is called. (Maybe that's what you meant, but just to be clear. Lol.) If you're OK with that, then fine, you can use the capture list pattern, but it seems curious that that a “B” notification wants to test some fixed reference to the original `B`, not the current one. But if that's really what you intended, then that's fine (if unusual). If your intent was solely to eliminate syntactic noise coming from `[weak self]` pattern, there are probably better ways to do that. – Rob Feb 16 '21 at 17:14
I would definitely love to learn the better ways to keep the code clean. The 'old' value of `b` is fine for me, because actually the test is a bit less straightforward, and either value will be fine. Then, maybe, I don't need `lock` as I use in `A2` above, too? – Alex Cohn Feb 16 '21 at 17:22
To my eye, the syntactic noise that bothers me is the repeated locking/unlocking. I'd clean that up first: https://gist.github.com/robertmryan/4395613b13d96a2d811f5b002f0aad1b. I grok why you're attracted to capture list to break strong reference cycle, but because it entails “not the current value”, it’s rarely going to be a pattern that is useful. (Use it when you explicitly want a copy, not just to avoid `guard let self = self` pattern.) But a simple optional chain of `self?....` results in code clear of too much syntactic noise without the limitations that the capturing of `b` entails. – Rob Feb 16 '21 at 19:45
I am afraid that I did not express my use case clear enough. I have tried to be more accurate to the actual case, but now it's too long to be a question for SO, so it's on GitHub. – Alex Cohn Feb 16 '21 at 21:56

how do I capture a property in thread-safe way

1 Answers1

Linked