Keypath translation for KVO notification seems to not work properly on iOS 10

bug

(Valentin Pertuisot) #1

Hello,

I encountered an issue on Xcode 9.4.1(9F2000) using Swift 4.1.

I'm implementing a custom Operation (NSOperation) and so I need to override different properties of the Operation class and notify through KVO when those values are changing (isExecuting and isFinished in my case) using willChangeValue(for: KeyPath) and didChangeValue(for: KeyPath).

Code example:

class CustomOperation: Operation

    private var internalExecutingState  = false
    private var internalFinishedState  = false

    override var isExecuting: Bool {

        get {
            return internalExecutingState
        }

        set {
            guard internalExecutingState != newValue else { return }
            willChangeValue(for: \CustomOperation.isExecuting)
            internalExecutingState = newValue
            didChangeValue(for: \CustomOperation.isExecuting)
        }
    }

    override var isFinished: Bool {

        get {
            return internalFinishedState
        }

        set {
            guard internalFinishedState != newValue else { return }
            
            willChangeValue(for: \CustomOperation.isFinished)
            internalFinishedState = newValue
            didChangeValue(for: \CustomOperation.isFinished)
        }
    }

This implementation works perfectly on iOS 11 and 12, the system correctly removes my operations from the queue and start the other operations in the queue. (This is a 1 concurrent operation queue).

Issue:

But I discovered that this implementation using Keypath isn't working on iOS 10. No errors are triggered when doing the willChange/didChange but the operations are never removed from the queue and so, the next operations are never getting started by the system.

After one hour of investigating this to understand the root cause of the issue, I discovered that if I notified through KVO using Strings instead of Keypath everything was working fine on both iOS 11/12 and iOS 10 (ie willChangeValue(forKey: "isExecuting") instead of willChangeValue(for: \CustomOperation.isExecuting) or willChangeValue(for : \.isExecuting))

Trying to find documentation for will/didChangeValue(for: Keypath) I discovered this was a Swift only implementation without official Apple documentation since KeyPath doesn't exist in Objective-C and so resorted to read the Swift source code of the will/didChangeValue(for: KeyPath) and discovered that internally the KeyPath were converted to String then passed to will/didChangeValue(forKey: String).

Is it possible that this conversion isn't working properly (due to a reason unknown to me) on iOS 10 ?

Thanks a lot.


(Jordan Rose) #2

It's specifically an issue with Operation, which has this behavior:

Class ObjC Property name Swift property name KVO property name
(NS)Progress totalUnitCount totalUnitCount totalUnitCount
(NS)Progress cancelled isCancelled cancelled
(NS)Operation dependencies dependencies dependencies
(NS)Operation executing isExecuting isExecuting

Strictly speaking, that last one doesn't match the rules for normal key-value coding. (The key name ought to be "executing" rather than "isExecuting".) This is fine in Objective-C where you're passing a string for the key anyway, but using a Swift key path it's based on the Objective-C name of the property you're observing.

(Why the Objective-C name? Because some Objective-C properties have significantly different names in Swift.)

Operation was fixed in iOS 11 to support both "executing" and "isExecuting" as KVO keys for this property, which would explain the behavior you're seeing. If you need to support older OSs, the string key is the way to go.


(Daryle Walker) #3

Have you ever heard of Property Observers? If so, is there a particular reason you're not using them here?

// Warning: typed in forum; not actually compiled
class CustomOperation: Operation {
    override var isExecuting: Bool = false {
        willSet {
            willChangeValue(for: \CustomOperation.isExecuting)
        }
        didSet {
            didChangeValue(for: \CustomOperation.isExecuting)
        }
    }

    override var isFinished: Bool = false {
        willSet {
            willChangeValue(for: \CustomOperation.isFinished)
        }
        didSet {
            didChangeValue(for: \CustomOperation.isFinished)
        }
    }
}

(Valentin Pertuisot) #4

@jrose Thanks for your thorough answer, I've decided to use string for iOS 10, and the keypath for iOS 11+ with a if #available.
As I see you work at Apple, maybe you should suggest the "Operation team" that there should be a warning about using the keypath approach in the Swift version of the Operation documentation when you have a deployment target lower than iOS 11 because currently it directly suggest to use Keypaths :grinning: (https://developer.apple.com/documentation/foundation/operation#1661231?changes=latest_minor)

@CTMacUser Using property observers is indeed a nice improvement, I didn't think about it.