Add destructors to Kotlin and Swift

[linabutler]

The Python bindings already define __del__ destructors for classes, but Kotlin and Swift don’t yet. We’ll want to make sure we wire these up and call them correctly, to avoid leaks and double-frees.

┆Issue is synchronized with this Jira Task

[rfk]

Our hand-written Kotlin bindings don't have destructors either, and I'm told that's on purpose because they're generally considered bad form in Java (for good reasons that I don't think apply in our case). I believe there's some elaborate infrastructure around the Cleaner class and its helpers that we're supposed to use instead but I haven't investigate it in any detail.

Which isn't to say that we shouldn't do it, just to brain-dump some thoughts on how.

[linabutler]

Ooooh, I think the Kotlin equivalent would be the AutoCloseable interface that’s mentioned in the Cleaner documentation! Swift’s deinit works more like a destructor than a GC finalizer, so there’s a closer match there in the semantics.

[thomcc]

Yeah, I agree. That's what all our existing code does. I think if the a-c team wants to do the cleaner infra. They should probably own that (or at least let us know), AFAIK the autocloseable version has been working fine for them.

[jhugman]

In Java 8/Kotlin/Android, the java.lang.ref.Cleaner (the preferred way of doing this) isn't available.

The evil way is the Object.finalize. Effective Java would have us believe that there are only two reasons we should do this, and this is one of them.

But Glean's experience would show that this is a source of crashes (via Sentry), and they went and removed it. This bit from @badboy scared me most:

their implementation in old Android versions is buggy, which means it's very easy to run into "timeout exceptions" because Android is misusing time.

So much for the magical ways on Android.

For the non-magical ways, the explicit cleanup method. So now we come to the second hardest problem: naming; specifically— name collisions.

Android-components is using Closeable; this does has the nice property of working with Closeable.use, but I'm not sure how useful that is. Additionally, aesthetically, I don't like close().

[linabutler]

Yeah, at first I was thinking we would implement {Auto}Closeable and rely on callers either using try-with-resources for short-lived objects, or manually calling close for long-lived ones like, say, PlacesConnection.

You're totally right, though! Aesthetically, close makes sense for types like connections—but connections are more likely to be statics, only closed when the process shuts down. For short-lived objects, like the sprite example, I can't imagine writing, say, sprite.close(), gross! Firefox Desktop calls cleanup methods like this finalize, but just by convention; they're still called explicitly...unlike Java where finalize is inherited and called by the GC. Maybe destroy is a good compromise for the name?

I don't think WebIDL has a concept like this, either—deleter in WebIDL means something totally different. We could maybe have a [Destructor=destroy] annotation on the interface itself, where destroy lets you set the destructor name (and in Swift and Python, we'd ignore those and generate deinit and __del__ methods). Or we just stick with calling it destroy in Kotlin, and add it to our list of banned names (#10).

Using a name like destroy gives up on try-with-resources, but a-c can always make a wrapper that implements AutoCloseable and calls destroy inside close if they really want.

[rfk]

this does has the nice property of working with Closeable.use, but I'm not sure how useful that is

I'm not aware of any concrete uses of this in practice (but then again, there are many things of which I'm not aware). It may be modestly useful for test code that creates and destroys a bunch of instances.

We could maybe have a [Destructor=destroy] annotation on the interface itself, where destroy lets you set the destructor name
(and in Swift and Python, we'd ignore those and generate deinit and del methods). Or we just stick with calling it destroy in
Kotlin, and add it to our list of banned names (#10).

FWIW, I feel like at this stage we should be pretty strongly opinionated about the shape of the generated code, and just declare a name for the destructor rather than trying to let consumers configurate-all-the-things. Naming it destroy until we have a reason not to seems 👍 to me.