shekohex / allo-isolate

Run Multithreaded Rust along with Dart VM (in isolate) 🌀
Apache License 2.0
120 stars 18 forks source link

Make `zero-copy` and `ZeroCopyBuffer` work with `Vec<T>` of length 0 #48

Closed temeddix closed 1 year ago

temeddix commented 1 year ago

Summary

When enabling the zero-copy feature or using ZeroCopyBuffer, Vec<T> with length of 0 always made the Dart-Rust app crash. This turned out to be a behavior with isolate_callback_data, which returns proper length value when the length of Vec<T> was not 0, but returns an oddly big number when Vec<T> has zero length.

https://github.com/cunarist/rust-in-flutter/issues/165

Experiment

Adding some println!()s can help us inspect this problem.

// into_dart.rs

fn vec_to_dart_native_external_typed_data<T>(
    vec_from_rust: Vec<T>,
) -> DartCObject
where
    T: DartTypedDataTypeTrait,
{
    let mut vec = ManuallyDrop::new(vec_from_rust);
    vec.shrink_to_fit();
    let length = vec.len();
    assert_eq!(length, vec.capacity());
    let ptr = vec.as_mut_ptr();
    println!("{:?}, {:?}", ptr, length); // THIS LINE

    DartCObject {
        ty: DartCObjectType::DartExternalTypedData,
        value: DartCObjectValue {
            as_external_typed_data: DartNativeExternalTypedData {
                ty: T::dart_typed_data_type(),
                length: length as isize,
                data: ptr as *mut u8,
                peer: ptr as *mut c_void,
                callback: T::function_pointer_of_free_zero_copy_buffer(),
            },
        },
    }
}
// into_dart.rs

            #[doc(hidden)]
            #[no_mangle]
            pub(crate) unsafe extern "C" fn $free_zero_copy_buffer_func(
                isolate_callback_data: *mut c_void,
                peer: *mut c_void,
            ) {
                let len = (isolate_callback_data as isize) as usize;
                let ptr = peer as *mut $rust_type;
                println!("{:?}, {:?}", ptr, len); // THIS LINE
                drop(Vec::from_raw_parts(ptr, len, len));
            }
        )+

When passing in Vec<T> of 6 bytes with zero-copy feature enabled(or using ZeroCopyBuffer), we get this output:

0x1, 6
0x1, 6

When passing in Vec<T> of 0 bytes with zero-copy feature enabled(or using ZeroCopyBuffer), we get this output:

0x1, 0
0x1, 140208384468704

The length value from Rust Vec<T> is correct, but the length value from isolate_callback_data is very weird. This led to deallocating memory of a totally wrong location which always made the Dart-Rust app crash.

I've confirmed that this change effectively works on an actual project that uses Vec<T> of length 0 with zero-copy. Tests are also added in this PR.

shekohex commented 1 year ago

So casting to isize made that zero number to be a very big number like 140208384468704? huh weird, I guess thats why using as is bad.

shekohex commented 1 year ago

Did you test this branch with your code? is that fixes your bug?

temeddix commented 1 year ago

I tested with the code provided in this issue, which should obviously work. The code just simply involved passing zero-length Vec<T>s. I confirmed that with my forked allo-isolate, zero-length Vec<T> with zero-copy works properly.

shekohex commented 1 year ago

Will publish a new version very soon.

temeddix commented 1 year ago

Thanks for notifying :)

shekohex commented 1 year ago

Published as v0.1.20 🎉