Merge pull request fzyzcjy#2653 from alexlapa/post-cobj-double-free

Fix Dart_PostCObject double free

Author: fzyzcjy

frb_dart/lib/src/ffigen_generated/intermediate/frb_rust.h

@@ -72,3 +72,10 @@ void frb_free_wire_sync_rust2dart_dco(WireSyncRust2DartDco value);
  * This function should never be called manually.
  */
 void frb_free_wire_sync_rust2dart_sse(struct WireSyncRust2DartSse value);
+
+/**
+ * # Safety
+ *
+ * This function should never be called manually.
+ */
+void (*frb_create_shutdown_callback(void))(void*);

frb_dart/lib/src/ffigen_generated/multi_package.dart

@@ -481,6 +481,25 @@ class MultiPackageCBinding {
   late final _frb_free_wire_sync_rust2dart_sse =
       _frb_free_wire_sync_rust2dart_ssePtr
           .asFunction<void Function(WireSyncRust2DartSse)>();
+
+  /// # Safety
+  ///
+  /// This function should never be called manually.
+  ffi.Pointer<ffi.NativeFunction<ffi.Void Function(ffi.Pointer<ffi.Void>)>>
+      frb_create_shutdown_callback() {
+    return _frb_create_shutdown_callback();
+  }
+
+  late final _frb_create_shutdown_callbackPtr = _lookup<
+      ffi.NativeFunction<
+          ffi.Pointer<
+                  ffi.NativeFunction<ffi.Void Function(ffi.Pointer<ffi.Void>)>>
+              Function()>>('frb_create_shutdown_callback');
+  late final _frb_create_shutdown_callback =
+      _frb_create_shutdown_callbackPtr.asFunction<
+          ffi.Pointer<
+                  ffi.NativeFunction<ffi.Void Function(ffi.Pointer<ffi.Void>)>>
+              Function()>();
 }
 
 /// A Dart_CObject is used for representing Dart objects as native C

frb_dart/lib/src/generalized_frb_rust_binding/_io.dart

@@ -10,6 +10,13 @@ class GeneralizedFrbRustBinding {
   final MultiPackageCBinding _binding;
   final String _externalLibraryDebugInfo;
 
+  /// Notifies Rust side of an isolate group shutdown. Initialized in
+  /// [initShutdownWatcher].
+  ///
+  /// It is static and initialized only once since there supposed to be only
+  /// one per isolate.
+  static _ShutdownWatcher? _shutdownWatcher;
+
   /// {@macro flutter_rust_bridge.only_for_generated_code}
   GeneralizedFrbRustBinding(ExternalLibrary externalLibrary)
       : _binding = MultiPackageCBinding(externalLibrary.ffiDynamicLibrary),
@@ -25,6 +32,12 @@ class GeneralizedFrbRustBinding {
     }
   }
 
+  /// {@macro flutter_rust_bridge.only_for_generated_code}
+  void initShutdownWatcher() {
+    _shutdownWatcher ??=
+        _ShutdownWatcher(_binding.frb_create_shutdown_callback());
+  }
+
   /// {@macro flutter_rust_bridge.only_for_generated_code}
   void initFrbDartApiDl() =>
       _binding.frb_init_frb_dart_api_dl(ffi.NativeApi.initializeApiDLData);
@@ -120,3 +133,13 @@ class GeneralizedFrbRustBinding {
     }
   }
 }
+
+/// {@macro flutter_rust_bridge.only_for_generated_code}
+final class _ShutdownWatcher implements ffi.Finalizable {
+  final ffi.NativeFinalizer _finalizer;
+
+  _ShutdownWatcher(ffi.Pointer<ffi.NativeFinalizerFunction> callback)
+      : _finalizer = ffi.NativeFinalizer(callback) {
+    _finalizer.attach(this, ffi.Pointer.fromAddress(0));
+  }
+}

frb_dart/lib/src/generalized_frb_rust_binding/_web.dart

