Deinit being flagged by concurrency checker when @Observable added?

Using Swift
1 
@Observable
@MainActor
final class WatchesOrientation {
    private var observer: NSObjectProtocol?
    
    init() {
        observer = NotificationCenter.default.addObserver(forName: UIDevice.orientationDidChangeNotification,
                                                           object: nil, queue: nil) { _ in
            print("Orientation changed")
        }
    }
    
    deinit {
        if let observer = observer {
            NotificationCenter.default.removeObserver(observer)
        }
    }
}

This flags the deinit with:
"Main actor-isolated property 'observer' can not be referenced from a non-isolated context"

  1. I understand that in theory my class can be deallocated from anywhere. Unfortunately, I don't see what to do about it; I can't launch a Task which outlives the deinit and captures self because that would "cross the streams." (aka "bad things" would happen.)

  2. Curiously, if you comment out "@Observable" at the top the warning goes away! That seems like other a bug (why should making it observable change how it interacts with concurrency?) or maybe I just don't understand what's going on.

At the end of the day: what is the current best way to handle a deinit() that needs to actually use self? Frankly, what kind of deinit wouldn't use self?

I have strict concurrency checking on in Xcode 15.3.

2 Likes
2

Not quite an answer, but, unless you have an aversion to it you could use the older selector based notification registration that IIRC doesn't require removing observer (hence no deinit or observer variable would be needed).

3

Thanks, however, I'm more interested in understanding what's going on. In actual fact, the instance I construct lasts the life of the program, so deinit won't actually ever be called, and I have simply commented it out for now.

Still, this is a problem that seems to need a solution at some point, and I'd really love to know how to do this right. For more complex examples, one would need to deinit and I'm curious if this is a compiler bug, or me not understanding things.

4

Yeah, got you. To this and the other concurrency warning question you posted just now: I take quite relaxed approach to concurrency warnings based on what I saw (e.g. DispatchQueue.main.async {} is fine but let q = DispatchQueue.main; q.async {} is somehow not fine). I consider the issues to be one or a combination of any of these:

  • it's a bug of concurrency checker
  • it's a bug in something that's not your code (std library, Observation, OS, etc)
  • it won't be actually an error in Swift 6
  • it will be fixed by the time Swift 6 is out
  • it's your bug that you'll be able fixing when Swift 6 is out based on the new information you'll have by then (WWDC sessions, this forum in a few months time, etc)

So unless you have to treat warning as errors I don't see this as a pressing issue at this time.

But, I'd also like to hear what others will come up with on the matter.

5

@Observable turns all your stored properties into computed properties, so you can observe them. That means if let observer = observer is going to potentially run arbitrary code in your deinit, as far as the compiler knows (it doesn't look into computed property implementations while type-checking deinit).

You don't actually want this behavior, so you have a few options:

  • Use let instead of var. But I assume from the fact that your observer is optional that you sometimes want to remove it early.

  • Use @ObservationIgnored to opt observer out of observing. (The two uses of "observe" in this example are definitely confusing!)

I would not advise ignoring these warnings on any type that has active macros without first expanding the macros. Maybe observer's getter really does do something that isn't safe off the main thread. (…okay, this particular example is pretty unlikely, but I really don't know how Observable is implemented.)

3 Likes
6

Those were some very good suggestions. Unfortunately....

  1. There's a reason it's "var" with an optional: otherwise, it won't compile because we reference "self" in the closure, and you get a "used before all properties set."

  2. And, unfortunately, adding @ObservationIgnored changes from an error to this warning: "Cannot access property 'observer' with a non-sendable type '(any NSObjectProtocol)?' from non-isolated deinit; this is an error in Swift 6"

This makes no sense, since I have concurrency set to "strict" so it should error out now. And given that we've opted out of observation, why is it happening?

1 Like
7

This compiles for me:

    @ObservationIgnored private let observer: NSObjectProtocol
    
    init() {
        observer = NotificationCenter.default.addObserver(forName: UIDevice.orientationDidChangeNotification, object: nil, queue: nil) { _ in
            print("Orientation changed")
        }
    }
    deinit {
        NotificationCenter.default.removeObserver(observer)
    }

Also this:

    @ObservationIgnored private var observer: NSObjectProtocol = NSObject
    ... as above
9

I'm sorry. I confused things by making the example too simple.
The actual code is:

observer = NotificationCenter.default.addObserver(forName: UIDevice.orientationDidChangeNotification,
              object: nil, queue: nil) { [unowned self] note in
            guard let device = note.object as? UIDevice else {
                return
            }

            Task { @MainActor in
                self.orientation =  device.orientation.isPortrait ? .portrait : .landscape
            }
        }

and once you do that, you cannot avoid using var and optional for observer.

10

Making it an initialised var helps a bit:

@ObservationIgnored private var observer: NSObjectProtocol = NSObject()
But indeed, you are getting this warning:

You already know what I think about those warnings :grin:

You also mentioned that this class is not going to be deinitialised? Maybe just put fatalError() in there?

11

I'm working in an absolutely new code base, and this is my chance to do everything the "right" way from the get-go, so I want to exploit this as best I can. I normally wouldn't even dream of turning on strict checking so early, but I'm determined to make this new project as concurrency correct as I can and see what happens.

I've ignored things like "Sendable" and annotating with actor isolation until now because it was unfeasible in an old code base. Right now I'm just drinking the cool-aid and not worrying about the hangover to come, if you'll permit some mixed metaphors...

12

Note: this error no longer appears i.e. adding @ObservationIgnored is now a viable path to silencing the error. (I'm sure I've upgraded Xcode versions between April and now.)

While the new proposal on allowing isolated deinit() would have been one pathway to fixing this issue, it is no longer necessary given that @ObservationIgnored works.

And in fact, given the choice between marking the deinit() as isolated, and using @ObservationIgnored, I'd go with the latter since there is no particular reason to schedule deinit() to run in the future when there's absolutely no need to.