MainActor isolated property can not be mutated from deinit

I just started the long process of turning an existing codebase into one that compiles when SWIFT_STRICT_CONCURRENCY = complete has been set. I'm stumped by the following error:
Main actor-isolated property 'observed' can not be mutated from a non-isolated context
The error is reported for the single statement in the deinit function below. The explicit setting of the property to nil is so that the setter can stop observing Key-Value change notifications on any previous, non-nil property value.

@MainActor
@objc public class MyPopUpButton: NSPopUpButton {
    private var _observed: SomeObjcNonSendableClass? = nil
    
    deinit {
        observed = nil // Main actor-isolated property 'observed' can not be mutated from a non-isolated context
    }
    
    @objc public var observed: SomeObjcNonSendableClass? {
        get {
            _observed
        }
        set {
            _observed?.removeObserver(self, forKeyPath: #keyPath(SomeObjcNonSendableClass.interestingValue))            
            _observed = newValue
            _observed?.addObserver(self, forKeyPath: #keyPath(SomeObjcNonSendableClass.interestingValue), options: [.initial, .new], context: nil)
        }
    }
    // Here imagine a observeValue(forKeyPath:...) function that does something interesting when 'observed.interestingValue' changes
}

I get it that deinit is not guaranteed to be executed under @MainActor isolation. I get it that most vintage code relying on these patterns has worked insofar as instances are deallocated/released on the main thread because KVO isn't thread safe (true enough if you consider how your average NSControl sub-class is used).

What technique would work here to eliminate this error and guarantee that MyPopUpButton instances stop observing Key-Value Change notifications when the instance is being released/dealloc'd?

Is the one-size-fits-all solution just massive bracketing of similar code across our codebases in MainActor.assumeIsolated { ... }?

1 Like

In this particular case you could repeat the call to removeObserver(_:forKeyPath:) and it should be fine, since that method is thread-safe. The access it's complaining about the setter for observed, because the setter could have been doing something that actually requires main-thread-isolation and not just synchronization. I'll leave general recommendations to others though.

You probably guessed that I'm trying to acquire some technique that is applicable in general, but for the record I did try the following:

deinit {
    _observed?.removeObserver(self, forKeyPath: #keyPath(SomeObjcNonSendableClass.interestingValue))
}

But the error is replaced by an error-to-be:

Cannot access property '_observed' with a non-sendable type 'SomeObjcNonSendableClass?' from non-isolated deinit; this is an error in the Swift 6 language mode
1 Like

My bad! I guess I had assumed the check would be waived for stored properties and dependent only on what you called, but that's not how it works at all. Hm! I'll wait for a proper answer along with you, then.

I may be showing how green I am re: Swift Concurrency and likely a lot more, but I would have expected a similar error/warning for something like this:

@MainActor
@objc public class SomeController: NSObject, NSTableViewDelegate {
    @IBOutlet public weak var pickerTableView: NSTableView? {
        didSet {
            pickerTableView?.delegate = self
        }
    }
}

Yes the entire class is stamped with a @MainActor, but a weak reference to a NSTableView may indeed become nil on just about any thread (after all its deinit isn't guaranteed to be called on main thread). So the statement pickerTableView?.delegate = self either isn't guaranteed to happen within Main Actor isolation, or the problem is a deeper misunderstanding: the didSet is only ever called when some code explicitly sets that weak property, and not when the weak reference becomes nil in general. (Before anyone gripes that "you cannot really observe when a weak instance becomes nil", I beg you to think of this and other impossibly cool things you can in fact do via ObjC runtime.) And so the compiler assumes that it can enforce Main Actor isolation on the explicit invocation of the property setter, and hence the body of didSet { } inherits that isolation context.

If my assumption is correct, a didSet on a weak variable doesn't make much sense in Swift, with or without concurrency. At the very least there should be a warning, e.g. Since "pickerTableView" is a weak variable, the didSet closure is only executed on explicit access of the property. You really don't want this.

Even in ObjC, a weak property’s setter is not called when the object is deallocated; the storage silently goes to nil. Swift is the same, except that some of the weak references are resolved lazily. ObjC does have destruction-notification features, but weak has never been one of them.

This is an interesting but very off-topic discussion. Yes ObjC doesn't provide any syntactic sugar or runtime APIs to specifically track when a variable becomes nil (weak or strong). I am in fact still confused as to what the Swift language spec says about the execution of that didSet { } closure on a weak variable. I hope we can all agree that if the value of the property can be "set", you would expect that closure to executed. And if the runtime cannot guarantee that the closure will execute under all circumstances in which the value is set... my opinion is that at least a warning by the compiler seems appropriate. As far as ObjC goes, you can build a way to track deallocation of strong or weak instances via associated objects (you probably know this well... but worth filling in some details for curious others). Tie an associated object to the one to which you hold a strong or weak variable to. When the observed object is deallocated, the associated object goes down too and in its dealloc method you can post a notification / invoke a method on a weakly-held delegate / invoke a block / etc. to notify any observers that the strong/weak reference has just become nil. There’s definitely a couple tricky aspects to consider: making sure your associated object is only retained by the observed object; generating a unique token to identify the instance since you cannot/should not really use the pointer value. But it’s both possible and incredibly useful, and very much off-topic :-) I'm still just hoping to figure out how bad of an idea it is to try and bring Swift Concurrency into a mixed ObjC / Swift codebase where existing concurrency has so far only been implemented through GCD queues/semaphores/mutexes, unfair locks, etc. I can't be the only one with this problem, right? If anyone has recommendations for websites, blogs, books they’d be very appreciated.

I wouldn't. Both because I assume that technically the variable is treated as just a uintptr_t in memory that gets zeroed, and also because semantically the value isn't being set, it's simply going away. nil is not a value, it's a special sentinel; it's what you get when you try to read the value and find that it doesn't exist. (it's also used, for convenience, as a way to delete the value through assignment, as opposed to separate syntax for that, e.g. del myVar)

I mean, I wouldn't have been bothered if it turned out didSet was called in this situation (I can see the appeal of that behaviour), I just don't think it has to be. And I acknowledge that it's not unambiguous as to what should happen, since it depends on semantic interpretation.

This fits well the earlier hypothesis: the compiler knows didSet only executes when the property is set explicitly. As to whether nil is a sentinel rather than a value... I can agree with that conceptually, but if that variable gets zeroed out because the last strong reference is gone, or because some part of your code just set it to nil, the same underlying "value" (aka bit pattern) is used to represent both states (right?). That brings up a valid point... there are no separate values to track "nil-by-memory-subsystem" vs "nil-by-setter" and maybe there should be. Aside from this detour, I’ve got no trouble accepting how the compiler works in my second example... but still really just hoping to figure out the answer to my original question. I'm barely scratching the surface and already feel very uneasy about how many @unchecked Sendable, MainActor.assumeIsolated { ... }, nonisolated(unsafe) annotations I'm adding without truly understanding what, if anything, can be done to avoid them altogether.

I think I asked pretty much the same thing, but there wasn't much answer except it should be better in Swift 6/the future. But it doesn't seem to have.

Removing an observer in deinit is pretty standard practice and I think recommended in some docs.

Thanks for sharing that link... looks like we need a couple more years for the dust to settle.

It looks like this is a long-standing open problem that has been addressed in this pitch / this proposal, which is being worked on but hasn't yet landed.

The proposal for isolated deinit will address this: Isolated synchronous deinit.

We've just given it another round of discussion [Sub-Pitch] Task-local values in isolated synchronous deinit and async deinit where we've reached a design conclusion -- so now @Nickolas_Pohilets will update the proposal and it will be reviewed and hopefully accepted :slight_smile:

Then you'd be able to say

    deinit isolated {
        observed = nil
    }

The second review will be happening hopefully very soon. I am not sure if it could be considered for 6.0 or not at this point.

3 Likes