Cannot access property with a non-sendable type from non-isolated deinit

Getting a new warning from some existing code, with Swift 5.7 / Xcode 14b1. I have several actual examples, but here's a template:

@MainActor class MyClass {
    // ^ usually implicitly @MainActor via inheriting a UIKit type

    var someProperty: SomeUnsendableType

    deinit {
        someProperty.doSomething()
        // ^ warning: Cannot access property 'someProperty' with a
        // non-sendable type 'SomeUnsendableType' from
        // non-isolated deinit; this is an error in Swift 6
    } 

I get why this is an error — Swift generally doesn't want to get into guarantees of which actor deinit might run on 'cos refcounting makes that hard. What I don't know is how to resolve the error:

  • I can't make the type of the property magically Sendable;
  • I can't make the class non-@MainActor (that's up to the UIKit superclass)
  • I can't write isolated deinit (if that would even mean anything, but the wording of the error suggests it)
  • I can't write @MainActor deinit
  • I can't write deinit async to be able to use MainActor.run
  • I can't delete the body of deinit, or my app will (at best) have a resource leak

Help?

8 Likes

Thanks for the thread reference. It goes around a lot of houses without apparently coming to any particularly useful answer. Presumably this is going to be a problem for a lot of existing UIKit code, since basically any UIKit subclass/protocol implementor with a deinit will run afoul of it.

Irrelevant to the Swift side of the discussion except that UIKit has no way to communicate it: it appears that for any UIView or UIViewController, deinit actually is guaranteed to be called on the main queue (thanks to @Nickolas_Pohilets for pointing it out in the other thread). Now if only there was a way to silence the warning :slight_smile:

Perhaps Swift and/or its runtime should employ a technique like UIView/UIViewController do, to generally schedule deinit for actor-bound instances with their actor?

2 Likes

I'm running into this as well, and the project has warnings as errors enabled, so i can't build currently. swift-evolution/0327-actor-initializers.md at main · apple/swift-evolution · GitHub has more information, but it's not immediately clear to me how to solve it, but at least it confirms it's a language feature and not a beta bug.

1 Like

I got the same issue as my ImageDownloader class instance performs stoping download and cleaning cache in deinit. Code actually crashes while launching simulator. We didn't asked for concurrency to be forced upon us :slight_smile: but here we are.

1 Like

Is there a practical solution for this? Any UIKit subclass that uses deinit now essentially causes warnings in Swift 5.7 while they are never touched on other threads anyways.

2 Likes

I ended up working around this by moving setup/teardown from init/deinit to viewWillAppear/viewDidDisappear. That won't work in all situations, but it works for the cases I have.

Looks bug to me: when the code hits deinit it means the object is no longer used by anyone else, and its someProperty is safe to call, thus the warning (or the error) makes no sense.

The problem is that deinit will be called on whichever thread drops the last reference to the object. In the case of @MainActor, there's a chance that's the main thread, but it's not guaranteed (unless you're a subclass of UIView or UIViewController, which override release to ensure that deinit is called on the main thread). In the case of any other global actor, it won't be "on that actor". So deinit acts more like a Sendable closure that captures the fields it references, and is free to run on any actor.

It all makes sense, it's just profoundly hostile to existing UIKit code.

1 Like

But could that lead to data races?! if so how? (the very point of using actors in the first place).

As you mentioned UIView & co is special and has this mentioned "last release treatment".

Yes, if I have some @MyGlobalActor, it's not safe to interact with @MyGlobalActor types or methods from any deinit, currently (because the deinit doesn't run on that actor). Which is why this error exists.

Ah, I see now.
Have a look at this fragment:

@MainActor
class MyObject: NSObject {
    
    var someVariable = "hello"
    
    func doSomething() {
        print(someVariable)
    }

    deinit {
        // can't currently call it here:
        // doSomething() // Error: Call to main actor-isolated instance method 'doSomething()' in a synchronous nonisolated context
        // but...
        
        Task { // or Task.detached, doesn't matter which in this case
            await MainActor.run {
                // wow. I can do this?!
                self.doSomething() // ok?!?! the object is long gone!
            }
        }
    }
}

Is it the case that the "doSomething()" usage is particular problematic here (i.e. the app would crash), and compiler doesn't (and can't possibly?!) issue a warning or an error?

If the compiler doesn't issue an error for capturing self in an escaping closure from deinit, I'd say it definitely should! That seems like a bug.

+1 on this. I'm starting to dig into testing with Xcode 14, and this is one of the more common warnings displayed. In most cases, it it happening around cleaning up block-based notification observers.

I haven't been able to come up with another way around this pattern, and it seems like a major bug if there's no safe way to remove block-based notification observers from UIKit subclasses.

3 Likes

+1 Block observers are about the only thing I use deinit code for. I usually don't have an observer by that point but I always check and release them if they still exist.

I found that simply wrapping cleanup code in a function clears the warning:

func wrap(_ block: () -> Void) {
    block()
}

final class ViewController2: UIViewController {
    private var observer: NSObjectProtocol?

    deinit {
        wrap {
            if let observer = observer {
                NotificationCenter.default.removeObserver(observer)
            }
        }
    }
...

I wonder is this a bug or a feature :thinking:

3 Likes

I created a drop in replacement for addObserver(forName name: ...)

    func addObserver(for name: Notification.Name?, object obj: Any?, queue: OperationQueue?, using block: @escaping (Notification) -> Void) -> NSObjectProtocol

that doesn't need explicit unregister in your class deinit as it does it itself. One less problem to deal with.

An engineer at WWDC recommended, for actors that need to perform cleanup that would previously be done in deinit, may want to add an invalidate call to insure that that work occurs with concurrency protections.

2 Likes

But when do you call it? The whole point is you don’t necessarily know when the object is being freed.

1 Like