Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[doc/ffi] NativeFinalizer shutdown guarantees #55511

Open
dcharkes opened this issue Apr 19, 2024 · 2 comments
Open

[doc/ffi] NativeFinalizer shutdown guarantees #55511

dcharkes opened this issue Apr 19, 2024 · 2 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi P3 A lower priority bug or feature request type-documentation A request to add or improve documentation

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Apr 19, 2024

Currently, the documentation states that native finalizers will be run before isolate group shutdown.

The implementation currently guarantees that native finalizers are run on isolate shutdown.

Currently we under-promise, and over-deliver.
It might be useful to actually promise run on isolate shutdown. This way users using the isolate APIs can use native finalizers freeing up globals in native code and reuse that native code when a new isolate is started.

Thanks for reporting @mraleph!

@lrhn Any concerns about over-promising things?

Side note: Shared (isolate group) native finalizers

If in the future we introduce shared variables, we'd likely want to have a type of native finalizer that is only guaranteed to run on isolate group shutdown. For such shared native finalizers, we might need a different constructor:

// Adapted from the doc-comment example.

class Database implements Finalizable {
  shared static final _finalizer =
      NativeFinalizer.shared(_nativeDatabaseBindings.closeDatabaseAddress.cast());

If we were to introduce a native finalizer shared in an isolate group with a different constructor, we'd need to update the doc comments to reflect these two different shutdown guarantees for shared and non-shared native finalizers.

The current workaround without the shared keyword is to use Dart_FinalizableHandles from the dart_api.h, these are isolate group scoped instead of isolate scoped.

Edit, also see:

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P3 A lower priority bug or feature request area-documentation Prefer using 'type-documentation' and a specific area label. library-ffi labels Apr 19, 2024
@dcharkes dcharkes self-assigned this Apr 19, 2024
@lrhn
Copy link
Member

lrhn commented Apr 19, 2024

A native finalizer triggers on an object being unreachable.
Currently we assume that isolate shutdown makes every object in the isolate unreachable (and Isolate.exit is treated as a very efficient copy, I guess).

If a variable is shared, its value is shared, and will not become (globally) unreachable if one isolate shuts down. That seems reasonable.

If, big if, we introduce shared variables, and therefore shared values, we should perhaps have shared finalizers too. (If the finalizer object is not shared, it would die with the isolate? So it should be stored in a shared variable too?)

So a separate constructor, or even a separate class with its own documentation, which would answer the question here.

@dcharkes
Copy link
Contributor Author

Some notes from discussion @mkustermann and @mraleph:

Sharable and isolate local constructor:

  • If we introduce a Sharable interface for shared memory multi-threading, it would make sense to have NativeFinalizer.shared because we can check that only sharable objects get attached to that one and only non-sharable objects get attached to the local NativeFinalizer.
  • If we don't introduce a Sharable interface, then we'd likely only want a single shared constructor for NativeFinalizers. And every developer should only store NativeFinalizers in shared static variables.

NativeFinalizers being GCable:

  • If we change NativeFinalizer semantics to not be detached when the NativeFinalizer object itself is GCed, then basically all attach events to NativeFinalizers work like being attached to a shared NativeFinalizer.
  • 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.)

@devoncarew devoncarew added type-documentation A request to add or improve documentation and removed area-documentation Prefer using 'type-documentation' and a specific area label. labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi P3 A lower priority bug or feature request type-documentation A request to add or improve documentation
Projects
None yet
Development

No branches or pull requests

3 participants