Deinit and MainActor

I've hit this problem when playing with actors. A possible solution would be to allow declaring deinit async. I haven't though it out exhaustively, but intuitively, here's how I think it would work:
if a type has a deinit async implemented, then when the reference count reaches zero an implicit call to something like object.deinit() is replaced by async { await object.deinit() }. I thought about the possibility of await-ing on the object if it went away in an async scope, but that would be inconsistent with the behavior from inside a sync scope. Regarding the "deterministic lifetime" principle: If a type defines deinit async, then by definition, the author realizes that there actual deinitialization happens outside of the normal control flow.

1 Like

IIRC, some UIKit classes (UIViewController at least) have some hackery that dispatches dealloc to main thread if last release happened from the background queue.

That wouldn't actually work because you can't keep an object alive in dealloc (at least with ARC). Once dealloc returns the object's memory is going to be reclaimed and possibly reused. If you tried to dispatch to another thread and then use self you would almost certainly crash.

You could potentially dispatch to another thread and do other cleanup things as long as you don't reference self and retain anything else you need to use, but it's generally not a good idea, and I don't believe UIViewController does any such thing.

The general guidance for things that require asynchronous cleanup is that they should have explicit lifetime management, i.e., an explicit stop or cleanup method that the client has to call. If the object is deallocated before that method is called then that's a programmer error and may crash (some APIs deliberately crash in this situation to make the behavior more predictable).

Relying on dealloc or deinit to to do cleanup that has to be asynchronous or has to be on a particular thread is just not a safe pattern.

I guess the question I still can't resolve is: why does this compile when I inline the contents of closeAll into deinit, but raise an error if I abstract my cleanup calls into a method and then call it in deinit?

1 Like

Object is kept alive not in dealloc, but rather in release. UIViewController overrides release method. Overridden implementation is implemented in non-ARC code, and its logic can be described by the following preudocode:

- (void)release {
    atomic decrement retain count
    if it was the last reference {
        if it is the main thread {
            [self dealloc];
        } else {
           // No retain, no resurrection
           // self is passed as non-retained context
           dispatch_barrier_async_f(dispatch_get_main_queue(), self, _objc_deallocOnMainThreadHelper);
        }
    }
}
void _objc_deallocOnMainThreadHelper(void *context) {
    [(id)context dealloc];
}

Implementation ensures that after atomic decrement reading weak references returns nil, even if dealloc was not called yet.

As far as I can tell that is no longer the case and hasn't been for a while. Regardless, you can't do that with ARC. In Objective-C you're allowed to not use ARC if you want to hack around things, but in Swift that's not an option so there's no getting around this.

I wrote my previous message staring into assembly of the UIKitCore from iOS 14.5 Simulator. Didn’t check on device, TBH, but I think it is reasonable to expect the same behavior on device as well.

I’m not sure what you mean here. I’m trying to make a point that technically it is possible to make deinit isolated on the actor. But that would be a feature of the Swift runtime, implemented in C++ (so, no ARC), supported by the compiler feature.

I’m not suggesting that @moreindirection should try to override the release method, that is indeed impossible, I’m questioning if deinit need to be non-isolated.

1 Like

Oh, I do see some usages of that pattern in UIKit (very well hidden), but not in UIViewController. Maybe it's somehow even more buried there. Oh well.

I'm still not convinced that's comparable to what's being proposed here. The code surrounding the usage of _objc_deallocOnMainThreadHelper uses manual refcounting logic such that it knows (synchronously) whether we're actually going to deallocate the object or not before dispatching dealloc to the main queue. By the time dealloc is called on the main queue we've already committed to deallocating it, and the normal ARC semantics apply (i.e., you can't resurrect the object at that point, and you can't do any further asynchronous cleanup from within dealloc).

If you allowed dealloc (in ObjC) or deinit (in Swift) to itself be asynchronous then that would require the ability to hold that object in memory for some indeterminate amount of time as it hops potentially from one thread to another. That would require further refcounting to track when it's really safe to deallocate that object. The semantics of these languages don't allow for that kind of resurrection. I've seen that in garbage-collected languages via their finalizers, but even those run before the object is officially committed to being collected, and the finalizer can't be asynchronous (it has to commit synchronously to either resurrecting or not).

I was talking about synchronous deinit, but isolated in the actor. This is sufficient to solve the original problem of this thread.

