Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-15443] KVO compound key path observer rooted at self does not unregister, causes use after free on change #3357

Open
nolanw opened this issue Nov 4, 2021 · 1 comment

Comments

@nolanw
Copy link

nolanw commented Nov 4, 2021

Previous ID SR-15443
Radar None
Original Reporter @nolanw
Type Bug
Environment

Apple Swift version 5.5.1 (swiftlang-1300.0.31.4 clang-1300.0.29.6)
Xcode Version 13.1 (13A1030d)
macOS Big Sur Version 11.6 (20G165)

Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug
Assignee None
Priority Medium

md5: 555024f1b9d5b4ae42896c0ae5f75845

Issue Description:

Hello! When I have an object create a Key-Value Observing observer for changes to that object's property's property (i.e. using a compound key path), no matter how I try to stop observing it, the deallocated observer ends up being notified of subsequent changes. In the best case this causes a crash. For example, running the following (from a file via swift bug.swift or in a Playground):

import Foundation

class Message: NSObject {
    @objc dynamic var delivered: Bool = false
}

class Emissary: NSObject {
    @objc dynamic var message: Message
    
    var observation: NSKeyValueObservation?
    
    init(message: Message) {
        self.message = message
        super.init()
        
        observation = observe(\.message.delivered, changeHandler: {
            object, change in
            print("object", object, "changed:", change)
        })
    }
   
    deinit {
        observation?.invalidate()
        observation = nil
        print("goodbye from", self)
    }
}

let message = Message()
var emissary: Emissary? = Emissary(message: message)

emissary = nil

message.delivered = true // crash here

results in a crash at the indicated line. I see the "goodbye from" get printed, and I see the expected lack of printing from the change handler, but the program does not exit cleanly. The exact crash and stack trace can differ depending on what the address of the deallocated observer points to, but it includes _NSSetCharValueAndNotify and some private NSKeyValueObserving methods.

I first noticed this issue when using Combine's NSObject.KeyValueObservingPublisher, but narrowed it down to Swift's KVO observer. Using the old-style observeValue(forKeyPath:of:change:context:) works in the scenario I show above. Is this meant to be a supported configuration for Swift's KVO observer?

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
@shengyang998
Copy link

shengyang998 commented Jan 12, 2023

Same here.

More info:
lldb shows that observation.helper = nil.
But it should never nil based on this line of code.

    @nonobjc private let helper: Helper
    
    fileprivate init(object: NSObject, keyPath: AnyKeyPath, options: NSKeyValueObservingOptions, callback: @escaping (NSObject, NSKeyValueObservedChange<Any>) -> Void) {
        helper = Helper(object: object, keyPath: keyPath, options: options, callback: callback)
    }

Guessing it's this line that prevents the removeObserver being called?

        @nonobjc func invalidate() {
            guard let object = self.object else { return }
            object.removeObserver(self, forKeyPath: path, context: nil)
            objc_setAssociatedObject(object, associationKey(), nil, .OBJC_ASSOCIATION_ASSIGN)
            self.object = nil
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants