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

Using Swift
21

I have experienced the same. For now, I couldn't find a solution. I believe that it will be fixed in the further releases.

22

FYI here is another similar non-isolated-deinit issue I encountered while using plain actor with Sendable property (array in this example):

actor Foo {
    // I want to clean up this array on `deinit`...
    var array: [String] = []

    func removeAll() {
        array.removeAll()
    }

    deinit {
        array.removeAll() // OK

        // WARNING: Actor-isolated instance method 'removeAll()' can not be referenced from a non-isolated context; this is an error in Swift 6
        self.removeAll()
    }
}

Apparently, array.removeAll() being OK while self.removeAll() showing warning looks strange to me.

23

Assuming Notification and key-value observes removal is sorted (one way or another) I am curious what else do we actually do in our deinits?

As an example, I remember using it for a hand made "leak hunter" (registering objects in init and deregistering them in deinit), that was using a global table without the need to access methods and properties of the object being deallocated. Plus of course a mere "print" logging / debugging / place to put a breakpoint.

24

For the block-based observer pattern, I think I'm going to start working with the following wrapping box, which appears to work as expected to automatically remove the observer on deinit without the warnings.

/// Class to wrap interaction with block-based notification observers. This object will automatically remove its observer on deinit to remove
/// the need to access the `NSObjectProtocol` object in deinit, which causes issues with `MainActor` classes like `UIViewController`
public class TMGBlockBasedObserverBox {
    /// The configured NotificationCenter to use for adding and removing notification observers
    let notificationCenter: NotificationCenter
    /// The notification observer object to use when removing the observer on deinit
    let observer: NSObjectProtocol

    /// Default initializer to create the warpper box instance for a given notification observation
    /// - Parameters:
    ///   - notificationCenter: The NotificationCenter to add the observer to
    ///   - name: The name of the notification to register for delivery to the observer block. Specify a notification name to deliver only
    ///           entries with this notification name. When `nil`, the sender doesn’t use notification names as criteria for delivery.
    ///   - object: The object that sends notifications to the observer block. Specify a sender to deliver only notifications from this sender.
    ///             When `nil`, the notification center doesn’t use the sender as criteria for the delivery.
    ///   - queue: The operation queue where the `block` runs. When `nil`, the block runs synchronously on the posting thread.
    ///   - block: The block that executes when receiving a notification.
    ///            The notification center copies the block. The notification center strongly holds the copied block until you remove the observer registration.
    ///            The block takes one argument: the notification.
    public init(notificationCenter: NotificationCenter = .default,
                name: NSNotification.Name?,
                object: Any? = nil,
                queue: OperationQueue? = .main,
                using block: @escaping (Notification) -> Void) {
        self.notificationCenter = notificationCenter
        self.observer = notificationCenter.addObserver(forName: name, object: object, queue: queue, using: block)
    }

    deinit {
        notificationCenter.removeObserver(observer)
    }
}
1 Like
25

We use it for deregistering internal listeners and other bookkeeping. This is code that has worked fine for almost a decade.

There doesn’t seem to be an escape hatch, so swift does what swift does best again: introduce churn by continously changing things :frowning_face:

1 Like
26

This is because self.removeAll() is actor-isolated, where Array.removeAll() is perfectly safe with a unique reference. In this case it's annoying to duplicate the code, but fundamentally the compiler is forcing you to do what's required to make the code safe.

1 Like
27

Interestingly, when self is accessed inside deinit, my previous successful code started to show warning:

actor Foo {
    // I want to clean up this array on `deinit`...
    var array: [String] = []

    func removeAll() {
        array.removeAll()
    }

    deinit {
        print(self) // Accessing `self`...

        // Accessing `self` will start showing:
        // WARNING: Cannot access property 'array' here in deinitializer; this is an error in Swift 6
        array.removeAll()

        // WARNING: Actor-isolated instance method 'removeAll()' can not be referenced from a non-isolated context; this is an error in Swift 6
        // self.removeAll()
    }
}

I think this warning difference is due to some subtle compiler bug.

And since actor-deinit is non-isolated now, and array.removeAll() is same as self.array.removeAll() which accesses self, I believe "always" showing warning is the correct compiler behavior (which of course is still annoying).

2 Likes
28

I would be interested in knowing how many of the problems people are seeing would be fixed if Swift understood that the deinit of certain Cocoa classes (e.g. UIViewController, if I understand correctly) is in fact isolated to the main actor.

1 Like
29

The fact that UIView and UIViewController try heroically to deallocate themselves on the main thread isn’t part of their API contract.

30

Should it be? It seems unlikely that they could be, or will ever be, modified to stop doing so.

31

Honestly, I just assumed this would be the case, given those classes are annotated with NS_SWIFT_UI_ACTOR in the headers. The deinit warnings were very unexpected when compiling our app using Xcode 14 for the first time, and I don't know how to fix them, without waiting for the compiler to understand they aren't actually a problem.

4 Likes
32

There’s a difference between a binary compatibility constraint and an API promise. Making it part of the formal API contract would retroactively impose the same behavior requirement on all subclasses of UIView. That’s not tenable, and would disincentivize framework authors from making best effort attempts to solve bincompat issues by threatening to turn them into documented requirements in the future.

33

Unless they implement a totally separate reference-counting scheme, a strict reading of the already documented semantics for release suggests to me that such subclasses are already in violation:

You would only implement this method to define your own reference-counting scheme. Such implementations should not invoke the inherited method; that is, they should not include a release message to super.

I'm not sure I understand exactly the failure case you imagine—is it that you'd have some UIView subclass which explicitly moves release calls off the main thread and then operates under the assumption that it will be deinit-ed or dealloc-ed on some non-main thread?

34

Yes, and I can even think of cases where that might already be true.

Like I said, UIView’s current behavior is a best-effort attempt to save users from crashing apps.

35

Yeah, I take your point. Can you think of examples which would not already violate the release semantics quoted above? I realize there may be classes which have fully reimplemented their own refcounting scheme, but I'm more skeptical of that possibility than implementations which just, like, move [super release] to a background queue.

I also realize that the wording there is perhaps not quite strong enough to consider it a prohibition on such behavior.

36

Sorry, I’m not at liberty to discuss the specific class I have in mind.

37

Ah, sorry, I didn't really mean to ask for specifically which class(s) you were thinking of—was just wondering whether the examples you had alluded to were already violating the documented semantics of release. If so, it doesn't make a ton of sense to me to object to this on the grounds that it would be changing the API contract of UIView/UIViewController—these classes were already ignoring other API contracts. (Of course, it may be that pragmatically, it's still not worth it to break such classes regardless of the fact that they're doing the 'wrong thing').

I've also observed sendability checking in UIView/UIViewController deinit to be a pretty significant number of the issues when trying to build a large iOS project in Xcode 14, so it would be great if we could have the compiler recognize that this pattern is safe. Perhaps a middle ground would be to only consider deinit to be main-actor-isolated if the entire hierarchy between the class in question and UIView/UIViewController is Swift-only?

2 Likes
38

I assume that would be pretty common for view controllers, but maybe not for views. I'll get in touch with the UIKit people and see what if anything they're comfortable with.

3 Likes
39

If we're okay with accepting a higher annotation burden, then that could also be augmented to "the entire hierarchy up to an Objective-C class that is annotated with NS_SWIFT_DEALLOC_UI_ACTOR" (or whatever).

1 Like
40

If there was no this special "last release treatment" done by OS and the deinit of my UIView or UIViewController subclass was called on a background queue would it crash my app or what?