Bug in NSOperation? KVO-compliant properties called twice when generating KVO notifications.

Hi,


I've noticed that when subclassing

NSOperation
,
isReady
,
isExecuting
and
isFinished
are called twice when generating KVO notifications. This issue occurs both in Swift and Objective-C.


Here is a short sample code:

let operation = CustomOperation()
operation.updateState()

@objcMembers class CustomOperation: Operation {

    override var isReady: Bool { // called twice
        return super.isReady
    }

    func updateState() {
        willChangeValue(forKey: "isReady")
        didChangeValue(forKey: "isReady")
    }
}


In this case, the issue appears to come from the fact that

class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String>
returns
["isReady"]
for the key
"ready"
and returns
["ready"]
for the key
"isReady"
. Similar issue occurs for
isExecuting
and
isFinished
.

This probably creates some loop that causes the KVO-compliant properties to be called twice.


As a workaround, if I override this property, the properties are only called once:

override class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String> {
     switch(key) {
     case "isReady", "isExecuting", "isFinished": return [] // can return custom state key here
     case "ready", "executing", "finished": return [] // prevent calling isXXX properties twice
     defaut return super.keyPathsForValuesAffectingValue(forKey: key)
     }
}


- Is it a bug of NSOperation that I should report?

- Is this workaround safe to use or may it break the inner workings of NSOperation in some specific cases?


Thanks.

Accepted Reply

I've filed a report for this issue: #34514933.

Thanks.

Isn't disabling automatic notifications for all keys dangerous if the NSOperation internals happen to change in the future and then rely on this feature (although unlikely)?

I don’t think this will be a problem because the queue should interact with the operation via the operaton’s public API but, honestly, I’ve never thought about this until you raised the issue.

In my case I’ve never monkeyed with

isReady
, instead I was focused on
isExecuting
and
isFinished
. In the context of an async operation you typically override them completely, ignoring the implementation you inherit from NSOperation, and thus disabling auto KVO on them is not going to be a problem.

I've just tested on iOS 10 and yes, this problem is new in iOS 11.

Interesting.

I've tried in Objective-C too and experienced the same issue.

Also interesting.

Combined these indicate that this is a change in the underlying implementation of NSOperation, not something tools related. This isn’t a huge surprise. NSOperation is regularly tweaked to improve both and correctness, and we specifically called out some changes in WWDC 2017 Session 244 Efficient Interactions with Frameworks. It’s seems likely that this round of changes is what’s causing you problems.

Share and Enjoy

Quinn “The Eskimo!”
Apple Developer Relations, Developer Technical Support, Core OS/Hardware

let myEmail = "eskimo" + "1" + "@apple.com"

Replies

Is it a bug of NSOperation that I should report?

You should definitely report this. I’m not actually sure where the problem lies here, but it’s a weirdness that someone needs to investigate in depth.

Please post your bug number, just for the record.

Is this workaround safe to use or may it break the inner workings of NSOperation in some specific cases?

Given the tight dependency between NSOperation and KVO, I never rely on automatic KVO notification in my NSOperation code. That is, I specifically model my internal state and implement the KVO notifications manually (disabling the automatic stuff via

+automaticallyNotifiesObserversForKey:
and its friends). In that case the dependency stuff never kicks in.

Two questions:

  • In your keywords you wrote: “ios 11, ios 11 gm”. Is this problem new in iOS 11? Or were you just saying that you tested this on iOS 11?

  • Have you tried doing this in Objective-C? I’m curious what behaviour you see in that case.

Share and Enjoy

Quinn “The Eskimo!”
Apple Developer Relations, Developer Technical Support, Core OS/Hardware

let myEmail = "eskimo" + "1" + "@apple.com"

Thank you very much for your answer.


I've filed a report for this issue: #34514933.


I never rely on automatic KVO notification in my NSOperation code

In my

NSOperation
code, I only generate KVO notifications for my own state variable manually, then propagate this notification to the right
isXXX
properties with
keyPathsForValuesAffectingValueForKey:
.


That is, I specifically model my internal state and implement the KVO notifications manually (disabling the automatic stuff via
+automaticallyNotifiesObserversForKey:
and its friends). In that case the dependency stuff never kicks in.

Do you return

false
for all keys or just your own?

Isn't disabling automatic notifications for all keys dangerous if the

