dart-archive / ffi

Utilities for working with Foreign Function Interface (FFI) code
https://pub.dev/packages/ffi
BSD 3-Clause "New" or "Revised" License
155 stars 32 forks source link

Expose pointer to free from allocators #203

Closed mraleph closed 1 year ago

mraleph commented 1 year ago

This enables to write code like this:

final buf = malloc.allocate<Uint8>(len).asTypedList(len, finalizer: malloc.nativeFree);
dcharkes commented 1 year ago

Closes: https://github.com/dart-lang/native/issues/910

dcharkes commented 1 year ago

@mraleph How about the interplay of Arena and the two allocators you've added the functionality to?

Are we expecting devs to mix and match allocators?

  using((arena) {
    final str = "myString".toNativeUtf8(allocator: arena);
    final buf = malloc<Uint8>(len).asTypedList(len, finalizer: malloc.nativeFree);
  });

In general we have a question about combining finalizers and eager finalization (https://github.com/dart-lang/sdk/issues/49906), but in this specific case, the native finalizer can't be cancelled, so there's no reason to use the arena for the list backing the typed data. It feels weird, using two allocators, but, I think it's the right thing to do.

mraleph commented 1 year ago

Are we expecting devs to mix and match allocators?

I think this change specifically addresses non-arena based usage, if you have malloc allocated memory and want to make sure you automatically free it (e.g. when you pass data you get through FFI to a consumer which works with typed data).

dcharkes commented 1 year ago

cc @lrhn for a potentially better name for NativeFreeableAllocator.

lgtm from my side.

dcharkes commented 1 year ago

I have many ideas for how to completely redesign Allocator, but a more conservative design would be to expose Malloc and Calloc classes (final and with private constructors), and have them expose the native free function as a static getter.

Hm, I had the same thought in the comments (but with a marker interface in case someone would want to write generic code).

Then you can't abstract over the NativeFreeableAllocator and get to its nativeFree, but it's an abstraction that doen't work for alloc, and which feels glued on the Allocator API.

The use cases that we have don't require a shared super type.

final buf = malloc.allocate<Uint8>(len).asTypedList(len, finalizer: malloc.nativeFree);

Making a Malloc and Calloc type would work for these use cases.

mraleph commented 1 year ago

I have many ideas for how to completely redesign Allocator, but a more conservative design would be to expose Malloc and Calloc classes (final and with private constructors), and have them expose the native free function as a static getter.

I am not sure I like this proposal - it makes the code look strange: allocate and free are also effectively static, but they are still instance methods because Dart does not allow to abstract over static methods. So you end up with the code like this:

final x = malloc.allocate<Uint8>(len).asTypedList(len, finalizer: Malloc.nativeFree);

Notice there is malloc and Malloc in the same line which looks somewhat weird to me.

Or if you prefer, drop the NativeFreeableAllocator type, and expose CallocAllocator and MallocAllocator types that each have instance getters for nativeFree and nativeAlloc, but not shared supertype.

I can do this - I don't think any other major changes are worth it.

dcharkes commented 1 year ago

Thanks @mraleph !