[SR-5816] KeyPath-based KVO: Unable to stop observing owned object on deinit

[7b0e3195-6b36-43c6-964e-4394cb941184]

Previous ID	SR-5816
Radar	None
Original Reporter	@antonvmironov
Type	Bug

Environment

Version 9.0 beta 6 (9M214v)

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 7 | |Component/s | Foundation | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: cc033b62e7f6826a74055dc7ea71bfeb

Issue Description:

MyHostObject owns MyObject and observes changes of it's value:

import Foundation

class MyObject: NSObject {
  @objc dynamic var value: Int = 0
}

class MyHostObject: NSObject {
  @objc dynamic let myObject = MyObject()
  var _observation: NSKeyValueObservation?

  override init() {
    super.init()
    _observation = observe(\.myObject.value, options: [.new]) { (myObject, change) in
      print("value changed to \(change.newValue!)")
    }
  }

  deinit {
    // `myObject` is in Limbo right now, but I can still call it's methods and, hopefully, remove observation
    _observation?.invalidate()
    _observation = nil
  }
}

var myHostObject: MyHostObject? = MyHostObject()
myHostObject = nil // crashing here with:
// *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'An instance 0x101c10290 of class issue_NSKeyValueObservation.MyHostObject was deallocated while key value observers were still registered with it. Current observation info: <NSKeyValueObservationInfo 0x101c14ed0> (
<NSKeyValueObservance 0x101c14c20: Observer: 0x101c115f0, Key path: myObject.value, Options: <New: YES, Old: NO, Prior: NO> Context: 0x0, Property: 0x101c129b0>
)'

With Objective-C equivalent, MyHostObject would still be able to remove observation from myObject.

This is happening because NSKeyValueObservation holds a weak reference to an object. That weak reference turns to nil too soon. My suggestion is to use unowned or Unmanaged\<MyObject> reference.

[belkadan]

cc @Catfish-Man

[Catfish-Man]

Hmm. I wouldn’t expect this to be an issue in iOS 11/macOS 10.13 due to KVO changes, but could be important for backwards compatibility.

[7b0e3195-6b36-43c6-964e-4394cb941184]

Just tested on iOS 11. No issue there.

[bobergj]

Still an issue in macOS 10.12.6 and iOS 10, Swift 4.0 Snapshot 2017-10-09 (a). This makes the new closure-based observe basically unusable for us since we need to be compatible with iOS 10. Only in objects with lifecycle methods (viewDidDissapear and such) can this be workaround by invalidating before dealloc.

[bobergj]

Also, according to the Foundation Release Notes for macOS 10.13 and iOS 11 (https://developer.apple.com/library/content/releasenotes/Foundation/RN-Foundation/index.html), the automatic unregistration of observers on dealloc seem to be subject to some conditions (automaticallyNotifiesObserversForKey).

[sinoru]

It seems like fixed in Swift 5.1 (iOS 11). But this fix leads app to crash with previous iOS workaround (Manually removing observer in deinit). What should I do?

Error: `Cannot remove an observer \<_NSKeyValueObservation 0x28196ef10> for the key path "textField.selected" from \<#{SomeClass} 0x10192a480> because it is not registered as an observer.`

[sinoru]

Definitely this is fixed by https://github.com/apple/swift/pull/20103