Crashing at queue.sync { self._state }

181 views Asked by At

I'm trying to create an async subclass of Operation. The code crashes on the getter of the "state" Property. I have found an article which solves the async Subclass, but i'm curios as to why a crash happens at queue.sync { self._state } The willSet and DidSet of the _state property is called and the control moves to the getter state where it crashes image of the crash

class ConcurrentOperation: Operation {
        private enum State: String {
            case ready = "isReady"
            case executing = "isExecuting"
            case finished = "isFinished"
        }
    
        private var _state: State = .ready {
            willSet {
                willChangeValue(forKey: newValue.rawValue)
                willChangeValue(forKey: _state.rawValue)
            }
            didSet {
                didChangeValue(forKey: _state.rawValue)
                didChangeValue(forKey: oldValue.rawValue)
            }
        }
    
        private let queue = DispatchQueue(label: "com.concurrentOperation.chanakkya",  attributes: .concurrent)
    
        private var state: State {
            get {
                queue.sync { self._state }
            }
            set {
                queue.sync(flags: .barrier) {
                    self._state = newValue
                }
            }
        }
    
        override var isReady: Bool {
            super.isReady && state == .ready
        }
    
        override var isExecuting: Bool {
            state == .executing
        }
    
        override var isFinished: Bool {
            state == .finished
        }
    
        override func start() {
            guard !isCancelled else {
                finish()
                return
            }
    
            if !isExecuting {
                state = .executing
            }
    
            main()
        }
    
        override func main() {
            fatalError()
        }
    
        func finish() {
            if isExecuting {
                state = .finished
            }
        }
    
        override func cancel() {
            super.cancel()
            finish()
        }
    
    }
2

There are 2 answers

1
Andrew On

You have to use a temp variable

private var state: State {
            get {
                var temp: State!
                queue.sync { 
                   temp  = self._state 
                }
                return temp
            }
            set {
                queue.sync(flags: .barrier) {
                    self._state = newValue
                }
            }
        }
0
Rob On

If you look at your stack trace on the left, it will show you that you are in the middle of state.setter (#27 in the trace of thread 6’s stack) when it tries to call state.getter (#6 in the stack trace). In this case, this is caused by an observer that is checking isExecuting (#7 in the stack trace). More generally, this can be caused by the use of KVO observing options that may choose to fetch .new/.old values as part of the KVO notification.

enter image description here

Bottom line, you are synchronizing the setter which simultaneously invokes and prevents subsequent synchronized getter.


There are a variety of solutions:

  1. You can use a recursive lock. (Generally, recursive locks indicate code smell, but this is one scenario where they may be appropriate.)

    private let lock = NSRecursiveLock()
    
    private var _state: State = .ready {
        willSet {
            willChangeValue(forKey: newValue.rawValue)
            willChangeValue(forKey: _state.rawValue)
        }
        didSet {
            didChangeValue(forKey: _state.rawValue)
            didChangeValue(forKey: oldValue.rawValue)
        }
    }
    
    private var state: State {
        get { lock.withLock { _state } }
        set { lock.withLock { _state = newValue } }
    }
    

    Personally, I would move the manual KVO into the computed property:

    private let lock = NSRecursiveLock()
    
    private var _state: State = .ready
    
    private var state: State {
        get { 
            lock.withLock { _state }
        }
        set { 
            lock.withLock {
                let oldValue = _state
                willChangeValue(forKey: newValue.rawValue)
                willChangeValue(forKey: oldValue.rawValue)
                _state = newValue
                didChangeValue(forKey: oldValue.rawValue)
                didChangeValue(forKey: newValue.rawValue)
            }
        }
    }
    
  2. You can move the manual KVO notifications out of synchronized set, eliminating the need for recursive locking:

    private let lock = NSLock()
    
    private var _state: State = .ready
    
    private var state: State {
        get {
            lock.withLock { _state }
        }
        set {
            willChangeValue(forKey: newValue.rawValue)
            willChangeValue(forKey: state.rawValue)
    
            lock.withLock {
                let oldValue = _state
                _state = newValue
            }
    
            didChangeValue(forKey: oldValue.rawValue)
            didChangeValue(forKey: newValue.rawValue)
        }
    }
    

    At this point, you can use whatever synchronization mechanism you want.

  3. You can just use the dynamic qualifier and let the compiler generate all the KVO for you. And then, adopting a pattern outlined in the code samples that once accompanied Advanced NSOperations, you can use either keyPathsForValuesAffectingValue (or keyPathsForValuesAffectingIsReady, keyPathsForValuesAffectingIsExecuting, and keyPathsForValuesAffectingIsFinished) to make sure that the state KVO notifications propagate to isReady, isExecuting and isFinished).

    class ConcurrentOperation: Operation {
        @objc private enum State: Int {
            case ready, executing, finished
        }
    
        private let queue = DispatchQueue(label: "com.concurrentOperation.chanakkya")
    
        private var _state: State = .ready
    
        @objc dynamic private var state: State {
            get { queue.sync { _state } }
            set { queue.sync { _state = newValue } }
        }
    
        open override class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String> {
            if [#keyPath(isReady), #keyPath(isFinished), #keyPath(isExecuting)].contains(key) {
                return [#keyPath(state)]
            }
    
            return super.keyPathsForValuesAffectingValue(forKey: key)
        }
    
        override var isReady:     Bool { super.isReady && state == .ready }
        override var isExecuting: Bool { state == .executing }
        override var isFinished:  Bool { state == .finished }
    
        override func start() {
            guard !isCancelled else {
                finish()
                return
            }
    
            if !isExecuting {
                state = .executing
            }
    
            main()
        }
    
        override func main() {
            fatalError()
        }
    
        func finish() {
            if isExecuting {
                state = .finished
            }
        }
    
        override func cancel() {
            super.cancel()
            finish()
        }
    }
    

    The dynamic property with keyPathsForValuesAffectingValue ensures that the relevant KVO notifications take place.