Filippo
Filippo

Reputation: 150

Swift 5.10: Cannot access property '*' with a non-sendable type '*' from non-isolated deinit; this is an error in Swift 6

The line authController.signInAnonymouslyIfNecessary() (near the end) in the following Swift UIKit code produces the warning "Cannot access property 'authController' with a non-sendable type 'AuthController' from non-isolated deinit; this is an error in Swift 6":

import UIKit

class AuthController {
    func signInAnonymouslyIfNecessary() {
        
    }
}

class AuthFormNavC: UINavigationController {
    let authController: AuthController

    init(authController: AuthController) {
        self.authController = authController
        super.init(rootViewController: ConsentVC())
    }
    
    required init?(coder aDecoder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
    
    deinit {
        authController.signInAnonymouslyIfNecessary()
    }
}

Swift 5.10, Xcode 15.3 with complete strict concurrency checking.

What is the workaround?

Please don't ask me why I'm doing what I'm doing or anything unrelated to the question.

If you're wondering why I want to call authController.signInAnonymouslyIfNecessary() when the navigation controller is denitialized, my goal is to call it when the navigation controller is dismissed (or popped), and I think that the deinitializer of a view controller is the only method that is called if and only if the view controller is being dismissed (or popped) in my case. I tried observing variables like isViewLoaded in the past using KVO but I couldn't get it to work passing any combination of options in observe(_:options:changeHandler:).

Upvotes: 2

Views: 1470

Answers (2)

Rob Napier
Rob Napier

Reputation: 299663

First, to start with the answer, then I'll explain why there's a real problem here, and then I'll work up to why this solves that real problem, not just a way to "make the compiler happy."

The answer:

  • Mark AuthController as @MainActor
  • Mark signInAnonymouslyIfNecessary as nonisolated
  • Wrap signInAnonymouslyIfNecessary in a Task:
@MainActor final class AuthController {
    nonisolated func signInAnonymouslyIfNecessary() {
        Task { @MainActor in
            /// ...
        }
    }
}

With that out of the way, here's why:

UINavigationController (and by extension AuthFormNavC) is @MainActor. However, there is no guarantee that deinit will be called on MainActor. (It turns out, through a great deal of work from the UIKit team, it is extremely likely to the point of near certainty that it will be called on MainActor for UIView and UIViewController, but this is explicitly not promised, and is not true in general.)

This means that the warning is correct. It is not valid to access self.authController on an arbitrary thread. This is a long-existing, real-world cause of crashes when trying to remove KVO observations in deinit, which is why we've needed packages like PMKVObserver.

It is also not valid to call authController.signInAnonymouslyIfNecessary() from a context other than the one that created authController (i.e. MainActor). AuthController is not Sendable (and probably is not thread-safe).

I say all of this to point out that this code was never actually legal, and in the general case (outside of UIKit's magic) it is a long-time cause of real-world crashes that Swift Concurrency is here to help stamp out.

But what do we do about it?

First, it seems clear that AuthController intends to be MainActor, so it should be marked such:

@MainActor class AuthController {

Second, while it is illegal to extend the lifetime of self in deinit, it is certainly not illegal to extend the lifetime of a property of self. Here's a direct way to do that (though there is a safer way that I'll get to next).

    // This is for illustration, see below for a safer solution
    deinit {
        // Do not overlook the `[authController]` here!
        Task { @MainActor [authController] in
            authController.signInAnonymouslyIfNecessary()
        }
    }

I need to very strongly stress the importance of the capture list. This makes authController a local, copied variable inside the Task, and avoids capturing self. Failure to include this capture will crash, and there will be no compiler warning about it. It's been discussed a few times on the lists, and there is at least one FB about it. I've opened a bug report as well. The good news is that capturing self in deinit will reliably crash rather than just being undefined behavior. Swift checks the retain count at the end of deinit to make sure self didn't escape.

So wrapping things up in a Task solves the problem, but it also creates a new headache. We have to be very careful about our capture lists. deinit code often isn't tested well, and the least mistake could crash the program. It would be nicer if we could say "no Tasks in deinit; they're too dangerous." And we can.

Keeping AuthController as @MainActor, we can say that it's safe to call signInAnonymouslyIfNecessary from any thread using nonisolated:

@MainActor final class AuthController {
    nonisolated func signInAnonymouslyIfNecessary() {
        Task { @MainActor in
            /// This block will likely capture `self`. That's good.
            /// There is no reason for a `weak` dance here; there's no cycle.
            /// We want `self` to live long enough to finish the job.
            /// ...
        }
    }
}

With that, your original deinit is fine. Even if you don't control AuthController, you can extend it to provide a nonisolated method that calls the actual method in a Task.

As a rule, deinit should be avoided, especially for anything important. It has always had many tricky corner-cases and has always been challenging to use reliably, even in the ObjC days with -dealloc. Swift Concurrency is just surfacing a lot of those long-standing problems. As @Sweeper notes, this particular case probably should be done in viewDidDisappear, which is likely the behavior you actually mean, or in response to the model changing, rather than relying on a side-effect of memory management of a view controller. But also it's an important question, and the general case is worth exploring.

Here is a simplified version of the code to play with and see what does and doesn't work:

@MainActor class Service {
    func doThings() {}
}

@MainActor class Controller {
    let service: Service

    init(service: Service) { self.service = service }

    func printMe() { print("Controller lives") }

    deinit {
        print("Controller deinit")

        // As written, this crashes. Adding [service] here will fix it.
        Task { @MainActor in
            service.doThings()
        }
    }
}

let service = Service()

do {
    let controller = Controller(service: service)
    controller.printMe()
}

Upvotes: 10

lorem ipsum
lorem ipsum

Reputation: 29696

You can make anything conform to Sendable if the requirements are met

Value types

Reference types with no mutable storage

Reference types that internally manage access to their state

Functions and closures (by marking them with @Sendable)

Are there any properties in AuthController? If there aren’t any (per your sample) you can add Sendable to it.

If there are, you need to pick an approach to make it safe.

https://www.avanderlee.com/swift/sendable-protocol-closures/

Upvotes: 1

Related Questions