dart-lang / site-www

Source for Dart website
https://dart.dev
Other
943 stars 683 forks source link

Add memory management tips to dart:ffi page #3193

Open johnpryan opened 3 years ago

johnpryan commented 3 years ago

Page: https://dart.dev/guides/libraries/c-interop

@dcharkes just fixed a memory leak in our samples (https://github.com/dart-lang/samples/pull/101). We should probably document this.

We could add some memory management tips, like:

void free_pointer(int *int_pointer)
{
    // Free native memory in C which was allocated in C.
    free(int_pointer);
}
// C free function - void free_pointer(int *int_pointer);
//
// Example of how to free pointers that were allocated in C.
typedef free_pointer_func = Void Function(Pointer<Int32> a);
typedef FreePointer = void Function(Pointer<Int32> a);
  // Free up allocated memory. This time in C, because it was allocated in C.
  final freePointerPointer =
      dylib.lookup<NativeFunction<free_pointer_func>>('free_pointer');
  final freePointer = freePointerPointer.asFunction<FreePointer>();
  freePointer(myPointer);

See also:

mattetti commented 2 years ago

I run into this issue using dart FFI with Go in c-shared mode. Everything works fine on mac where ffi's malloc.free does what's expected. But on windows, when trying to free the native memory allocated in cgo (via C.CString for instance) would result in a segfault.

As workaround was to follow the recommendation shared by @johnpryan and to define an exported function in Go that can be called from dart to free memory. Here is some more info in case it helps someone down the line.

In go, export a C function to free a specific pointer.

// Free native memory which we allocated in cgo such as en using C.CString.
// This is needed so the caller can safely release the memory allocated by our code.
//export free_pointer
func free_pointer(pointer unsafe.Pointer) {
    println("freeing pointer from cgo") // to prove it works
    C.free(pointer)
}

Then in your dart code, make the ffi function as John explained:

late final _cfreePointerPointer = _lookup<NativeFunction<Void Function(Pointer<Int32> a)>>('free_pointer')
      .asFunction<void Function(Pointer<Int32> a)>();

And define a dart function to wrap this call and convert the address into a pointer:

void _freeCPointer(int address) {
    _cfreePointerPointer(Pointer.fromAddress(address));
  }

Now let's pretend that our Go/cgo code exports a hello function returning a string/char*.

//export hello
function hello() *C.char {
  return C.CString("hello world!")
}

The allocated string won't be garbage collected by Go and therefore needs to be freed by the caller when done with the data. I expected malloc.free to work from dart but end ed up having issues as mentioned before. It therefore seems better to call back into Go to free the string.

final Pointer<Utf8> cString = _cHello();
final msg = cHello.toDartString();
_freeCPointer(cHello.address);
 print(msg);

Go should handle the freeing of the pointer and print the debug message we added there.

Jordan-Nelson commented 2 years ago

Is there a recommended way to track memory leaks using the Dart Dev tools?

If I create a simple program that allocates pointers in a loop without freeing them, I see the RSS (Resident Size Set) creep up in the Dart dev tools for the first minute or so, which I think would be expected given pointers are being allocated and never freed. However, it eventually starts to drop and eventually seems to mostly stabilize (with some ups and downs).

At first I thought maybe those pointers were eventually getting freed (or reused) somehow, but the amount of memory being used by the process (as shown in Activity Monitor on MacOS) doesn't drop off. It keeps climbing steadily.

Here is an example:

void main() {
  for (var i = 0; i < 1000000000; i++) {
    if (i % 1000 == 0) print('iteration: $i');
    i.toRadixString(2).toNativeUtf8();
    i.toRadixString(3).toNativeUtf8();
    i.toRadixString(4).toNativeUtf8();
  }
}

Here is the memory chart from Dart Dev tools:

Screen Shot 2022-04-11 at 3 21 38 PM

Note: The solid blue line is really a bunch of individual blue dots from garbage collection. They are just all on top of each other. You can see RSS goes up for a couple minutes until it hits ~1GB, but then drops pretty quickly. After that it goes up and down but never goes above 1GB again. However, after a few minutes of this running I see the process is using 5GB of memory (as shown in Activity Monitor on MacOS).

As far as I can tell, there isn't a way to view the memory leaks from native pointers using the dart dev tools. Is this a bug with the Dart Dev tools, or do they not include native memory?

parlough commented 2 years ago

Hi @Jordan-Nelson, I'm not too familiar with DevTools but with the new Finalizer API that Dart is introducing in 2.17, I believe that should allow for more exploration in tooling for finding memory leaks. As for not exposing native memory, I can't speak to that. You can likely find more information on the devtools repository or open a new issue there for further discussion.

You can read and comment on a new proposal/design doc for adding memory leak detection to DevTools here: https://flutter.dev/go/detect-memory-leaks

dcharkes commented 2 years ago

If I create a simple program that allocates pointers in a loop without freeing them, I see the RSS (Resident Size Set) creep up in the Dart dev tools for the first minute or so, which I think would be expected given pointers are being allocated and never freed. However, it eventually starts to drop and eventually seems to mostly stabilize (with some ups and downs).

The Dart Pointer objects that point to the allocated C memory (allocated by toNativeUtf8) will be freed by the Dart GC. The allocated C memory is not freed.

Is this a bug with the Dart Dev tools, or do they not include native memory?

Pointers do not include native memory.

In order to tell the Dart GC that you're holding on to native memory, one can use the NativeFinalizer [1]. The NativeFinalizer can also free the C memory when an object gets GCed.

The typical pattern would be to also provide an eager close method for when you'd want to free the native memory early [2]:

class MyNativeResource implements Finalizable {
  final Pointer<Void> pointer;

  bool _closed = false;

  MyNativeResource._(this.pointer, {int externalSize}) {
    print('pointer $pointer');
    freeFinalizer.attach(this, pointer,
        externalSize: externalSize, detach: this);
  }

  factory MyNativeResource() {
    const num = 1;
    const size = 16;
    final pointer = calloc(num, size);
    return MyNativeResource._(pointer, externalSize: size);
  }

  /// Eagerly stop using the native resource. Cancelling the finalizer.
  void close() {
    _closed = true;
    freeFinalizer.detach(this);
    free(pointer);
  }

  void useResource() {
    if (_closed) {
      throw UnsupportedError('The native resource has already been released');
    }
    print(pointer.address);
  }
}

Please note that NativeFinalizers cannot yet be use with WinHeapFree because it has a different signature. The work to support that is tracked in:

If you're only implemented in leak-detection, not in freeing the native memory, then you can use Finalizer instead to raise a warning on running the finalizer, and always canceling the finalizer on manually freeing the memory [3].

cc @polina-c is working on adding leak detection to our dev tools.

[1] https://dart-review.googlesource.com/c/sdk/+/236320/21/sdk/lib/ffi/native_finalizer.dart. [2] https://dart-review.googlesource.com/c/sdk/+/236320/21/tests/ffi_2/vmspecific_native_finalizer_test.dart# [3] https://github.com/polina-c/spikes/blob/master/dart-leaks/bin/gc.dart

Jordan-Nelson commented 2 years ago

cc @polina-c is working on adding leak detection to our dev tools.

Nice. I think that would be very useful. Looking forward to using it!

Jordan-Nelson commented 2 years ago

There are two things I am trying to determine. First, does the app/program have a memory leak. Second, what is causing the leak if one exists (which allocation is not freed up). I figured out a simple way to achieve the first goal. This might be obvious to others, but it wasn't to me, so I figured I would share.

  1. Create a script that exercises to code paths you want to test (ex: memory_profiling.dart)
  2. Compile that script to native code dart compile exe memory_profiling.dart -o out
  3. Use a leaks detection tool such as leaks on MacOS (part of developer Command Line Tools) to run the compiled program. Ex: leaks -atExit -- ./out. If you do have a leak, it won't be very helpful in determining where the leak is coming from as everything will be considered a "root" leak. But it should inform you if there is a leak.
mpl commented 1 year ago

@mattetti Hello, I've just stumbled into the same problem you had. Do you confirm that the approach you took is still the same you would recommend today?

mattetti commented 1 year ago

@mpl I haven't written any dart code in a while but yes, it's always recommended to free memory in the binary that allocated it. My guess is that the first time around, I got lucky on Mac where Go and Dart might have used the same toolchain allowing freeing from dart to work fine, but it's risky. I know it's cumbersome but even in C++ when providing a dll exporting a function allocating memory, I provide a matching exported function to clear the allocated memory.

mpl commented 1 year ago

@mpl I haven't written any dart code in a while but yes, it's always recommended to free memory in the binary that allocated it. My guess is that the first time around, I got lucky on Mac where Go and Dart might have used the same toolchain allowing freeing from dart to work fine, but it's risky. I know it's cumbersome but even in C++ when providing a dll exporting a function allocating memory, I provide a matching exported function to clear the allocated memory.

yeah, I experienced the same (that it was working from mac, but not from windows). thanks. Speaking of which, since you're doing cgo in dart for windows, I'm surprised you haven't hit: https://github.com/flutter/flutter/issues/129764 ? Or are you not doing any dynamic loading from flutter?

Levi-Lesches commented 1 year ago
  • Use calloc.free() when freeing memory allocated in Dart (via malloc, or toNativeUtf8 from package:ffi, for example)
  • Use a native function to free memory that was allocated in C:

As someone who didn't know this and had some crashes because of it (especially because it happened to work on WSL but not on Windows), it would be nice to have this documented. Maybe it can also be prevented in-code? FFI functions can return a Pointer, and the Allocator class takes Pointers as well. What if memory allocated in Dart was a subtype of Pointer? That way, FFI functions can continue to use Pointers, but Allocator.allocate and Allocator.free would use DartPointers. Would that break anything important? Because it seems like this would turn a few segfaults into compiler errors.

dcharkes commented 1 year ago
  • Use calloc.free() when freeing memory allocated in Dart (via malloc, or toNativeUtf8 from package:ffi, for example)
  • Use a native function to free memory that was allocated in C:

As someone who didn't know this and had some crashes because of it (especially because it happened to work on WSL but not on Windows), it would be nice to have this documented. Maybe it can also be prevented in-code? FFI functions can return a Pointer, and the Allocator class takes Pointers as well. What if memory allocated in Dart was a subtype of Pointer? That way, FFI functions can continue to use Pointers, but Allocator.allocate and Allocator.free would use DartPointers. Would that break anything important?

Unfortunately, pointers are not subclassable. In fact, we want to not have an object at runtime at all (just the memory address). See some info here: https://github.com/dart-lang/sdk/issues/49935.

Because it seems like this would turn a few segfaults into compiler errors.

I believe you mean it would turn segfaults into Dart runtime errors. I don't believe we could have compile-time errors catch this?

You could write your own abstraction wrapping Pointer and calloc.

(As a very off-topic tangent. With WASM-GC and multiple linear memories we also need to keep track of which memory a pointer belongs to. So we might need some kind of abstraction that has a pointer field at some point. cc @askeksa-google)

Levi-Lesches commented 1 year ago

I believe you mean it would turn segfaults into Dart runtime errors. I don't believe we could have compile-time errors catch this?

If Allocator (eg, calloc) were defined with

class DartPointer extends Pointer { } 
DartPointer allocate(...) { ... }
void free(DartPointer pointer) { ... }

then trying to pass a regular Pointer obtained by an FFI function would result in a type error.

In fact, we want to not have an object at runtime at all (just the memory address).

Since this would be a static check, could this still be possible by implementing DartPointer with a similar mechanism?

You could write your own abstraction wrapping Pointer and calloc.

Also true, but then I'd have to have the prescience to use my wrapper instead of calloc, in which case I'd just be careful to free the memory properly.


While on the topic of freeing with free() or a native call, I was wondering how to handle this in the case of NativeFinalizer. It needs a native function to call but when the memory is allocated on the Dart side, wouldn't it be dangerous to free it natively? But Finalizer isn't guaranteed to run, so it's possible to have native memory that isn't de-allocated? As per

If I create a simple program that allocates pointers in a loop without freeing them, I see the RSS (Resident Size Set) creep up in the Dart dev tools for the first minute or so, which I think would be expected given pointers are being allocated and never freed. However, it eventually starts to drop and eventually seems to mostly stabilize (with some ups and downs).

The Dart Pointer objects that point to the allocated C memory (allocated by toNativeUtf8) will be freed by the Dart GC. The allocated C memory is not freed.

My example is that I have a struct that can be allocated natively or on the Dart side. I have a wrapper class with a bool to keep track of which method was used, and in my dispose function, I free it appropriately. I have to be able to allocate on the Dart side because I need to pass a string, so even if I allocated the struct natively, I'd still need to allocate (and free) the string from Dart.

dcharkes commented 1 year ago

Since this would be a static check, could this still be possible by implementing DartPointer with a similar mechanism?

Maybe if you're only interested in static checks (e.g. you would give every source of Pointers a different type, rather than having a field inside DartPointer which keeps track of the allocator that was used and throwing on calling free with the wrong allocator), we could use inline classes / extension types. But that language feature is still in development.

While on the topic of freeing with free() or a native call, I was wondering how to handle this in the case of NativeFinalizer. It needs a native function to call but when the memory is allocated on the Dart side, wouldn't it be dangerous to free it natively?

package:ffi uses an FFI call to allocate the memory natively. calloc/malloc on non-Windows and CoTaskMemAlloc on Windows. https://github.com/dart-lang/ffi/blob/main/lib/src/allocation.dart So freeing with a native finalizer is the right thing to do. (We actually have an issue open to provide a finalizer on the allocators: https://github.com/dart-lang/native/issues/910)

Levi-Lesches commented 1 year ago

Maybe if you're only interested in static checks (e.g. you would give every source of Pointers a different type, rather than having a field inside DartPointer which keeps track of the allocator that was used and throwing on calling free with the wrong allocator), we could use inline classes / extension types. But that language feature is still in development.

Yeah this would be a great feature. Or maybe, in the meantime, it could be implemented in the same way Pointer currently is a static type but isn't re-ified, like https://github.com/dart-lang/sdk/issues/49935. Either way, should I open a new issue for it?

So freeing with a native finalizer is the right thing to do.

Okay now I'll admit I'm a little confused. From the top comment on this issue:

  • Use calloc.free() when freeing memory allocated in Dart (via malloc, or toNativeUtf8 from package:ffi, for example)
  • Use a native function to free memory that was allocated in C:

...but you're saying it's okay to natively free memory that was allocated from Dart? Then what is the point of keeping track of DartPointers vs regular Pointers?

askeksa-google commented 1 year ago

(As a very off-topic tangent. With WASM-GC and multiple linear memories we also need to keep track of which memory a pointer belongs to. So we might need some kind of abstraction that has a pointer field at some point. cc @askeksa-google)

In Wasm, the identity of the accessed memory is a static property of the accessing instruction. Thus, if we add support for accessing multiple Wasm memories through FFI pointers, this will necessarily be a static property of every access. So this will not add any runtime tracking requirements.

dcharkes commented 1 year ago

(As a very off-topic tangent. With WASM-GC and multiple linear memories we also need to keep track of which memory a pointer belongs to. So we might need some kind of abstraction that has a pointer field at some point. cc @askeksa-google)

In Wasm, the identity of the accessed memory is a static property of the accessing instruction. Thus, if we add support for accessing multiple Wasm memories through FFI pointers, this will necessarily be a static property of every access. So this will not add any runtime tracking requirements.

Ah right, so in that case a type argument per linear memory or an extension type per linear memory could work.

andycall commented 2 weeks ago

[calloc/malloc on non-Windows and CoTaskMemAlloc on Windows](package:ffi uses an FFI call to allocate the memory natively. calloc/malloc on non-Windows and CoTaskMemAlloc on Windows.)

One of my Dart FFI guides discussed the best practices for freeing native-allocated memory using Dart FFI (both for non-Windows and Windows) to ensure code safety.

Memory Management in Dart FFI

Levi-Lesches commented 1 week ago

@dcharkes

Maybe if you're only interested in static checks (e.g. you would give every source of Pointers a different type, rather than having a field inside DartPointer which keeps track of the allocator that was used and throwing on calling free with the wrong allocator), we could use inline classes / extension types. But that language feature is still in development.

extension type DartPointer<T extends NativeType>(Pointer<T> p) implements Pointer<T> { }
DartPointer<T> allocate<T extends NativeType>() => DartPointer(nullptr);
void free(DartPointer p) { }

👀