NSOperation
internals happen to change in the future and then rely on this feature (although unlikely)?



In your keywords you wrote: “ios 11, ios 11 gm”. Is this problem new in iOS 11? Or were you just saying that you tested this on iOS 11?

I've just tested on iOS 10 and yes, this problem is new in iOS 11. This issue occurs both in the Xcode simulator and on actual devices.


I've added a print command in the

keyPathsForValuesAffectingValueForKey
method (only return the results from
super
) and obtained the following results in iOS 11:


keyPathsForValuesAffectingValueForKey isFinished = ["finished"]

keyPathsForValuesAffectingValueForKey finished = ["isFinished"]

keyPathsForValuesAffectingValueForKey isFinished = ["finished"]

keyPathsForValuesAffectingValueForKey finished = ["isFinished"]

keyPathsForValuesAffectingValueForKey isExecuting = ["executing"]

keyPathsForValuesAffectingValueForKey executing = ["isExecuting"]

keyPathsForValuesAffectingValueForKey isExecuting = ["executing"]

keyPathsForValuesAffectingValueForKey executing = ["isExecuting"]

keyPathsForValuesAffectingValueForKey isReady = ["ready"]

keyPathsForValuesAffectingValueForKey ready = ["isReady"]

keyPathsForValuesAffectingValueForKey isReady = ["ready"]

keyPathsForValuesAffectingValueForKey ready = ["isReady"]


In iOS 10, I only get:

keyPathsForValuesAffectingValueForKey isFinished = []

keyPathsForValuesAffectingValueForKey isReady = []

keyPathsForValuesAffectingValueForKey isExecuting = []


There is definitely something wrong going on here in iOS 11.


Have you tried doing this in Objective-C? I’m curious what behaviour you see in that case.

I've tried in Objective-C too and experienced the same issue.


Thanks again.

I've filed a report for this issue: #34514933.

Thanks.

Isn't disabling automatic notifications for all keys dangerous if the NSOperation internals happen to change in the future and then rely on this feature (although unlikely)?

I don’t think this will be a problem because the queue should interact with the operation via the operaton’s public API but, honestly, I’ve never thought about this until you raised the issue.

In my case I’ve never monkeyed with

isReady
, instead I was focused on
isExecuting
and
isFinished
. In the context of an async operation you typically override them completely, ignoring the implementation you inherit from NSOperation, and thus disabling auto KVO on them is not going to be a problem.

I've just tested on iOS 10 and yes, this problem is new in iOS 11.

Interesting.

I've tried in Objective-C too and experienced the same issue.

Also interesting.

Combined these indicate that this is a change in the underlying implementation of NSOperation, not something tools related. This isn’t a huge surprise. NSOperation is regularly tweaked to improve both and correctness, and we specifically called out some changes in WWDC 2017 Session 244 Efficient Interactions with Frameworks. It’s seems likely that this round of changes is what’s causing you problems.

Share and Enjoy

Quinn “The Eskimo!”
Apple Developer Relations, Developer Technical Support, Core OS/Hardware

let myEmail = "eskimo" + "1" + "@apple.com"

Thanks. I'll give a look at this session.


Also, in "macOS 10.13 and iOS 11 Release Notes", it says:


NSOperation now responds to KVO and KVC for representing the finished, cancelled, executing and ready states as the strings @“finished”, @“cancelled”, @“executing” and @“ready” in addition to the older versions like @“isReady”.


I guess this change is related to this "issue": https://bugs.swift.org/browse/SR-4397. In fact,

#keyPath
returned the correct key path, but
NSOperation
was not consistent with other Foundation objects.


However, the fact that

isXXX
properties are called twice for each KVO notification doesn't feel right.


If I change

keyPathsForValuesAffectingValueForKey:
to this:
override class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String> {
        switch key {
        case "isReady":
            return ["ready"]
        case "isExecuting":
            return ["executing"]
        case "isFinished":
            return ["finished"]
        case "ready":
            return ["isReady"]
        case "executing":
            return ["isExecuting"]
        case "finished":
            return ["isFinished"]
        default:
            return super.keyPathsForValuesAffectingValue(forKey: key)
        }
    }


then

isReady
is still only called once in iOS 10, but twice in iOS 11.

However,

keyPathsForValuesAffectingValueForKey:
is also called 12 times (there is probably some optimizations to be done here).