dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.21k stars 1.57k forks source link

`Finalizable` and eager finalization. #49906

Open dcharkes opened 2 years ago

dcharkes commented 2 years ago

A common pattern with Finalizables is to have an eager finalization method that manually cleans up the native resource and cancels the native finalizer. If we were to define that pattern in an interface, we could consume that interface in other APIs. For example in Arena.

Consider the following code:

  testWidgets("Long.intValue() using JniObject", (tester) async {
    final longClass = jni.findJniClass("java/lang/Long");
    final longCtor = longClass.getConstructorID("(J)V");
    final long = longClass.newObject(longCtor, [176]);

    final intValue = long.callIntMethodByName("intValue", "()I", []);
    expect(intValue, equals(176));
  });

longClass and long would have a native finalizer attached, but we'd like to eagerly finalize them, rather than waiting for the GC. And we don't want to manually do eager finalization, we'd like to use Arena. We could add an extension method to Arena that enables managing objects of whatever type longClass and long are.

  testWidgets("Long.intValue() using JniObject", (tester) async {
    using((Arena arena) {
      final longClass = arena.manage(jni.findJniClass("java/lang/Long"));
      final longCtor = longClass.getConstructorID("(J)V");
      final long = arena.manage(longClass.newObject(longCtor, [176]));

      final intValue = long.callIntMethodByName("intValue", "()I", []);
      expect(intValue, equals(176));
    });
  });

However, that approach requires every class that is used in this pattern to implement an extension method on Arena with likely name clashes as a result.

If we would have an interface

abstract class EagerFinalizable implements Finalizable {
  /// Finalizes this object.
  ///
  /// All native resources are released, and this object can no longer be used afterwards.
  ///
  /// Implementers of this interface should ensure that all [NativeFinalizer]s attached to `this`
  /// are cancelled when this method returns.
  abstract void finalize();
}

then Arena could have a manage(EagerFinalizable finalizable) that registers the object to be eagerly finalized on the end of the arena.

Moreover, this interface would standardize a naming plethora with Finalizable resources, where one API designer calls it release and the other delete. (Though, this might be a downside, depending on context.)

Examples taken from https://github.com/dart-lang/native/issues/766

Thoughts @mraleph @lrhn @mkustermann @liamappelbe?

liamappelbe commented 2 years ago

Broadly SGTM. I'm wondering if the finalize method can have a default implementation in though, rather than being abstract and requiring the user to write it. Users could still override the method if they have a non-trivial use case. Consider the objective C bindings:

class _ObjCWrapper implements ffi.Finalizable {
  final ffi.Pointer<ObjCObject> _id;
  final AVFAudio _lib;
  bool _pendingRelease;

  _ObjCWrapper._(this._id, this._lib,
      {bool retain = false, bool release = false})
      : _pendingRelease = release {
    if (retain) {
      _lib._objc_retain(_id.cast());
    }
    if (release) {
      _lib._objc_releaseFinalizer2.attach(this, _id.cast(), detach: this);
    }
  }

  /// Releases the reference to the underlying ObjC object held by this wrapper.
  /// Throws a StateError if this wrapper doesn't currently hold a reference.
  void release() {
    if (_pendingRelease) {
      _pendingRelease = false;
      _lib._objc_release(_id.cast());
      _lib._objc_releaseFinalizer2.detach(this);
    } else {
      throw StateError(
          'Released an ObjC object that was unowned or already released.');
    }
  }

There's only 2 lines in release that might be non-generic: _lib._objc_release(_id.cast()); and _lib._objc_releaseFinalizer2.detach(this);. But both of those can be generic if the Finalizable knows about which NativeFinalizer it is attached to.

dcharkes commented 2 years ago

Good idea! I that would work if we always use this as a detachKey, and the NativeFinalizer should then also expose its callback (which it currently doesn't).

dcharkes commented 1 year ago

Notes from discussion with @polina-c:

Disposable is an interface which has the contract that the user must dispose the object. dispose is a Dart method. More info: https://flutter.dev/go/introduce-disposable

With EagerFinalizable, we do have the contract that the user may finalize. Instead, finalize would eagerly finalize, but if the user does not finalize the GC will take care of it. finalize likely will only call the single C function that the native finalizer would have called.

Theoretically, these two are not mutually exclusive, one could dispose some dart resources eagerly and then have the native finalizer release some native resources lazily (however, most likely you'd want to finalize in the dispose).