Asynchronous deinit is a different topic, but it is also possible. Resurrection is a problem, but it is completely orthogonal to asyncronousity.

It is a problem for synchronous deinit as well. The following code compiles without errors, but crashes in runtime:

var objects: [AnyObject] = []

class Inner {
    var name: String

    init(name: String) {
        self.name = name
    }

    deinit {
        print("Inner.deinit(): name=\(name)")
    }
}

class Test: CustomStringConvertible {
    var inner: Inner?

    init() {
        inner = Inner(name: "\(ObjectIdentifier(self))")
    }

    var description: String {
        return "Test(\(inner?.name ?? "nil"))"
    }

    deinit {
        print("Test.deinit()")
        objects.append(self)
    }
}

func foo() {
    let t = Test()
    print(t)
}

func bar() {
    foo()
    print("Now in bar()")
    for x in objects {
        print(x)
    }
    objects.removeAll()
}

bar()

Some options to solve this problem, both for sync and async deinit:

  • No nothing, crash only if resurrected object is accessed (observed behaviour)
  • Run deinit with refcount=1 and assert that it is still 1 after deinit
  • Implement escape analysis for values other than closures and catch this in compile time.

Note the for the last solution, closeAll() from the original problem would need to be marked somehow to indicate that self is non-escaping. Or vice versa - assume that self is non-escaping in methods by default, and annotate escaping methods explicitly.

1 Like

Oooh, I see. So as long as deinit starts on the right actor context and completes synchronously then it should be safe, and that is analogous to the example with UIKit that you mentioned. You're right, I don't see any obvious flaws in that approach. That said, I'm definitely not an expert on the actor proposals.

Sorry for the derail. I saw the talk about async stuff above and thought that was what was being proposed.

I still would be interested to hear from the core team on the subject of isolated deinit. After quick searching I don't see this topic being raised before (or did I miss it?). @Douglas_Gregor, @Joe_Groff, wdyt?

Would it be possible to make deinit isolated for actors and classes isolated in global actors? I guess implementation would involve adding a slot for the function getting a scheduler for the deinit to class metadata, but if wrapped into a flag new runtime still would be binary compatible with existing binaries, right?

3 Likes

As @benlings notes, the last reference to an actor can go away anywhere---in any task---so you are not guaranteed to be on the actor. You might be in synchronous code, so you cannot "hop" over to the actor. For global-actor-qualified classes, that's the end of the story: there is no way to get to the global actor without escaping self, which you aren't permitted to do.

For actor instances, you do know that you have a unique reference to the instance. For the native actors, we could call this isolated because we know that each actor instance is independently synchronized. However, this falls apart immediately when an actor has a custom executor for the same reasons that global actors don't work: the custom executor might be shared with other actors, and you cannot synchronize appropriately with them. Similar issues issues exist for initializers, which have a similar property of having a unique self (unless they escape it).

Doug

2 Likes

Would it be possible to wrap deinit into a top-level task when last release happens from synchronous code?

In asynchronous code, it may be possible to run deinit as a child task, but this would turn every release into an implicit suspension point. So, instead even for asynchronous code release should remain a synchronous operation which launches deinit in a top-level task. "Detached" task and "top-level" task are the same thing, right?

Could you elaborate on this? Where would references to self remain after deinit is executed on the executor? Is it possible to create from C++ code of the runtime a job that stores self without retaining it?

This is effectively escaping self. Remember, deinit is the teardown after we've already done the last release. It's reference count hit zero and must not increase again, and we can't block on this other task.

If the right semantics for your actor is to schedule a task to run later to complete teardown, capture the stored property values you need into that new task and let the deinit finish synchronously. But the actor instance itself has to go away.

Doug

4 Likes

Escaping from what boundary?

Calling deinit on the scheduler de-facto makes release asynchronous. And self does not escape this asynchronous release(). But as I argued above, release should not await on deinit, so deinit must be detached and de-jure release remains synchronous. IMO, escaping self from synchronous release is not a problem, as a long as it does not escape from logical asynchronous release. Do I miss something?

I don't see how reference counting is related to blocking. If we can create a job that captures self as unowned(unsafe), it should not matter if we block, await or detach. But as I argued before, it needs to be detaching. Can job capture self as unowned(unsafe) or is there something in the concurrency design that prevents this?

This topic was brought up again in another thread.

