[vm/ffi] KeepAlive marker interface

[dcharkes]

We currently have a Finalizable interface that serves two purposes:

1. It keeps objects alive until the end of the syntactic scope.
2. It can have NativeFinalizers attached.

Due to (2), we disallow sending objects implementing Finalizable to other isolates. As that that will most likely be an error.

However, when attaching finalizers via the dart_api.h, we also need the behavior of (1), and we actually want to send objects to other isolates (that's why we resorted to using finalizers from the dart_api.h in the first place.)

We should introduce another marker interface that only has behavior 1.
Maybe the interfaces should even be related.

Ready for bike shedding on the name:

a. KeepAlive
b. IsolateGroupFinalizable

The marker interface should probably live in dart:ffi as well. Because it's mostly used when sending objects via FFI with Handles. Also it would enable relating the two interfaces, helping with documentation. But I can be convinced of other options.

Use case:

• JNI DETECTED ERROR IN APPLICATION: JNI ERROR (app bug): jclass is an invalid global reference: 0x25afa (deleted reference at index 4823) http#1260 (comment)

cc @HosseinYousefi @mkustermann

[dcharkes]

@mkustermann: name option NativeFinalizale.

[mkustermann]

(I misremembered how this works in today's meeting)

The current setup is:

• We can use Finalizer API with any dart object (no restriction) - under the assumption that invoked finalizer callbacks are delayed in event loop, delayed enough to ensure no finalization-before-use issues occur (**)
• We can use NativeFinalizer API only with Finalizables

I think the main reasons we currently disallow Finalizers to be shared across isolates is that:

• If the NativeFinalizer object is GCed then the finalizers are silently / don't trigger anymore
  => That is IMHO a mistake - if C code needs to free up something, we should invoke it, whether the NativeFinalizer object is still alive or not
  => It also leads to non-determinism as an unreachable NativeFinalizer may or may not be collected before the collection of Finalizables. So whether or not the callback is inovked is dependent on how GC happens to be implemented.
• On isolate shutdown we eagerly run all native finalizers (only if the corresponding NativeFinalizers haven't been collected yet)
  => Any other isolate using the objects after isolate shutdown would have use-after-free error
• Objects are copied when sending across isolates: Objects with native resources cannot be copied
  => We do have @pragma('vm:deeply-immutable') now for that.

So marking a class as extends Finalizable and marking it as @pragma('vm:deeply-immutable') allows sending/sharing across isolates but

• is dangerous because isolate death will eagerly run NativeFinalizers and therefore possible create use-after-free scenarios.
• is safe to do if one uses the C APIs - e.g. Dart_NewFinalizableHandle as those are a) tied to an isolate group and are b) guaranteed to run (neither of which are NativeFinalizers are not - see above)

Eventually (with shared multi threading) we want a finalizer API that works across isolates (just like our existing C API works across isolates). So one way would be to have

• isolate-local resources & finalizer APIs - those prevent sending/sharing (shouldn't even work with deeply immutable objects)
• non-isolate-local resources & finalizer APIs - those allow sending/sharing (should work with deeply immutable objects)

But this may require two different marker interfaces or otherwise leaves room for user bugs where objects that are isolate-local are shared.

We could go instead the other direction:

• We establish a guarantee that native finalizers are always run
• They run even if the NativeFinalizer object was GCed
• Don't run them on isolate shutdown (but isolate group shutdown)

=> This would effectively align the NativeFinalizer behavior with that of the C API.
=> This would make it safe to allow sharing Finalizable & deeply immutable objects across isolates.

So the main question is: Do we really care about early reclamation of isolate-local resources on unexpected isolate shutdown? This is only important if isolates are short lived and native resources aren't eagerly freed up by user code (or e.g. isolates get often killed) and we cannot wait for the GC to eventually run them a bit later.

/cc @mraleph wdyt?

(**) This assumption only makes sense for synchronous code. In asynchronous code one can easily have

foo() async {
  final obj = Foo(field);
  finalizer.attach(foo, ...., () => ...);
  final field = obj.field;
  await <...>
  // ^^^^
  //   - takes long time
  //   - may cause GC
  //   - may collect `obj` (as it's not used anymore & it's not `Finalizable)
  //   - GC may enqueue finalizer,
  //   - may run finalizer
  useField(field);  // <-- use field after finalizer has run
}

[dcharkes]

=> This would effectively align the NativeFinalizer behavior with that of the C API.

That's not entirely true, canceling finalizers from another isolate doesn't work, from a previous discussion of ours:

• However, detach would not work shared. Trying to detach from a non-shared static nativeFinalizer would lead to a no-op on another isolate. Because it would be a different native finalizer (that happens to have the same callback address.)

• [doc/ffi] NativeFinalizer shutdown guarantees #55511 (comment)

Do we really care about early reclamation of isolate-local resources on unexpected isolate shutdown?

I don't know of any such use cases. So, I like the idea of trying to make NativeFinalizer shared always instead of introducing a SharedNativeFinalizer later, from another previous discussion of ours:

• [vm/ffi] Introduce pragma's 'vm:shareable' and 'vm:shared' and make NativeFinalizers live in an isolate group instead of isolate. #55062

I believe we could make the existing NativeFinalizer sharable (and thus make detach work from any isolate) by making it's backing store for entries (a map), concurrency safe. (We would need to do the same work for introducing a SharedNativeFinalizer later anyway.)

[mkustermann]

That's not entirely true, canceling finalizers from another isolate doesn't work, from a previous discussion of ours:

The code may not care about detachment at all, it may only detach on the main isolate anyway, helper isolates could ask main isolate to detach or it may (in future) use static shared NativeFinalizer = ... (in which case one gets the same semantics as the C api)

Trying to detach from a non-shared static nativeFinalizer would lead to a no-op

Wouldn't it throw a nice exception: "You cannot detach object foo from this finalizer as it was not attached" ?

I believe we could make the existing NativeFinalizer sharable (and thus make detach work from any isolate) by making it's backing store for entries (a map), concurrency safe. (We would need to do the same work for introducing a SharedNativeFinalizer later anyway.)

Making NativeFinalizer shared (i.e. make them work in upcoming shared multithreading shared isolates) isn't necessary for this.

What is necessary is committing to the semantic changes of NativeFinalizer: Make them always run (even if NativeFinalizer object is GCed), make them not run on isolate shutdown. As those two semantic changes will prevent use-after-free errors.

[dcharkes]

Right, if you cannot send the NativeFinalizer to another isolate, you cannot try to detach there. 👍

➕ For changing NativeFinalizers to be run on isolate group shutdown, and not GCable themselves if they still have attachments.

Wouldn't it throw a nice exception: "You cannot detach object foo from this finalizer as it was not attached" ?

It does not. #56632