fzyzcjy / flutter_rust_bridge

Flutter/Dart <-> Rust binding generator, feature-rich, but seamless and simple.
https://fzyzcjy.github.io/flutter_rust_bridge/
MIT License
3.64k stars 256 forks source link

Support for Result<MyType, MyError> with exception throwing on Dart #582

Closed lattice0 closed 11 months ago

lattice0 commented 1 year ago

Close #533

Adds support for Rust's std::Result, where you can return either a value or an error.

Example


pub enum CustomError {
    Error0(String),
    Error1(u32),
}

pub fn return_err_custom_error() -> Result<u32, CustomError> {
    Err(CustomError::Error1(3))
}

Becomes something that can be used like this:


try {
      final r = await api.returnErrCustomError();
      print("received $r");
    } catch (e) {
      print('dart catch e: $e');
      expect(e, isA<CustomError>());
    }
}

Checklist

lattice0 commented 1 year ago

PS: Should fail until we get anyhow support for IntoDart as done here https://github.com/sunshine-protocol/allo-isolate/pull/23

fzyzcjy commented 1 year ago

Great job!

Since CI is not functioning before the upstream release, I just comment partially currently.

I am also wondering: Can we make this breaking change as little as possible? I want to cheat a little bit and do not bump the major version (i.e. release 2.0.0), since this feature though great is not as huge as a major version bump.

fzyzcjy commented 1 year ago

Well I mean:


class FrbException implements Exception {} // no fields

abstract class FrbBacktracedException implements FrbException {
  Backtrace get backtrace;
}

class FrbAnyhowException extends FrbBacktraceException {
  final String description; // or something else etc
  final String backtrace;

  FrbAnyhowException(this.description, this.backtrace);
}

@freezed
class CustomNestedError2 with _$CustomNestedError2 {
  // NOTE for data *without* field "backtrace", just implement FrbException
  @Implements<FrbException>()
  const factory CustomNestedError2.customNested2(
    String field0,
  ) = CustomNested2;

  // NOTE **only** implements FrbBacktracedException **if** there is **really** a "backtrace" field
  @Implements<FrbBacktracedException>()
  const factory CustomNestedError2.customNested2Number(
    int field0,
    String backtrace,
  ) = CustomNested2Number;
}
lattice0 commented 1 year ago

Why limit to errors with backtrace if we can always put a backtrace? Most people won't put they own backtraces so we should just put it something that is always received

fzyzcjy commented 1 year ago

Why limit to errors with backtrace if we can always put a backtrace? Most people won't put they own backtraces so we should just put it something that is always received

Sounds reasonable. So what about this:

@freezed
class CustomNestedError2 with _$CustomNestedError2 {
  @Implements<FrbBacktracedException>()
  const factory CustomNestedError2.customNested2(
    String field0,
    // NOTE *even if* the Rust struct does not have a backtrace field, we add one here
    String backtrace,
  ) = CustomNested2;

  @Implements<FrbBacktracedException>()
  const factory CustomNestedError2.customNested2Number(
    int field0,
    String backtrace,
  ) = CustomNested2Number;
}
lattice0 commented 1 year ago
@freezed
class CustomError with _$CustomError implements FrbException {
  @Implements<FrbBacktracedException>()
  const factory CustomError.error0(
    String field0,
    Backtrace backtrace,
  ) = Error0;
  @Implements<FrbBacktracedException>()
  const factory CustomError.error1(
    int field0,
    Backtrace backtrace,
  ) = Error1;
}

gives

The redirected constructor 'Error0 Function(String)' has incompatible parameters with 'CustomError Function(String, Backtrace)'.
Try redirecting to a different constructor.

on both Error0 and Error1.

I think I have to modify them:

abstract class Error0 implements CustomError, FrbBacktracedException {
  const factory Error0(final String field0) = _$Error0;

  String get field0;
  @JsonKey(ignore: true)
  _$$Error0CopyWith<_$Error0> get copyWith =>
      throw _privateConstructorUsedError;
}

but it's self generated into the .freezed.

lattice0 commented 1 year ago

Ops, I added manually and forgot to run build_runner. This error goes away.

lattice0 commented 1 year ago

At the freezed file:

