Deinit and MainActor

[Nickolas_Pohilets (Nickolas Pokhylets)]

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?

[KeithBauerXero (Keith Bauer)]

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?

[inamiy (Yasuhiro Inami)]

@epam-gor-gyolchanyan

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 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:

@Douglas_Gregor

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.

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.

@Nickolas_Pohilets

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.

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()
    }
}

[Nickolas_Pohilets (Nickolas Pokhylets)]

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.
    }
}

[ktoso (Konrad 'ktoso' Malawski 🐟🏴‍☠️)]

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

[bobspryn]

Late to the party here, but I'm curious how folks would go about canceling a stored task if not allowed to do so on deinit. We have View Models that are ObservableObjects and keep handles on the tasks to cancel them if the ViewModel is deinitialized. Is the only solution making a call from the View .onDisappear instead? What if it weren't tied directly to a view?