@@ -16,6 +16,9 @@ class GeneralizedFrbRustBinding {
   /// {@macro flutter_rust_bridge.only_for_generated_code}
   void initFrbDartApiDl() {}
 
+  /// {@macro flutter_rust_bridge.only_for_generated_code}
+  void initShutdownWatcher() {}
+
   /// {@macro flutter_rust_bridge.only_for_generated_code}
   void pdeFfiDispatcherPrimary({
     required int funcId,

frb_dart/lib/src/main_components/entrypoint.dart

@@ -167,6 +167,7 @@ class _EntrypointState<A extends BaseApi> {
   }) {
     _setUpRustToDartCommunication(generalizedFrbRustBinding);
     _initializeApiDlData(generalizedFrbRustBinding);
+    _initializeShutdownWatcher(generalizedFrbRustBinding);
   }
 
   void dispose() {
@@ -199,3 +200,8 @@ void _setUpRustToDartCommunication(
 void _initializeApiDlData(GeneralizedFrbRustBinding generalizedFrbRustBinding) {
   generalizedFrbRustBinding.initFrbDartApiDl();
 }
+
+void _initializeShutdownWatcher(
+    GeneralizedFrbRustBinding generalizedFrbRustBinding) {
+  generalizedFrbRustBinding.initShutdownWatcher();
+}

frb_rust/src/ffi_binding/io.rs

@@ -1,7 +1,14 @@
 use crate::codec::dco::Rust2DartMessageDco;
 use crate::codec::sse::Rust2DartMessageSse;
 use crate::codec::Rust2DartMessageTrait;
+use crate::misc::atomic::AtomicU64;
 use crate::platform_types::{WireSyncRust2DartDco, WireSyncRust2DartSse};
+use allo_isolate::{
+    ffi::{DartCObject, DartPort},
+    store_dart_post_cobject,
+};
+use std::ffi::c_void;
+use std::sync::atomic::Ordering;
 
 /// # Safety
 ///
@@ -29,3 +36,44 @@ pub unsafe extern "C" fn frb_free_wire_sync_rust2dart_dco(value: WireSyncRust2Da
 pub unsafe extern "C" fn frb_free_wire_sync_rust2dart_sse(value: WireSyncRust2DartSse) {
     let _ = Rust2DartMessageSse::from_raw_wire_sync(value);
 }
+
+/// # Safety
+///
+/// This function should never be called manually.
+#[no_mangle]
+pub unsafe extern "C" fn frb_create_shutdown_callback() -> unsafe extern "C" fn(*mut c_void) {
+    /// Counter for how many active shutdown callbacks there are.
+    ///
+    /// It is incremented when `frb_shutdown_callback` is returned and
+    /// decremented when it is called.
+    static ISOLATES_NUM: AtomicU64 = AtomicU64::new(0);
+
+    _ = ISOLATES_NUM.fetch_add(1, Ordering::SeqCst);
+
+    /// Called by Dart's `NativeFinalizer` on isolate group shutdown.
+    unsafe extern "C" fn frb_shutdown_callback(_: *mut c_void) {
+        unsafe extern "C" fn devnull(_: DartPort, _: *mut DartCObject) -> bool {
+            // Returning true is wrong since message is not enqueued and this
+            // might cause memory leaks. But since application is shutting down
+            // we don't really care and just want it to die silently without
+            // triggering any send errors.
+            true
+        }
+
+        let running = ISOLATES_NUM.fetch_sub(1, Ordering::SeqCst);
+
+        // If this is the last callback we assume that application is shutting
+        // down.
+        if running == 1 {
+            // So `Dart_PostCObject` won't do anything from now on. We need this
+            // cause once shutdown have started `Dart_Cleanup` might be called any
+            // moment from now and `Dart_PostCObject` can only be used before
+            // `Dart_Cleanup` has been called
+            // For more information refer to:
+            // https://github.com/dart-lang/native/issues/2079
+            store_dart_post_cobject(devnull);
+        };
+    }
+
+    frb_shutdown_callback
+}