class _$CustomNested1 implements CustomNested1 {

I get


Missing concrete implementation of 'getter FrbBacktracedException.backtrace'.
Try implementing the missing method, or make the class abstract

where

abstract class CustomNested1
    implements CustomNestedError1, FrbBacktracedException {
lattice0 commented 1 year ago

The problem is that

abstract class CustomNested1
    implements CustomNestedError1, FrbBacktracedException {

even though this implements FrbBacktracedException, it does not implement the getter for the backtrace, so

class _$CustomNested1 implements CustomNested1 {

will also not implement is, it's generated automatically as well.

fzyzcjy commented 1 year ago

Hmm what about looking at "Mixins and Interfaces for individual classes for union types" section of https://pub.dev/packages/freezed. Firstly run the example there successfully and then migrate to our case.

lattice0 commented 1 year ago

Found the problem:


@freezed
class CustomNestedError1 with _$CustomNestedError1 implements FrbException {
  @Implements<FrbBacktracedException>()
  const factory CustomNestedError1.customNested1(
    String field0,
  ) = CustomNested1;
  @Implements<FrbBacktracedException>()
  const factory CustomNestedError1.errorNested(
    CustomNestedError2 field0,
  ) = ErrorNested;
}

must be

@freezed
class CustomNestedError1 with _$CustomNestedError1 implements FrbException {
  @Implements<FrbBacktracedException>()
  const factory CustomNestedError1.customNested1(
    String field0,
    Backtrace? backtrace,/// <----- Has to have this otherwise it won't generate the implementation
  ) = CustomNested1;
  @Implements<FrbBacktracedException>()
  const factory CustomNestedError1.errorNested(
    CustomNestedError2 field0,
    Backtrace? backtrace, // <------
  ) = ErrorNested;
}

sorryyyy

lattice0 commented 1 year ago

Ok, now there is a way to pass a backtrace when the enum or struct is for an exception. However I still don't know how to catch a backtrace in Rust. I found one method but it uses nightly. The failed tests are because of pure_dart_multi etc, gotta run it later (cargo just ?)

fzyzcjy commented 1 year ago

However I still don't know how to catch a backtrace in Rust. I found one method but it uses nightly.

Firstly, for anyhow errors: There is a method, see https://docs.rs/anyhow/latest/anyhow/

Secondly, for thiserror, there is one similarly: https://docs.rs/thiserror/1.0.31/thiserror/

As for users' own home-made error, I am not sure whether we should (can?) get backtraces.

fzyzcjy commented 1 year ago

The failed tests are because of pure_dart_multi etc, gotta run it later (cargo just ?)

Sure, just run it before merging the PR and it is ok

fzyzcjy commented 1 year ago

Feel free to ping me (or click "request a review" to me) when the PR is ready (or near ready) for review!

lattice0 commented 1 year ago

I'm not sure how the backtrace would work.

Here:

let _ = panic::catch_unwind(move || {
            let wrap_info2 = wrap_info.clone();
            if let Err(error) = panic::catch_unwind(move || {
                let task = prepare();
                self.executor.execute(wrap_info2, task);
            }) {
                self.error_handler
                    .handle_error(wrap_info.port.unwrap(), Error::Panic(error), None);
            }
        });

the last argument to handle_error is the Option<&dyn Backtrace>. We're generic on the task return and etc but we're not 'catching' its return value on the executor.execute, we only catch the panic. But if we want to return the error from the execute then we cannot catch unwind the panics

fzyzcjy commented 1 year ago

Wait a bit - that is a panic, not a *Result::Error**.

  1. For Result::Err, it is strongly typed, and we should be able to transform it
  2. For panic, and stable Rust, have a look at last section of https://stackoverflow.com/questions/68556819/transform-backtrace-to-string-during-catch-unwind, which uses backtrace crate.
  3. To see a working sample, maybe have a look at https://github.com/getsentry/sentry-rust. I personally uses Sentry to report my code exceptions to the backend.
lattice0 commented 1 year ago

I added backtrace support but std::backtrace::Backtrace is nightly only, so all tests failed. What do you suggest?

fzyzcjy commented 1 year ago

but std::backtrace::Backtrace is nightly only, so all tests failed. What do you suggest?

https://crates.io/crates/backtrace

lattice0 commented 1 year ago

What is pure_dart_multi? I'm getting a duplicated

#[no_mangle]
pub extern "C" fn free_WireSyncReturnStruct(val: support::WireSyncReturnStruct) {
    unsafe {
        let _ = support::vec_from_leak_ptr(val.ptr, val.len);
    }
}

on it

fzyzcjy commented 1 year ago

What is pure_dart_multi? I'm getting a duplicated

Support multiple files. See http://cjycode.com/flutter_rust_bridge/feature/multiple_files.html implemented by @dbsxdbsx

lattice0 commented 1 year ago

cargo run --manifest-path=frb_codegen/Cargo.toml -- --rust-input frb_example/pure_dart_multi/rust/src/api_1.rs frb_example/pure_dart_multi/rust/src/api_2.rs --dart-output frb_example/pure_dart_multi/dart/lib/bridge_generated_api_1.dart frb_example/pure_dart_multi/dart/lib/bridge_generated_api_2.dart --class-name ApiClass1 ApiClass2 --rust-output frb_example/pure_dart_multi/rust/src/generated_api_1.rs frb_example/pure_dart_multi/rust/src/generated_api_2.rs --dart-format-line-length 120

generates a duplicated:


error: symbol `free_WireSyncReturnStruct` is already defined
  --> frb_example/pure_dart_multi/rust/src/generated_api_1.rs:92:1
   |
92 | pub extern "C" fn free_WireSyncReturnStruct(val: support::WireSyncReturnStruct) {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

is it a bug?

fzyzcjy commented 1 year ago

/cc @dbsxdbsx

fzyzcjy commented 1 year ago

Btw if it works on master branch, maybe check what code change causes this failure

The ci yaml files may also be helpful - check how they run the code generator

lattice0 commented 1 year ago

No problem it works, it had a bridge_generated.rs that shouldn't be there.

set -ex
cargo run --manifest-path=frb_codegen/Cargo.toml -- --rust-input frb_example/pure_dart/rust/src/api.rs --dart-output frb_example/pure_dart/dart/lib/bridge_generated.dart --dart-format-line-length 120
cargo run --manifest-path=frb_codegen/Cargo.toml -- --rust-input frb_example/with_flutter/rust/src/api.rs --dart-output frb_example/with_flutter/lib/bridge_generated.dart --dart-format-line-length 120
cargo run --manifest-path=frb_codegen/Cargo.toml -- --rust-input frb_example/pure_dart_multi/rust/src/api_1.rs frb_example/pure_dart_multi/rust/src/api_2.rs --dart-output frb_example/pure_dart_multi/dart/lib/bridge_generated_api_1.dart frb_example/pure_dart_multi/dart/lib/bridge_generated_api_2.dart --class-name ApiClass1 ApiClass2 --rust-output frb_example/pure_dart_multi/rust/src/generated_api_1.rs frb_example/pure_dart_multi/rust/src/generated_api_2.rs --dart-format-line-length 120
dart format --line-length 120 frb_example/pure_dart/dart/
dart format --line-length 120 frb_example/pure_dart_multi/dart/
cargo fmt
dart format --line-length 80 frb_dart
cargo build
cd ~/flutter_rust_bridge/frb_example/pure_dart/dart && dart run lib/main.dart ~/flutter_rust_bridge/target/debug/libflutter_rust_bridge_example.so

works. I just don't want to install cargo just because of the huge amount of dependencies just for a simple functionality.

fzyzcjy commented 1 year ago

I just don't want to install cargo just because of the huge amount of dependencies just for a simple functionality.

Maybe we can raise an issue there, asking them to provide a precompiled binary just like what @Desdaemon has done to this lib :)

lattice0 commented 1 year ago

The problem is blindly trusting all those deps on my computer just to run a cargo just file, so I made this alternative script. No problem for those who want to install cargo just, but I think that the deps are too much for simple functionality. A binary would also contain the dependencies in a certain way.

fzyzcjy commented 1 year ago

I see. (btw I realized they have packages https://github.com/casey/just#packages)

lattice0 commented 1 year ago

This is ready for review. I'll fix the CI but locally the main.dart tests pass.

also if you know why I get a Cargo.lock diff here I'll be really glad https://github.com/fzyzcjy/flutter_rust_bridge/runs/7588913378?check_suite_focus=true

fzyzcjy commented 1 year ago

Ok I will review it in a minute.

Maybe try to firstly pull from upstream?

dbsxdbsx commented 1 year ago

No problem it works, it had a bridge_generated.rs that shouldn't be there.

set -ex
cargo run --manifest-path=frb_codegen/Cargo.toml -- --rust-input frb_example/pure_dart/rust/src/api.rs --dart-output frb_example/pure_dart/dart/lib/bridge_generated.dart --dart-format-line-length 120
cargo run --manifest-path=frb_codegen/Cargo.toml -- --rust-input frb_example/with_flutter/rust/src/api.rs --dart-output frb_example/with_flutter/lib/bridge_generated.dart --dart-format-line-length 120
cargo run --manifest-path=frb_codegen/Cargo.toml -- --rust-input frb_example/pure_dart_multi/rust/src/api_1.rs frb_example/pure_dart_multi/rust/src/api_2.rs --dart-output frb_example/pure_dart_multi/dart/lib/bridge_generated_api_1.dart frb_example/pure_dart_multi/dart/lib/bridge_generated_api_2.dart --class-name ApiClass1 ApiClass2 --rust-output frb_example/pure_dart_multi/rust/src/generated_api_1.rs frb_example/pure_dart_multi/rust/src/generated_api_2.rs --dart-format-line-length 120
dart format --line-length 120 frb_example/pure_dart/dart/
dart format --line-length 120 frb_example/pure_dart_multi/dart/
cargo fmt
dart format --line-length 80 frb_dart
cargo build
cd ~/flutter_rust_bridge/frb_example/pure_dart/dart && dart run lib/main.dart ~/flutter_rust_bridge/target/debug/libflutter_rust_bridge_example.so

works. I just don't want to install cargo just because of the huge amount of dependencies just for simple functionality.

@fzyzcjy, sorry for the late reply, it seems that the multi issue is solved.

fzyzcjy commented 1 year ago

(Btw I guess you made some code change but did not push them? So I mark some conversations as un-resolved temporarily since I did not see code updates...)

fzyzcjy commented 1 year ago

@dbsxdbsx It's OK

lattice0 commented 1 year ago

This is done for review, finally anyhow is supported and both anyhow and panic generates exceptions on the dart side, as well as custom errors, with support for struct and enum errors.

fzyzcjy commented 1 year ago

Great job! This is a non-trivial PR that greatly enhances the error handling system :) I will review it soon (most probably today)

lattice0 commented 1 year ago

Anyone experienced in valgrind to give a little help? No idea of whats happening

fzyzcjy commented 1 year ago

Looking through the log, is this the problem:

2022-08-05T22:34:25.1472340Z ==5917== Thread 8 frb_executor:
2022-08-05T22:34:25.1472964Z ==5917== Syscall param statx(file_name) points to unaddressable byte(s)
2022-08-05T22:34:25.1473419Z ==5917==    at 0x4ADF88E: statx (statx.c:29)
2022-08-05T22:34:25.1473821Z ==5917==    by 0x5A88C51: statx (weak.rs:176)
2022-08-05T22:34:25.1474238Z ==5917==    by 0x5A88C51: std::sys::unix::fs::try_statx (fs.rs:159)
2022-08-05T22:34:25.1474647Z ==5917==    by 0x5A7FE09: file_attr (fs.rs:884)
2022-08-05T22:34:25.1475084Z ==5917==    by 0x5A7FE09: std::fs::File::metadata (fs.rs:510)
2022-08-05T22:34:25.1475530Z ==5917==    by 0x59A3871: backtrace::symbolize::gimli::mmap (gimli.rs:138)
2022-08-05T22:34:25.1476059Z ==5917==    by 0x59A5162: backtrace::symbolize::gimli::elf::<impl backtrace::symbolize::gimli::Mapping>::new (elf.rs:21)
2022-08-05T22:34:25.1476625Z ==5917==    by 0x59A3FB9: backtrace::symbolize::gimli::Cache::mapping_for_lib (gimli.rs:320)
2022-08-05T22:34:25.1477151Z ==5917==    by 0x59A4466: backtrace::symbolize::gimli::resolve::{{closure}} (gimli.rs:354)
2022-08-05T22:34:25.1477663Z ==5917==    by 0x59A3BA2: backtrace::symbolize::gimli::Cache::with_global (gimli.rs:266)
2022-08-05T22:34:25.1478163Z ==5917==    by 0x59A4303: backtrace::symbolize::gimli::resolve (gimli.rs:346)
2022-08-05T22:34:25.1478652Z ==5917==    by 0x5A1A57A: backtrace::symbolize::resolve_frame_unsynchronized (mod.rs:178)
2022-08-05T22:34:25.1479150Z ==5917==    by 0x5A1A37A: backtrace::symbolize::resolve_frame (mod.rs:105)
2022-08-05T22:34:25.1479639Z ==5917==    by 0x5A0C124: backtrace::capture::Backtrace::resolve (capture.rs:232)
2022-08-05T22:34:25.1480499Z ==5917==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
2022-08-05T22:34:25.1480870Z ==5917== 
2022-08-05T22:34:25.1482463Z ==5917== Syscall param statx(buf) points to unaddressable byte(s)
2022-08-05T22:34:25.1483127Z ==5917==    at 0x4ADF88E: statx (statx.c:29)
2022-08-05T22:34:25.1483513Z ==5917==    by 0x5A88C51: statx (weak.rs:176)
2022-08-05T22:34:25.1484284Z ==5917==    by 0x5A88C51: std::sys::unix::fs::try_statx (fs.rs:159)
2022-08-05T22:34:25.1484707Z ==5917==    by 0x5A7FE09: file_attr (fs.rs:884)
2022-08-05T22:34:25.1485121Z ==5917==    by 0x5A7FE09: std::fs::File::metadata (fs.rs:510)
2022-08-05T22:34:25.1485558Z ==5917==    by 0x59A3871: backtrace::symbolize::gimli::mmap (gimli.rs:138)
2022-08-05T22:34:25.1486074Z ==5917==    by 0x59A5162: backtrace::symbolize::gimli::elf::<impl backtrace::symbolize::gimli::Mapping>::new (elf.rs:21)
2022-08-05T22:34:25.1486629Z ==5917==    by 0x59A3FB9: backtrace::symbolize::gimli::Cache::mapping_for_lib (gimli.rs:320)
2022-08-05T22:34:25.1487143Z ==5917==    by 0x59A4466: backtrace::symbolize::gimli::resolve::{{closure}} (gimli.rs:354)
2022-08-05T22:34:25.1487640Z ==5917==    by 0x59A3BA2: backtrace::symbolize::gimli::Cache::with_global (gimli.rs:266)
2022-08-05T22:34:25.1488107Z ==5917==    by 0x59A4303: backtrace::symbolize::gimli::resolve (gimli.rs:346)
2022-08-05T22:34:25.1488605Z ==5917==    by 0x5A1A57A: backtrace::symbolize::resolve_frame_unsynchronized (mod.rs:178)
2022-08-05T22:34:25.1489104Z ==5917==    by 0x5A1A37A: backtrace::symbolize::resolve_frame (mod.rs:105)
2022-08-05T22:34:25.1489587Z ==5917==    by 0x5A0C124: backtrace::capture::Backtrace::resolve (capture.rs:232)
2022-08-05T22:34:25.1490214Z ==5917==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
2022-08-05T22:34:25.1490593Z ==5917== 

Maybe comment out the specific test, and see whether errors disappear. Then we know which one test errors.

Btw ignore Mismatched free() / delete / delete [] which comes from Dart vm's odd behavior

lattice0 commented 1 year ago

Looks like the error happens on the custom error return.

Example:

  test('Throw CustomError', () async {
    expect(() async => await api.returnErrCustomError(), throwsA(isA<CustomError>()));
  });
  CustomError _wire2api_custom_error(dynamic raw) {
  switch (raw[0]) {
    case 0:
      return Error0(
        e: _wire2api_String(raw[1]),
        backtrace: _wire2api_String(raw[2]),
      );
    case 1:
      return Error1(
        e: _wire2api_u32(raw[1]),
        backtrace: _wire2api_String(raw[2]),
      );
    default:
      throw Exception("unreachable");
  }
}
impl support::IntoDart for CustomError {
    fn into_dart(self) -> support::DartCObject {
        match self {
            Self::Error0 { e, backtrace } => {
                vec![0.into_dart(), e.into_dart(), backtrace.into_dart()]
            }
            Self::Error1 { e, backtrace } => {
                vec![1.into_dart(), e.into_dart(), backtrace.into_dart()]
            }
        }
        .into_dart()
    }
}
 /// Send a detailed error back to the specified port.
    pub fn error(&self, e: impl IntoDart) -> bool {
        self.isolate
            .post(vec![RUST2DART_ACTION_ERROR.into_dart(), e.into_dart()])
    }

I don't see anything wrong in the amount of stuff passed

fzyzcjy commented 1 year ago

What exact error does it say?

lattice0 commented 1 year ago

Well I don't know, I never understood valgrind outputs. Here's one example:

03:29 +58 -1: Throw CustomError nonstatic method

==5920== Thread 3 DartWorker:
==5920== Mismatched free() / delete / delete []
==5920==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==5920==    by 0x43DDD0: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x43DAFC: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x43CBA9: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x43CD41: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x4C78F4: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x508AFD5: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x5142D70: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x5149F2C: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x50AA593: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x50AA769: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x50AA184: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==  Address 0x8b26840 is 0 bytes inside a block of size 8 alloc'd
==5920==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==5920==    by 0x43DAE2: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x43CBA9: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x43C796: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x4C77CB: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x508AFD5: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x5142D93: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x50E4C3B: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x50F918B: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x515686D: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x5156538: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920==    by 0x5110D70: ??? (in /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/dart/main.exe)
==5920== 
backtrace:    0: flutter_rust_bridge_example::api::SomeStruct::non_static_return_err_custom_error
             at /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/rust/src/api.rs:779:24
   1: flutter_rust_bridge_example::bridge_generated::wire_non_static_return_err_custom_error__method__SomeStruct::{{closure}}::{{closure}}
             at /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_example/pure_dart/rust/src/bridge_generated.rs:1051:34
   2: <flutter_rust_bridge::handler::ThreadPoolExecutor<EH> as flutter_rust_bridge::handler::Executor>::execute::{{closure}}::{{closure}}
             at /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_rust/src/handler.rs:233:27
   3: std::panicking::try::do_call
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:492:40
   4: __rust_try
   5: std::panicking::try
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:456:19
   6: std::panic::catch_unwind
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panic.rs:137:14
   7: <flutter_rust_bridge::handler::ThreadPoolExecutor<EH> as flutter_rust_bridge::handler::Executor>::execute::{{closure}}
             at /home/runner/work/flutter_rust_bridge/flutter_rust_bridge/frb_rust/src/handler.rs:230:33
   8: <F as threadpool::FnBox>::call_box
             at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/threadpool-1.8.1/src/lib.rs:95:9
   9: threadpool::spawn_in_pool::{{closure}}
             at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/threadpool-1.8.1/src/lib.rs:769:17
  10: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/sys_common/backtrace.rs:122:18
  11: std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/thread/mod.rs:501:17
  12: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/panic/unwind_safe.rs:271:9
  13: std::panicking::try::do_call
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:492:40
  14: __rust_try
  15: std::panicking::try
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:456:19
  16: std::panic::catch_unwind
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panic.rs:137:14
  17: std::thread::Builder::spawn_unchecked_::{{closure}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/thread/mod.rs:500:30
  18: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/ops/function.rs:248:5
  19: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/alloc/src/boxed.rs:1872:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/alloc/src/boxed.rs:1872:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/sys/unix/thread.rs:108:17
  20: start_thread
             at /build/glibc-SzIz7B/glibc-2.31/nptl/pthread_create.c:477:8
  21: clone
             at /build/glibc-SzIz7B/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

but whenever I disable one test that throws an error, the other one fails, so it looks all of them are having memory leaks

fzyzcjy commented 1 year ago

I never understood valgrind outputs

Maybe spend a little bit of time learning it :)

It is almost a must to understand this well-known tool for a programmer playing with Rust

Here's one example:

Not sure whether that is the error causing it. Is the backtrace printed by our code deliberately? Or its error is not captured and the system prints it?

lattice0 commented 1 year ago

Ok, I think the error happens only when we have an error with backtrace.

This with backtrace fails:

https://github.com/fzyzcjy/flutter_rust_bridge/runs/7702357989?check_suite_focus=true

this error without backtrace does not fail:

https://github.com/fzyzcjy/flutter_rust_bridge/runs/7705833770?check_suite_focus=true

The one with backtrace gives:


==5865== Thread 9 frb_executor:
==5865== Syscall param statx(file_name) points to unaddressable byte(s)
==5865==    at 0x4ADF88E: statx (statx.c:29)
==5865==    by 0x5A88C51: statx (weak.rs:176)
==5865==    by 0x5A88C51: std::sys::unix::fs::try_statx (fs.rs:159)
==5865==    by 0x5A7FE09: file_attr (fs.rs:884)
==5865==    by 0x5A7FE09: std::fs::File::metadata (fs.rs:510)
==5865==    by 0x59A3871: backtrace::symbolize::gimli::mmap (gimli.rs:138)
==5865==    by 0x59A5162: backtrace::symbolize::gimli::elf::<impl backtrace::symbolize::gimli::Mapping>::new (elf.rs:21)
==5865==    by 0x59A3FB9: backtrace::symbolize::gimli::Cache::mapping_for_lib (gimli.rs:320)
==5865==    by 0x59A4466: backtrace::symbolize::gimli::resolve::{{closure}} (gimli.rs:354)
==5865==    by 0x59A3BA2: backtrace::symbolize::gimli::Cache::with_global (gimli.rs:266)
==5865==    by 0x59A4303: backtrace::symbolize::gimli::resolve (gimli.rs:346)
==5865==    by 0x5A1A57A: backtrace::symbolize::resolve_frame_unsynchronized (mod.rs:178)
==5865==    by 0x5A1A37A: backtrace::symbolize::resolve_frame (mod.rs:105)
==5865==    by 0x5A0C124: backtrace::capture::Backtrace::resolve (capture.rs:232)
==5865==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==5865== 
==5865== Syscall param statx(buf) points to unaddressable byte(s)
==5865==    at 0x4ADF88E: statx (statx.c:29)
==5865==    by 0x5A88C51: statx (weak.rs:176)
==5865==    by 0x5A88C51: std::sys::unix::fs::try_statx (fs.rs:159)
==5865==    by 0x5A7FE09: file_attr (fs.rs:884)
==5865==    by 0x5A7FE09: std::fs::File::metadata (fs.rs:510)
==5865==    by 0x59A3871: backtrace::symbolize::gimli::mmap (gimli.rs:138)
==5865==    by 0x59A5162: backtrace::symbolize::gimli::elf::<impl backtrace::symbolize::gimli::Mapping>::new (elf.rs:21)
==5865==    by 0x59A3FB9: backtrace::symbolize::gimli::Cache::mapping_for_lib (gimli.rs:320)
==5865==    by 0x59A4466: backtrace::symbolize::gimli::resolve::{{closure}} (gimli.rs:354)
==5865==    by 0x59A3BA2: backtrace::symbolize::gimli::Cache::with_global (gimli.rs:266)
==5865==    by 0x59A4303: backtrace::symbolize::gimli::resolve (gimli.rs:346)
==5865==    by 0x5A1A57A: backtrace::symbolize::resolve_frame_unsynchronized (mod.rs:178)
==5865==    by 0x5A1A37A: backtrace::symbolize::resolve_frame (mod.rs:105)
==5865==    by 0x5A0C124: backtrace::capture::Backtrace::resolve (capture.rs:232)
==5865==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

could it be a false positive? Or even a problem in the backtrace lib?

I reviewed the backtrace code to see if there's any possibility of memory leaks:

impl support::IntoDart for CustomError {
    fn into_dart(self) -> support::DartCObject {
        match self {
            Self::Error0 { e, backtrace } => {
                vec![0.into_dart(), e.into_dart(), backtrace.into_dart()]
            }
            Self::Error1 { e, backtrace } => {
                vec![1.into_dart(), e.into_dart(), backtrace.into_dart()]
            }
        }
        .into_dart()
    }
}

This is how the data is posted:

   pub fn post(&self, msg: impl IntoDart) -> bool {
        if let Some(func) = POST_COBJECT.load(Ordering::Relaxed) {
            unsafe {
                let boxed_msg = Box::new(msg.into_dart());
                let ptr = Box::into_raw(boxed_msg);
                // Send the message
                let result = func(self.port, ptr);
                // free the object
                let boxed_obj = Box::from_raw(ptr); // Data is freed here

Since backtrace is just a string, it should be freed, independently of what dart receives on the other side. The only way it could be wrong, maybe, on how dart receives the stuff:


  void wire_return_err_custom_error(
    int port_,
  ) {
    return _wire_return_err_custom_error(
      port_,
    );
  }

  late final _wire_return_err_custom_errorPtr =
      _lookup<ffi.NativeFunction<ffi.Void Function(ffi.Int64)>>('wire_return_err_custom_error');
  late final _wire_return_err_custom_error = _wire_return_err_custom_errorPtr.asFunction<void Function(int)>();

CustomError _wire2api_custom_error(dynamic raw) {
  switch (raw[0]) {
    case 0:
      return Error0(
        e: _wire2api_String(raw[1]),
        backtrace: _wire2api_String(raw[2]),
      );
    case 1:
      return Error1(
        e: _wire2api_u32(raw[1]),
        backtrace: _wire2api_String(raw[2]),
      );
    default:
      throw Exception("unreachable");
  }
}
#[no_mangle]
pub extern "C" fn wire_return_err_custom_error(port_: i64) {
    FLUTTER_RUST_BRIDGE_HANDLER.wrap(
        WrapInfo {
            debug_name: "return_err_custom_error",
            port: Some(port_),
            mode: FfiCallMode::Normal,
        },
        move || move |task_callback| return_err_custom_error(),
    )
}

but as far as I understood, dart is free to not use anything posted from the Rust side, as it knows what was posted so can delete it later.

fzyzcjy commented 1 year ago

Or even a problem in the backtrace lib?

I think so. backtrace, according to its official doc, says it is naturally unsafe. https://docs.rs/backtrace/0.3.66/backtrace/#backtrace-accuracy ; also see https://github.com/rust-lang/backtrace-rs/issues/441.

Indeed, looking at the stack trace of the error log, we see it is gimli's problem, not ours. Not our memory leak, etc.

So, maybe we should file an issue at backtrace crate https://github.com/rust-lang/backtrace-rs/issues , showing the error stack to them, and listen what they think - is it a bug in backtrace, or a feature there?

Or, another possibility is here: https://github.com/rust-lang/rust/issues/68979. In that issue, "0" is in syscall (same as our case) and valgrind is unhappy. The result is:

Try valgrind master. It was fixed there.

Maybe we can try that as well

lattice0 commented 1 year ago

newest valgrind: https://github.com/fzyzcjy/flutter_rust_bridge/runs/7708296830?check_suite_focus=true

maybe a valgrind bug for when the what was an error before happens?

lattice0 commented 1 year ago

forgot to post the error:


00:10 +44: Throw CustomError

02:41 +44 -1: Throw CustomError [E]

  TimeoutException after 0:00:30.000000: Test timed out after 30 seconds. See https://pub.dev/packages/test#timeouts

  package:test_api/src/backend/invoker.dart 334  Invoker._handleError.<fn>
  dart:async/zone.dart 1418                      _rootRun
  dart:async/zone.dart 1328                      _CustomZone.run
  package:test_api/src/backend/invoker.dart 332  Invoker._handleError
  package:test_api/src/backend/invoker.dart 288  Invoker.heartbeat.<fn>.<fn>
  dart:async/zone.dart 1426                      _rootRun
  dart:async/zone.dart 1328                      _CustomZone.run
  package:test_api/src/backend/invoker.dart 287  Invoker.heartbeat.<fn>
  dart:async-patch/timer_patch.dart 18           Timer._createTimer.<fn>
  dart:isolate-patch/timer_impl.dart 398         _Timer._runTimers
  dart:isolate-patch/timer_impl.dart 429         _Timer._handleMessage
  dart:isolate-patch/isolate_patch.dart 192      _RawReceivePortImpl._handleMessage

it's just a timeout

fzyzcjy commented 1 year ago

Weird... Maybe we should use the old valgrind + suppress that specific one error https://valgrind.org/docs/manual/manual-core.html#manual-core.suppress

fzyzcjy commented 1 year ago

@shekohex Btw I see you in the "Reviewers" list of this PR, and allo-isolate does have valgrind checks. Any suggestions about the bug above? :)

lattice0 commented 1 year ago

I couldn't even print the supression because I get a timeout at the moment of the error, on my computer. So at some time on the CI for a newer valgrind it will start giving a timeout as well.

fzyzcjy commented 1 year ago

Really weird... Maybe we should make a minimal reproducible sample (is it possible to even create a Rust-only sample without Dart?), and then post it to

It will also be easier to debug if we have a minimal repro sample.

For example, with the minimal sample, we may use gdb or equivalent tools to check where it stuck forever

Btw that's life of a system-language programmer :) Need to deal with low-level bugs

shekohex commented 1 year ago

@shekohex Btw I see you in the "Reviewers" list of this PR, and allo-isolate does have valgrind checks. Any suggestions about the bug above? :)

You need to add a small test or example that involves anyhow errors or backtrace somewhere in this file, to let valgrind check it.