I looked up how executors are normally switched for async methods - it is done using swift_task_switch which takes async context as one of its parameters, what we obviously don't have in the release/dealloc context.

But maybe it is doable using more low-level tools. @Douglas_Gregor, can Job be constructed without any task? If so, then compiler could synthesise code in __deallocating_deinit that switches to the desired executor.

void MyClass__deallocating_deinit(void *self) {
    ExecutorRef myExecutor = ... // Read executor from self in case of actor or use one from global actor
    if (swift_task_isCurrentExecutor(myExecutor)) {
        return MyClass__deallocating_deinit_impl(self);
    }
    // New runtime function
    // Creates job in regular heap, does not use task allocator
    // Should priority be copied from the current task?
    Job *deallocJob = swift_task_createDeallocJob(self, MyClass__deallocating_deinit_impl);
    swift_task_enqueue(deallocJob, myExecutor);
}

void MyClass__deallocating_deinit_impl(void *self) {
   ...
   swift_deallocClassInstance(self, ...);
}

Would this work?

Even if it can't be made to work for a general global actor, it would still be useful to have a special case for @MainActor. Though I suspect there's a bunch of details to work out for all those ObjC base classes that don't currently special-case like UIView/UIViewController do?

I like this @epam-gor-gyolchanyan's idea, and having actor-isolated deinit async is probably a better solution than trying to capture all actor properties into a new task manually for clean up as @Douglas_Gregor mentioned, which is obviously getting difficult when actor has tons of internal properties:

I also think deinit async is slightly better than having synchronous actor-isolated deinit (without async) since non-isolated deinit is preferred whenever possible, and it looks symmetric to how actor-initializer works (synchronous non-isolated init and isolated init async).

cf.

I don't know much about compiler-internal whether deinit async (or synchronous actor-isolated deinit for my 2nd favor) can be technically possible, but since accessing actor-isolated methods is difficult inside deinit, if Swift keeps providing non-isolated synchronous deinit only, I really wish there is at least some magicalSelf keyword to help Swift developers not do heavy lifting.

actor Foo {
    // Endless actor-isolated internal properties that can each call `cleanUp()`.
    var state1, state2, state3, ...

    // Clean up code used in many places, including `deinit`.
    func cleanUpAll() {
        state1.cleanUp()
        state2.cleanUp()
        ...
    }

    // Assume `deinit` is still non-isolated as is:
    deinit {
        // self.cleanUpAll()  // ERROR: can't use actor-isolated method

        // BAD: Copy-pasting same code here is cumbersome. 
        Task.detached { [state1, state2, ...] in
            state1.cleanUp()
            state2.cleanUp()
            ...
        }

        // GOOD: Single line of code, and works like magic!
        magicalSelf.cleanUpAll()
    }
}

Update: Pitch thread - Isolated synchronous deinit

I've a made PoC for isolated synchronous deinit - Comparing apple:main...nickolas-pohilets:mpokhylets/isolated-deinit · apple/swift · GitHub. Will make a pitch soon, but early feedback is welcomed.

I still need to make few changes to support @objc classes and update Sema rules to not generate isolated deinit if there is no custom deinitialising code.

  • Introduced new runtime function swift_task_performOnExecutor. If executor switch is needed, it wraps provided context pointer and work function into an AdHoc job and schedules it on the new executor. It doesn't do any reference counting is safe to be used from dealloc. Ad hoc job is created with priority of Task.currentPriority.

  • If deinit is isolated, then usual implementation of the __deallocating_deinit goes to a new function - __isolated_deallocating_deinit, and __deallocating_deinit (called from swift_release) becomes a thunk that schedules __isolated_deallocating_deinit on the correct executor.

// foo.swift
@MainActor
func mainFunc() {
    if Thread.isMainThread {
        print("MainThread!")
    } else {
        print("Not Main Thread")
    }
}

@MainActor
private final class Bar {
    deinit {
        mainFunc() // OK, deinit is isolated.
    }
}


2 Likes

I can’t speak on behalf of the core team, but I believe we’d be more interested in making async deinit be the “opt into isolation” for deinitislizers. The same way as initializers do. There’s no real downside to making the deinit also be async as well as making that the isolated one. One could also argue for “isolated deinit” but that’s a somewhat new spelling we don’t have anywhere else…

This should all also work for instance actors, not just global ones imho

1 Like