Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
POC for step 1 of mozilla#419 - replace Handlemaps with Arcs.
I played with this primarily to better understand how uniffi actually worked and to try and get enough context to understand the challenges. It actually worked out surprisingly well (although is nowhere near mergable) The context for this is Ryan's [proposal for passing around object instances]( mozilla#419) but I've only played with "step 1" - replacing Handlemaps with `Arc<>` or `Arc<Mutex<>>`. I kept support for both threadsafe and non-threadsafe interfaces, primarily to avoid touching all the examples. It turns out the difference between `[Threadsafe]` and not is quite trivial. This might change in the future when we start supporting those other attributes, but maybe not. I'm still on the fence about this. There are some nasty hacks. The most obvious is using both `usize` and `u64` for pointer values. `usize` is probably correct, but `u64`is still handed around due to the lack of `usize` in Types. I tried using `const *TypeName` but this meant that `TypeName` needed to be `pub`, which isn't true in all the examples. You could argue that forcing these types to be `pub` is actually the right thing, but I don't care enough to actually do that :) It's also incomplete. I'm currently failing to run the kotlin tests (they fail in the same way in both this branch and on main - WSL FTW! :) (I very lightly edited the code blocks below - removing comments and logging) What it generates: in the non-threadsafe case, the constructor is: ```rust pub extern "C" fn threadsafe_ffd6_Counter_new( err: &mut uniffi::deps::ffi_support::ExternError, ) -> usize /* *const Counter */ { match std::panic::catch_unwind(|| { let _new = std::sync::Mutex::new(Counter::new()); let _arc = std::sync::Arc::new(_new); std::sync::Arc::into_raw(_arc) }) { Ok(ptr) => { *err = uniffi::deps::ffi_support::ExternError::default(); ptr as usize } Err(e) => { *err = e.into(); 0 as usize /*`std::ptr::null()` is a compile error */ } } } ``` The threadsafe case is identical except for 1 line: ```rust let _new = ThreadsafeCounter::new(); ``` (ie, no mutex) The `free()` function is also quite trivial and identical in both cases ```rust pub extern "C" fn ffi_threadsafe_ffd6_Counter_object_free(ptr: u64) { if let Err(e) = std::panic::catch_unwind(|| { assert_ne!(ptr, 0); // turn it into an Arc and let the value naturally drop. unsafe { std::sync::Arc::from_raw(ptr as *const Counter) }; }) { uniffi::deps::log::error!("ffi_threadsafe_ffd6_Counter_object_free panicked: {:?}", e); } } ``` The generated code for methods does have more variation between the 2, but not much. A non-threadsafe method: ```rust pub extern "C" fn threadsafe_ffd6_Counter_busy_wait( ptr: u64, ms: i32, err: &mut uniffi::deps::ffi_support::ExternError, ) -> () { uniffi::deps::ffi_support::call_with_output(err, || { let _arc = unsafe { std::sync::Arc::from_raw(ptr as *const std::sync::Mutex<Counter>) }; // This arc now "owns" the reference but we need an outstanding reference still. std::sync::Arc::into_raw(std::sync::Arc::clone(&_arc)); let _retval = Counter::busy_wait( &mut *_arc.lock().unwrap(), <i32 as uniffi::ViaFfi>::try_lift(ms).unwrap(), ); _retval }) } ``` for contrast, the thread-safe version: ```rust pub extern "C" fn threadsafe_ffd6_ThreadsafeCounter_busy_wait( ptr: u64, ms: i32, err: &mut uniffi::deps::ffi_support::ExternError, ) -> () { uniffi::deps::ffi_support::call_with_output(err, || { let _arc = unsafe { std::sync::Arc::from_raw(ptr as *const ThreadsafeCounter) }; // This arc now "owns" the reference but we need an outstanding reference still. std::sync::Arc::into_raw(std::sync::Arc::clone(&_arc)); let _retval = ThreadsafeCounter::busy_wait(&*_arc, <i32 as uniffi::ViaFfi>::try_lift(ms).unwrap()); _retval }) } ``` (As in the current version, there are small differences depending on whether an error is returned or not, but that's not particularly interesting here) The "keep the Arc alive by leaking a clone" wasn't immediately obvious to me until Python kept crashing :)
- Loading branch information
