Double free when posting DartExternalTypedData

[alexlapa]

So Im using flutter_rust_bridge and I have a StreamSink that send message to Dart every ~50 milliseconds. StreamSink serializes message to Vec<u8> and it is later converted to a DartExternalTypedData that is being posted to Dart via DartPostCObject.

So the problem is that I have an occasional segfault when closing the Flutter application:

Thread 32 Crashed:: Thread 0x0x14b619530
0   libsystem_kernel.dylib                 0x1904ab720 __pthread_kill + 8
1   libsystem_pthread.dylib                0x1904e3f70 pthread_kill + 288
2   libsystem_c.dylib                      0x1903f0908 abort + 128
3   libsystem_malloc.dylib                 0x1902f9e38 malloc_vreport + 896
4   libsystem_malloc.dylib                 0x1902fd9bc malloc_report + 64
5   libsystem_malloc.dylib                 0x19031c144 find_zone_and_free + 528
6   libflutterwebrtc.dylib         0x10847e2b4 _$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$::deallocate::h4707bbc82795b3c8 + 132
7   libflutterwebrtc.dylib         0x10847d8e8 alloc::raw_vec::RawVecInner$LT$A$GT$::deallocate::h88be447e750de4c8 + 96
8   libflutterwebrtc.dylib         0x10847e410 _$LT$alloc..raw_vec..RawVec$LT$T$C$A$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::hda09f9faaa9c4f5a + 36
9   libflutterwebrtc.dylib         0x10847ca9c core::ptr::drop_in_place$LT$alloc..raw_vec..RawVec$LT$u8$GT$$GT$::h3e85702e083d7e24 + 24
10  libflutterwebrtc.dylib         0x10847ca6c core::ptr::drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$::h762d7487b55fbe57 + 68
11  libflutterwebrtc.dylib         0x108478704 core::ptr::drop_in_place$LT$alloc..boxed..Box$LT$alloc..vec..Vec$LT$u8$GT$$GT$$GT$::he0dcf7b4e995225a + 32
12  libflutterwebrtc.dylib         0x108475708 core::mem::drop::h6d3cf56b1ececf5f + 28
13  libflutterwebrtc.dylib         0x108477c98 free_zero_copy_buffer_u8 + 64
14  libflutterwebrtc.dylib         0x108479bb8 allo_isolate::ffi::run_destructors::h7ef8dbdce778c75b + 140
15  libflutterwebrtc.dylib         0x1083d8920 allo_isolate::Isolate::post::h91f35f6c05f955ff + 528 (lib.rs:143)
16  libflutterwebrtc.dylib         0x1083eda8c flutter_rust_bridge::rust2dart::sender::Rust2DartSender::send::h0d5268c4230f2b94 + 24 (sender.rs:17)
17  libflutterwebrtc.dylib         0x108357d8c flutter_rust_bridge::stream::stream_sink::StreamSinkBase$LT$T$C$Rust2DartCodec$GT$::add_raw::hd79fa9ece921a353 + 136 (stream_sink.rs:40)

So it segfaults in free_zero_copy_buffer_u8 and the only thing this function does is drop(Box::from_raw(peer.cast::<Vec<u8>()));, so my first guess was that this must be a double free. Which I was able to confirm with additional logging:

vec_to_dart_native_external_typed_data data = 0x6000025844f0, peer: 0x6000027d8840, callback: 0x106072638
free_zero_copy_buffer_u8 _isolate_callback_data = 0x0, peer = 0x6000027d8840, vec = [0, 1, 0, 0, 0, 1, 0, 0, 0], thread = Thread { id: ThreadId(23), name: None, .. }
run_destructors DartExternalTypedData callback = 0x106072638
free_zero_copy_buffer_u8 _isolate_callback_data = 0x6000025844f0, peer = 0x6000027d8840, vec = [], thread = Thread { id: ThreadId(23), name: None, .. }

So what we see here is that free_zero_copy_buffer_u8 callback is called twice for the same vector (peer = 0x6000027d8840). Second call comes from ffi::run_destructors so DartPostCObject returned false. I'm not sure where the first call comes from, but my guess is Dart VM breaks its contract and reclaims CObject despite returning false from post. What you think about this?

[fzyzcjy]

Happens to see this when checking #65.

I'm not sure where the first call comes from

Quick brainstorm: Maybe we can print stack trace near the free_zero_copy_buffer_u8 log

[alexlapa]

@fzyzcjy ,

I was able to reproduce this outside of flutter_rust_bridge/dart_isolate: https://github.com/alexlapa/double_free_race_mre

So its either a Dart VM issue (dart-lang/native/issues/2079), or we are misusing DartPostCObject somehow, but it seems fine to me ¯_(ツ)_/¯

[shekohex]

@alexlapa I see dart-lang/native#2079 is now closed, should we close this one too?

[alexlapa]

Okay, so the problem is that we should not call DartPostCObject after Dart_Cleanup has been called, and we don't really know when it is called, but our best option is using NativeFinalizer to detect shutdown like this

So the main questing is how exactly this should be fixed in allo-isolate/frb.

allo-isolate does not have any Dart part so no way to create NativeFinalizer. And it's already possible to make it stop sending messages to dart at any moment by simply calling store_dart_post_cobject with fake function. So I guess there is already everything necessary. Well, maybe some mention in documentation would be nice.

So I guess it comes to frb/frb users. @fzyzcjy , what you think about this? Is there is anything in frb to bind NativeFinalizer to, or are you okay with reexporting allo_isolate::store_dart_post_cobject in Rust?

[fzyzcjy]

One approach may be, we just add things like alexlapa/double_free_race_mre@3aa1264...7a35a9a in frb_dart's init logic (create finalizer) and posting logic (check whether it is disposed).

[alexlapa]

So if we agree that there is nothing to do in allo-isolate then I'm closing this issue and I'll come up with a PR in frb later this week.