From ef30b3cc99d91e15207027fd604dded2e8975729 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Tue, 25 Nov 2025 02:50:47 -0800 Subject: [PATCH] [vm/ffi] Test and Document `NativeFinalizer` Dart API limitations Explicitly allow calling `Dart_DeletePersistentHandle` and `Dart_DeleteWeakPersistentHandle` from `NativeFinalizers`. And exercise this behavior in tests. TEST=tests/ffi/vmspecific_native_finalizer_api_calls_test.dart TEST=tests/ffi/vmspecific_native_finalizer_isolates_test.dart Closes: https://github.com/dart-lang/sdk/issues/62076 CoreLibraryReviewExempt: VM-only. Doc-only. Change-Id: I4915092e4b13cc55d0d48e7977149fb32059b854 Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-x64-try,vm-dyn-linux-debug-x64-try,vm-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-tsan-linux-release-x64-try,vm-msan-linux-release-x64-try,vm-asan-linux-release-x64-try,vm-ffi-mac-debug-simarm64_arm64-try,vm-win-debug-x64-try,vm-reload-linux-debug-x64-try,vm-gcc-linux-x64-try,vm-appjit-linux-debug-x64-try,vm-aot-linux-release-x64-try,vm-aot-dyn-linux-debug-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464180 Commit-Queue: Daco Harkes Reviewed-by: Slava Egorov Reviewed-by: Liam Appelbe --- .../ffi_test/ffi_test_functions_vmspecific.cc | 20 +++++ runtime/docs/gc.md | 2 + sdk/lib/ffi/native_finalizer.dart | 16 ++-- ...cific_native_finalizer_api_calls_test.dart | 80 +++++++++++++++++++ ...ecific_native_finalizer_isolates_test.dart | 80 +++++++++++++++++++ 5 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 tests/ffi/vmspecific_native_finalizer_api_calls_test.dart diff --git a/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc b/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc index 252580250df..6bdc16d1a39 100644 --- a/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc +++ b/runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc @@ -1170,6 +1170,26 @@ DART_EXPORT void SetArgumentTo42(void* token) { *reinterpret_cast(token) = 42; } +DART_EXPORT Dart_PersistentHandle NewPersistentHandle(Dart_Handle object) { + return Dart_NewPersistentHandle(object); +} + +DART_EXPORT void DeletePersistentHandleFinalizer(void* handle) { + printf("C: Finalizer deleting persistent handle %p\n", handle); + Dart_DeletePersistentHandle(reinterpret_cast(handle)); +} + +DART_EXPORT Dart_WeakPersistentHandle +NewWeakPersistentHandle(Dart_Handle object) { + return Dart_NewWeakPersistentHandle(object, nullptr, 0, [](void*, void*) {}); +} + +DART_EXPORT void DeleteWeakPersistentHandleFinalizer(void* handle) { + printf("C: Finalizer deleting weak persistent handle %p\n", handle); + Dart_DeleteWeakPersistentHandle( + reinterpret_cast(handle)); +} + //////////////////////////////////////////////////////////////////////////////// // Functions for testing @Native. diff --git a/runtime/docs/gc.md b/runtime/docs/gc.md index 479d9b83a48..99859b1fbab 100644 --- a/runtime/docs/gc.md +++ b/runtime/docs/gc.md @@ -45,6 +45,8 @@ Note that a mutator can be at a safepoint without being suspended. It might be p Because a safepoint operation excludes execution of Dart code, it is sometimes used for non-GC tasks that requires only this property. For example, when a background compilation has completed and wants to install its result, it uses a safepoint operation to ensure no Dart execution sees the intermediate states during installation. +The state of each thread is represented using `Thread::ExecutionState` enum. A thread is considered to be at a safepoint if its state is `kThreadInNative` (executing external native code) or `kThreadInBlockedState` (blocked on a lock). Conversely, a thread in `kThreadInVM` (executing C++ VM code) or `kThreadInGenerated` (executing compiled Dart code) is not at a safepoint. The VM relies on threads transitioning between these states carefully to ensure that a safepoint operation can begin only when all threads are in a safe state. + ## Scavenge See [Cheney's algorithm](https://en.wikipedia.org/wiki/Cheney's_algorithm). diff --git a/sdk/lib/ffi/native_finalizer.dart b/sdk/lib/ffi/native_finalizer.dart index 2385f25d56a..620557c78ea 100644 --- a/sdk/lib/ffi/native_finalizer.dart +++ b/sdk/lib/ffi/native_finalizer.dart @@ -347,13 +347,17 @@ abstract final class NativeFinalizer { /// Creates a finalizer with the given finalization callback. /// /// The [callback] must be a native function which can be executed outside of - /// a Dart isolate. This means that passing an FFI trampoline (a function - /// pointer obtained via [Pointer.fromFunction]) is not supported. + /// a Dart isolate. This also means that passing an FFI trampoline (a function + /// a function pointer obtained via [Pointer.fromFunction]) is not supported. /// - /// The [callback] might be invoked on an arbitrary thread and not necessary - /// on the same thread that created [NativeFinalizer]. - // TODO(https://dartbug.com/47778): Implement isolate independent code and - // update the above comment. + /// The callback is not allowed to re-enter the Dart VM via Dart C APIs, with + /// two exceptions: it is allowed to call `Dart_DeletePersistentHandle` and + /// `Dart_DeleteWeakPersistentHandle`. Calling any other Dart C API function + /// results in undefined behavior, which means it can cause anything from + /// crashes and deadlocks to silent memory corruptions. + /// + /// The [callback] might be invoked on an arbitrary thread. It will have a + /// current isolate group but will not have a current isolate. external factory NativeFinalizer(Pointer callback); /// Attaches this finalizer to [value]. diff --git a/tests/ffi/vmspecific_native_finalizer_api_calls_test.dart b/tests/ffi/vmspecific_native_finalizer_api_calls_test.dart new file mode 100644 index 00000000000..ed63b2496bc --- /dev/null +++ b/tests/ffi/vmspecific_native_finalizer_api_calls_test.dart @@ -0,0 +1,80 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +// +// SharedObjects=ffi_test_functions +// +// VMOptions=--trace-finalizers + +import 'dart:ffi'; + +import 'dylib_utils.dart'; +import 'ffi_test_helpers.dart'; + +void main() { + testDeletePersistentHandleFromFinalizer(); + testDeleteWeakPersistentHandleFromFinalizer(); + print('Done.'); +} + +final ffiTestFunctions = dlopenPlatformSpecific('ffi_test_functions'); + +// C functions. +final newPersistentHandle = ffiTestFunctions + .lookupFunction< + Pointer Function(Handle), + Pointer Function(Object) + >('NewPersistentHandle'); + +final deletePersistentHandleFinalizer = ffiTestFunctions + .lookup)>>( + 'DeletePersistentHandleFinalizer', + ); + +final newWeakPersistentHandle = ffiTestFunctions + .lookupFunction< + Pointer Function(Handle), + Pointer Function(Object) + >('NewWeakPersistentHandle'); + +final deleteWeakPersistentHandleFinalizer = ffiTestFunctions + .lookup)>>( + 'DeleteWeakPersistentHandleFinalizer', + ); + +class MyClass { + final int a; + MyClass(this.a); +} + +class MyFinalizable implements Finalizable { + final int a; + MyFinalizable(this.a); +} + +void testDeletePersistentHandleFromFinalizer() { + final finalizer = NativeFinalizer(deletePersistentHandleFinalizer); + final objectToKeepAlive = MyClass(1); + final persistentHandle = newPersistentHandle(objectToKeepAlive); + var gcObject = MyFinalizable(2); + finalizer.attach(gcObject, persistentHandle, detach: gcObject); + + // Lose the object, the finalizer should run. + gcObject = MyFinalizable(3); + doGC(); + // Test passes if it does not crash. +} + +void testDeleteWeakPersistentHandleFromFinalizer() { + final finalizer = NativeFinalizer(deleteWeakPersistentHandleFinalizer); + final objectToKeepAlive = MyClass(1); + final weakHandle = newWeakPersistentHandle(objectToKeepAlive); + + var gcObject = MyFinalizable(2); + finalizer.attach(gcObject, weakHandle, detach: gcObject); + + // Lose the object, the finalizer should run. + gcObject = MyFinalizable(3); + doGC(); + // Test passes if it does not crash. +} diff --git a/tests/ffi/vmspecific_native_finalizer_isolates_test.dart b/tests/ffi/vmspecific_native_finalizer_isolates_test.dart index aa27420327f..3ecf2f8bf90 100644 --- a/tests/ffi/vmspecific_native_finalizer_isolates_test.dart +++ b/tests/ffi/vmspecific_native_finalizer_isolates_test.dart @@ -20,6 +20,8 @@ void main() async { await testSendAndExitFinalizable(); await testSendAndExitFinalizer(); await testFinalizerRunsOnIsolateShutdown(); + await testDeletePersistentHandleOnIsolateShutdown(); + await testDeleteWeakPersistentHandleOnIsolateShutdown(); print('End of test, shutting down.'); } @@ -48,6 +50,60 @@ Future testFinalizerRunsOnIsolateShutdown() async { }); } +void runIsolateDeletePersistentHandleOnShutdown(int objectAddress) { + final finalizer = NativeFinalizer(deletePersistentHandleFinalizer); + final persistentHandle = Pointer.fromAddress(objectAddress); + final objectToFinalize = MyFinalizableObject(); + finalizer.attach( + objectToFinalize, + persistentHandle, + detach: objectToFinalize, + ); +} + +Future testDeletePersistentHandleOnIsolateShutdown() async { + final objectToKeepAlive = + Object(); // Keep a strong reference to ensure the handle stays alive + final persistentHandle = newPersistentHandle(objectToKeepAlive); + final portExitMessage = ReceivePort(); + await Isolate.spawn( + runIsolateDeletePersistentHandleOnShutdown, + persistentHandle.address, + onExit: portExitMessage.sendPort, + ); + await portExitMessage.first; // Wait for the isolate to exit + doGC(); + + // The test passes if no crash occurred. We can't directly verify the handle deletion + // from outside the isolate, but the lack of a crash indicates success. + print('Persistent handle deletion test on shutdown completed cleanly.'); +} + +void runIsolateDeleteWeakPersistentHandleOnShutdown(int objectAddress) { + final finalizer = NativeFinalizer(deleteWeakPersistentHandleFinalizer); + final weakHandle = Pointer.fromAddress(objectAddress); + final objectToFinalize = MyFinalizableObject(); + finalizer.attach(objectToFinalize, weakHandle, detach: objectToFinalize); + print('Isolate for weak persistent handle deletion done.'); +} + +Future testDeleteWeakPersistentHandleOnIsolateShutdown() async { + final objectToKeepAlive = + Object(); // Keep a strong reference to ensure the handle stays alive + final weakHandle = newWeakPersistentHandle(objectToKeepAlive); + final portExitMessage = ReceivePort(); + await Isolate.spawn( + runIsolateDeleteWeakPersistentHandleOnShutdown, + weakHandle.address, + onExit: portExitMessage.sendPort, + ); + await portExitMessage.first; // Wait for the isolate to exit + doGC(); + + // The test passes if no crash occurred. + print('Weak persistent handle deletion test on shutdown completed cleanly.'); +} + Future testSendAndExitFinalizable() async { final receivePort = ReceivePort(); await Isolate.spawn((SendPort sendPort) { @@ -75,3 +131,27 @@ Future testSendAndExitFinalizer() async { final result = await receivePort.first; Expect.contains("Invalid argument: is unsendable", result); } + +final newPersistentHandle = ffiTestFunctions + .lookupFunction< + Pointer Function(Handle), + Pointer Function(Object) + >('NewPersistentHandle'); + +final deletePersistentHandleFinalizer = ffiTestFunctions + .lookup)>>( + 'DeletePersistentHandleFinalizer', + ); + +final newWeakPersistentHandle = ffiTestFunctions + .lookupFunction< + Pointer Function(Handle), + Pointer Function(Object) + >('NewWeakPersistentHandle'); + +final deleteWeakPersistentHandleFinalizer = ffiTestFunctions + .lookup)>>( + 'DeleteWeakPersistentHandleFinalizer', + ); + +class MyFinalizableObject implements Finalizable {}