diff --git a/docs/manual/src/internals/lifting_and_lowering.md b/docs/manual/src/internals/lifting_and_lowering.md index 3f3e14cf18..0743fb0681 100644 --- a/docs/manual/src/internals/lifting_and_lowering.md +++ b/docs/manual/src/internals/lifting_and_lowering.md @@ -66,7 +66,7 @@ Calling this function from foreign language code involves the following steps: | `record` | `RustBuffer` struct pointing to serialized bytes | | `enum` and `[Enum] interface` | `RustBuffer` struct pointing to serialized bytes | | `dictionary` | `RustBuffer` struct pointing to serialized bytes | -| `interface` | `uint64_t` opaque integer handle | +| `interface` | `void*` opaque pointer to object on the heap | ## Serialization Format @@ -88,7 +88,7 @@ The details of this format are internal only and may change between versions of | `record` | Serialized `i32` item count followed by serialized items; each item is a serialized `string` followed by a serialized `T` | | `enum` and `[Enum] interface` | Serialized `i32` indicating variant, numbered in declaration order starting from 1, followed by the serialized values of the variant's fields in declaration order | | `dictionary` | The serialized value of each field, in declaration order | -| `interface` | *Cannot currently be serialized* | +| `interface` | Fixed-width 8-byte unsigned integer encoding a pointer to the object on the heap | Note that length fields in this format are serialized as *signed* integers despite the fact that they will always be non-negative. This is to help diff --git a/docs/manual/src/internals/object_references.md b/docs/manual/src/internals/object_references.md index b9836dfd7c..c3a9ba31ea 100644 --- a/docs/manual/src/internals/object_references.md +++ b/docs/manual/src/internals/object_references.md @@ -1,7 +1,7 @@ # Managing Object References UniFFI [interfaces](../udl/interfaces.md) represent instances of objects -that have methods and contain shared mutable state. One of Rust's core innovations +that have methods and contain state. One of Rust's core innovations is its ability to provide compile-time guarantees about working with such instances, including: @@ -16,6 +16,8 @@ system. UniFFI itself tries to take a hands-off approach as much as possible and depends on the Rust compiler itself to uphold safety guarantees, without assuming that foreign-language callers will be "well behaved". +## Concurrency + UniFFI's hands-off approach means that all object instances exposed by UniFFI must be safe to access concurrently. In Rust terminology, they must be `Send+Sync` and must be useable without taking any `&mut` references. @@ -27,18 +29,22 @@ of the component - as much as possible, UniFFI tries to stay out of your way, si that the object implementation is `Send+Sync` and letting the Rust compiler ensure that this is so. -## Handle Maps +## Lifetimes + +In order to allow for instances to be used as flexibly as possible from foreign-language code, +UniFFI wraps all object instances in an `Arc` and leverages their reference-count based lifetimes, +allowing UniFFI to largely stay out of handling lifetimes entirely for these objects. -For additional typechecking safety, UniFFI indirects all object access through a -"handle map", a mapping from opaque integer handles to object instances. This indirection -imposes a small runtime cost but helps us guard against errors or oversights -in the generated bindings. +When constructing a new object, UniFFI is able to add the `Arc` automatically, because it +knows that the return type of the Rust constructor must be a new uniquely-owned struct of +the corresponding type. -For each interface declared in the UDL, the UniFFI-generated Rust scaffolding -will create a global handlemap that is responsible for owning all instances -of that interface, and handing out references to them when methods are called. -The handlemap requires that its contents be `Send+Sync`, helping enforce requirements -around thread-safety. +When you want to return object instances from functions or methods, or store object instances +as fields in records, the underlying Rust code will need to work with `Arc` directly, to ensure +that the code behaves in the way that UniFFI expects. + +When accepting instances as arguments, the underlying Rust code can choose to accept it as an `Arc` +or as the underlying struct `T`, as there are different use-cases for each scenario. For example, given a interface definition like this: @@ -50,53 +56,90 @@ interface TodoList { }; ``` -The Rust scaffolding would define a lazyily-initialized global static like: +On the Rust side of the generated bindings, the instance constructor will create an instance of the +corresponding `TodoList` Rust struct, wrap it in an `Arc<>` and return the Arc's raw pointer to the +foreign language code: ```rust -lazy_static! { - static ref UNIFFI_HANDLE_MAP_TODOLIST: ArcHandleMap = ArcHandleMap::new(); +pub extern "C" fn todolist_12ba_TodoList_new( + err: &mut uniffi::deps::ffi_support::ExternError, +) -> *const std::os::raw::c_void /* *const TodoList */ { + uniffi::deps::ffi_support::call_with_output(err, || { + let _new = TodoList::new(); + let _arc = std::sync::Arc::new(_new); + as uniffi::ViaFfi>::lower(_arc) + }) } ``` -On the Rust side of the generated bindings, the instance constructor will create an instance of the -corresponding `TodoList` Rust struct, insert it into the handlemap, and return the resulting integer -handle to the foreign language code: +The UniFFI runtime implements lowering for object instances using `Arc::into_raw`: ```rust -pub extern "C" fn todolist_TodoList_new(err: &mut ExternError) -> u64 { - // Give ownership of the new instance to the handlemap. - // We will only ever operate on borrowed references to it. - UNIFFI_HANDLE_MAP_TODOLIST.insert_with_output(err, || TodoList::new()) +unsafe impl ViaFfi for std::sync::Arc { + type FfiType = *const std::os::raw::c_void; + fn lower(self) -> Self::FfiType { + std::sync::Arc::into_raw(self) as Self::FfiType + } } ``` -When invoking a method on the instance, the foreign-language code passes the integer handle back -to the Rust code, which borrows a reference to the instance from the handlemap for the duration -of the method call: +which does the "arc to pointer" dance for us. Note that this has "leaked" the +`Arc<>` reference out of Rusts ownership system and given it to the foreign-language code. +The foreign-language code must pass that pointer back into Rust in order to free it, +or our instance will leak. + +When invoking a method on the instance, the foreign-language code passes the +raw pointer back to the Rust code, conceptually passing a "borrow" of the `Arc<>` to +the Rust scaffolding. The Rust side turns it back into a cloned `Arc<>` which +lives for the duration of the method call: ```rust -pub extern "C" fn todolist_TodoList_add_item(handle: u64, todo: RustBuffer, err: &mut ExternError) -> () { - let todo = ::try_lift(todo).unwrap() - // Borrow a reference to the instance so that we can call a method on it. - UNIFFI_HANDLE_MAP_TODOLIST.call_with_result_mut(err, handle, |obj| -> Result<(), TodoError> { - TodoList::add_item(obj, todo) +pub extern "C" fn todolist_12ba_TodoList_add_item( + ptr: *const std::os::raw::c_void, + todo: uniffi::RustBuffer, + err: &mut uniffi::deps::ffi_support::ExternError, +) -> () { + uniffi::deps::ffi_support::call_with_result(err, || -> Result<_, TodoError> { + let _obj = as uniffi::ViaFfi>::try_lift(ptr).unwrap(); + let _retval = + TodoList::add_item(&_obj, ::try_lift(todo).unwrap())?; + Ok(_retval) }) } ``` -Finally, when the foreign-language code frees the instance, it passes the integer handle to -a special destructor function so that the Rust code can delete it from the handlemap: +The UniFFI runtime implements lifting for object instances using `Arc::from_raw`: ```rust -pub extern "C" fn ffi_todolist_TodoList_object_free(handle: u64) { - UNIFFI_HANDLE_MAP_TODOLIST.delete_u64(handle); -} +unsafe impl ViaFfi for std::sync::Arc { + type FfiType = *const std::os::raw::c_void; + fn try_lift(v: Self::FfiType) -> Result { + let v = v as *const T; + // We musn't drop the `Arc` that is owned by the foreign-language code. + let foreign_arc = std::mem::ManuallyDrop::new(unsafe { Self::from_raw(v) }); + // Take a clone for our own use. + Ok(std::sync::Arc::clone(&*foreign_arc)) + } ``` -This indirection gives us some important safety properties: +Notice that we take care to ensure the reference that is owned by the foreign-language +code remains alive. + +Finally, when the foreign-language code frees the instance, it +passes the raw pointer a special destructor function so that the Rust code can +drop that initial reference (and if that happens to be the final reference, +the Rust object will be dropped.) + +```rust +pub extern "C" fn ffi_todolist_12ba_TodoList_object_free(ptr: *const std::os::raw::c_void) { + if let Err(e) = std::panic::catch_unwind(|| { + assert!(!ptr.is_null()); + unsafe { std::sync::Arc::from_raw(ptr as *const TodoList) }; + }) { + uniffi::deps::log::error!("ffi_todolist_12ba_TodoList_object_free panicked: {:?}", e); + } +} +``` -* If the generated bindings incorrectly pass an invalid handle, or a handle for a different type of object, - then the handlemap will throw an error with high probability, providing some amount of run-time typechecking - for correctness of the generated bindings. -* The handlemap can ensure we uphold Rust's requirements around unique mutable references and threadsafey, - by specifying that the contained type must be `Send+Sync`, and by refusing to hand out any mutable references. +Passing instances as arguments and returning them as values works similarly, except that +UniFFI does not automatically wrap/unwrap the containing `Arc`. diff --git a/docs/manual/src/udl/functions.md b/docs/manual/src/udl/functions.md index 9766b1875a..f86273e6d0 100644 --- a/docs/manual/src/udl/functions.md +++ b/docs/manual/src/udl/functions.md @@ -15,3 +15,4 @@ The UDL file will look like: namespace Example { string hello_world(); } +``` \ No newline at end of file diff --git a/docs/manual/src/udl/interfaces.md b/docs/manual/src/udl/interfaces.md index cc54a77f40..e654e024d9 100644 --- a/docs/manual/src/udl/interfaces.md +++ b/docs/manual/src/udl/interfaces.md @@ -93,6 +93,104 @@ For each alternate constructor, UniFFI will expose an appropriate static-method, in the foreign language binding, and will connect it to the Rust method of the same name on the underlying Rust struct. +## Managing Shared References + +To the foreign-language consumer, UniFFI object instances are designed to behave as much like +regular language objects as possible. They can be freely passed as arguments or returned as values, +like this: + +```idl +interface TodoList { + ... + + // Copy the items from another TodoList into this one. + void import_items(TodoList other); + + // Make a copy of this TodoList as a new instance. + TodoList duplicate(); +}; +``` + +To ensure that this is safe, UniFFI allocates every object instance on the heap using +[`Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html), Rust's built-in smart pointer +type for managing shared references at runtime. + +The use of `Arc` is transparent to the foreign-language code, but sometimes shows up +in the function signatures of the underlying Rust code. For example, the Rust code implementing +the `TodoList::duplicate` method would need to explicitly return an `Arc`, since UniFFI +doesn't know whether it will be returning a new object or an existing one: + +```rust +impl TodoList { + fn duplicate(&self) -> Arc { + Arc::new(TodoList { + items: RwLock::new(self.items.read().unwrap().clone()) + }) + } +} +``` + +By default, object instances passed as function arguments will also be passed as an `Arc`, so the +Rust implementation of `TodoList::import_items` would also need to accept an `Arc`: + +```rust +impl TodoList { + fn import_items(&self, other: Arc) { + self.items.write().unwrap().append(other.get_items()); + } +} +``` + +If the Rust code does not need an owned reference to the `Arc`, you can use the `[ByRef]` UDL attribute +to signal that a function accepts a borrowed reference: + +```idl +interface TodoList { + ... + // +-- indicate that we only need to borrow the other list + // V + void import_items([ByRef] TodoList other); + ... +}; +``` + +```rust +impl TodoList { + // +-- don't need to care about the `Arc` here + // V + fn import_items(&self, other: &TodoList) { + self.items.write().unwrap().append(other.get_items()); + } +} +``` + +Conversely, if the Rust code explicitly *wants* to deal with an `Arc` in the special case of +the `self` parameter, it can signal this using the `[Self=ByArc]` UDL attribute on the method: + + +```idl +interface TodoList { + ... + // +-- indicate that we want the `Arc` containing `self` + // V + [Self=ByArc] + void import_items(TodoList other); + ... +}; +``` + +```rust +impl TodoList { + // `Arc`s everywhere! --+-----------------+ + // V V + fn import_items(self: Arc, other: Arc) { + self.items.write().unwrap().append(other.get_items()); + } +} +``` + +You can read more about the technical details in the docs on the +[internal details of managing object references](../internals/object_references.md). ## Concurrent Access @@ -132,7 +230,7 @@ impl Counter { Self { value: 0 } } - // No mutable references to self allowed in in UniFFI interfaces. + // No mutable references to self allowed in UniFFI interfaces. fn increment(&mut self) { self.value = self.value + 1; } @@ -194,4 +292,4 @@ impl Counter { ``` You can read more about the technical details in the docs on the -[internal details of managing object references](../internals/object_references.md). \ No newline at end of file +[internal details of managing object references](../internals/object_references.md). diff --git a/docs/manual/src/udl/structs.md b/docs/manual/src/udl/structs.md index 7dd583bb1a..b3f3bc15aa 100644 --- a/docs/manual/src/udl/structs.md +++ b/docs/manual/src/udl/structs.md @@ -2,6 +2,8 @@ Dictionaries can be compared to POJOs in the Java world: just a data structure holding some data. +A Rust struct like this: + ```rust struct TodoEntry { done: bool, @@ -10,7 +12,7 @@ struct TodoEntry { } ``` -can be converted in UDL to: +Can be exposed via UniFFI using UDL like this: ```idl dictionary TodoEntry { @@ -18,7 +20,44 @@ dictionary TodoEntry { u64 due_date; string text; }; +``` + +The fields in a dictionary can be of almost any type, including objects or other dictionaries. +The current limitations are: + +* They cannot recursively contain another intance of the *same* dictionary. +* They cannot contain references to callback interfaces. + +## Fields holding Object References + +If a dictionary contains a field whose type is an [interface](./interfaces.md), then that +field will hold a *reference* to an underlying instance of a Rust struct. The Rust code for +working with such fields must store them as an `Arc` in order to help properly manage the +lifetime of the instance. So if the UDL interface looked like this: + +```idl +interface User { + // Some sort of "user" object that can own todo items +}; + +dictionary TodoEntry { + User owner; + string text; +} +``` + +Then the corresponding Rust code would need to look like this: +```rust +struct TodoEntry { + owner: std::sync::Arc, + text: String, +} ``` -Dictionaries can contain each other and every other data type available, except objects. +Depending on the languge, the foreign-language bindings may also need to be aware of +these embedded references. For example in Kotlin, each Object instance must be explicitly +destroyed to avoid leaking the underlying memory, and this also applies to Objects stored +in record fields. + +You can read more about managing object references in the section on [interfaces](./interfaces.md). diff --git a/examples/todolist/Cargo.toml b/examples/todolist/Cargo.toml index 73a52daf0f..ef58034a60 100644 --- a/examples/todolist/Cargo.toml +++ b/examples/todolist/Cargo.toml @@ -14,6 +14,7 @@ name = "uniffi_todolist" uniffi_macros = {path = "../../uniffi_macros"} uniffi = {path = "../../uniffi", features=["builtin-bindgen"]} thiserror = "1.0" +lazy_static = "1.4" [build-dependencies] uniffi_build = {path = "../../uniffi_build", features=["builtin-bindgen"]} diff --git a/examples/todolist/src/lib.rs b/examples/todolist/src/lib.rs index 1d7db705fc..1ea1d0da80 100644 --- a/examples/todolist/src/lib.rs +++ b/examples/todolist/src/lib.rs @@ -2,13 +2,19 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; #[derive(Debug, Clone)] pub struct TodoEntry { text: String, } +lazy_static::lazy_static! { + // There is a single "default" TodoList that can be shared + // by all consumers of this component. + static ref DEFAULT_LIST: RwLock>> = RwLock::new(None); +} + #[derive(Debug, thiserror::Error)] enum TodoError { #[error("The todo does not exist!")] @@ -23,6 +29,22 @@ enum TodoError { DeligatedError(#[from] std::io::Error), } +/// Get a reference to the global default TodoList, if set. +/// +fn get_default_list() -> Option> { + DEFAULT_LIST.read().unwrap().clone() +} + +/// Set the global default TodoList. +/// +/// This will silently drop any previously set value. +/// +fn set_default_list(list: Arc) { + *DEFAULT_LIST.write().unwrap() = Some(list); +} + +/// Create a new TodoEntry from the given string. +/// fn create_entry_with>(item: S) -> Result { let text = item.into(); if text == "" { @@ -117,6 +139,10 @@ impl TodoList { items.remove(idx); Ok(()) } + + fn make_default(self: Arc) { + set_default_list(self); + } } include!(concat!(env!("OUT_DIR"), "/todolist.uniffi.rs")); diff --git a/examples/todolist/src/todolist.udl b/examples/todolist/src/todolist.udl index 5120fb70af..5c923314cd 100644 --- a/examples/todolist/src/todolist.udl +++ b/examples/todolist/src/todolist.udl @@ -1,4 +1,7 @@ namespace todolist { + TodoList? get_default_list(); + undefined set_default_list(TodoList list); + [Throws=TodoError] TodoEntry create_entry_with(string todo); }; @@ -30,4 +33,6 @@ interface TodoList { string get_first(); [Throws=TodoError] void clear_item(string todo); + [Self=ByArc] + undefined make_default(); }; diff --git a/examples/todolist/tests/bindings/test_todolist.py b/examples/todolist/tests/bindings/test_todolist.py index 0e3b5b434b..03994df2c3 100644 --- a/examples/todolist/tests/bindings/test_todolist.py +++ b/examples/todolist/tests/bindings/test_todolist.py @@ -23,3 +23,24 @@ entry2 = TodoEntry("Test Ünicode hàndling in an entry can't believe I didn't test this at first 🤣") todo.add_entry(entry2) assert(todo.get_last_entry().text == "Test Ünicode hàndling in an entry can't believe I didn't test this at first 🤣") + +todo2 = TodoList() +assert(todo != todo2) +assert(todo is not todo2) + +assert(get_default_list() is None) + +set_default_list(todo) +assert(get_default_list() is todo) +assert(get_default_list() is not todo2) + +todo2.make_default() +assert(get_default_list() is not todo) +assert(get_default_list() is todo2) + +todo.add_item("Test liveness after being demoted from default") +assert(todo.get_last() == "Test liveness after being demoted from default") + +todo2.add_item("Test shared state through local vs default reference") +assert(get_default_list().get_last() == "Test shared state through local vs default reference") + diff --git a/fixtures/coverall/src/coverall.udl b/fixtures/coverall/src/coverall.udl index 49cfdaeddf..4f7cef4e60 100644 --- a/fixtures/coverall/src/coverall.udl +++ b/fixtures/coverall/src/coverall.udl @@ -22,6 +22,13 @@ dictionary SimpleDict { float? maybe_float32; double float64; double? maybe_float64; + Coveralls? coveralls; +}; + +[Enum] +interface MaybeSimpleDict { + Yeah(SimpleDict d); + Nah(); }; [Error] @@ -52,6 +59,34 @@ interface Coveralls { /// Calls `Arc::strong_count()` on the `Arc` containing `self`. [Self=ByArc] u64 strong_count(); + + /// Takes an `Arc` and stores it in `self`, dropping the existing + /// reference. Note you can create circular references by passing `self`. + void take_other(Coveralls? other); + + /// Returns what was previously set via `take_other()`, or null. + Coveralls? get_other(); + + /// Same signature as `take_other` but always fails. + [Self=ByArc, Throws=CoverallError] + void take_other_fallible(); + + /// Same signature as `take_other` but with an extra string arg - always + /// panics with that message.. + [Self=ByArc] + void take_other_panic(string message); + + // can't name it `clone` as it conflicts with the Clone trait and ours has a different signature + Coveralls clone_me(); +}; + +// All coveralls end up with a patch. +enum Color {"Red", "Blue", "Green"}; + +interface Patch { + constructor(Color color); + + Color get_color(); }; interface ThreadsafeCounter { diff --git a/fixtures/coverall/src/lib.rs b/fixtures/coverall/src/lib.rs index 2741ab8c06..74f969d040 100644 --- a/fixtures/coverall/src/lib.rs +++ b/fixtures/coverall/src/lib.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use std::sync::atomic::{AtomicBool, AtomicI32, Ordering}; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex, RwLock}; lazy_static::lazy_static! { static ref NUM_ALIVE: RwLock = { @@ -35,6 +35,13 @@ pub struct SimpleDict { maybe_float32: Option, float64: f64, maybe_float64: Option, + coveralls: Option>, +} + +#[derive(Debug, Clone)] +pub enum MaybeSimpleDict { + Yeah{d: SimpleDict}, + Nah, } fn create_some_dict() -> SimpleDict { @@ -55,6 +62,7 @@ fn create_some_dict() -> SimpleDict { maybe_float32: Some(22.0 / 7.0), float64: 0.0, maybe_float64: Some(1.0), + coveralls: Some(Arc::new(Coveralls::new("some_dict".to_string()))), } } @@ -76,6 +84,7 @@ fn create_none_dict() -> SimpleDict { maybe_float32: None, float64: 0.0, maybe_float64: None, + coveralls: None, } } @@ -88,12 +97,18 @@ type Result = std::result::Result; #[derive(Debug)] pub struct Coveralls { name: String, + // A reference to another Coveralls. Currently will be only a reference + // to `self`, so will create a circular reference. + other: Mutex>>, } impl Coveralls { fn new(name: String) -> Self { *NUM_ALIVE.write().unwrap() += 1; - Self { name } + Self { + name, + other: Mutex::new(None), + } } fn fallible_new(name: String, should_fail: bool) -> Result { @@ -127,6 +142,38 @@ impl Coveralls { fn strong_count(self: Arc) -> u64 { Arc::strong_count(&self) as u64 } + + fn take_other(&self, other: Option>) { + *self.other.lock().unwrap() = match other { + None => None, + Some(arc) => Some(Arc::clone(&arc)), + } + } + + fn get_other(&self) -> Option> { + match &*self.other.lock().unwrap() { + Some(other) => Some(Arc::clone(other)), + None => None, + } + } + + fn take_other_fallible(self: Arc) -> Result<()> { + Err(CoverallError::TooManyHoles) + } + + fn take_other_panic(self: Arc, message: String) -> () { + panic!("{}", message); + } + + fn clone_me(&self) -> Arc { + let other = self.other.lock().unwrap(); + let new_other = Mutex::new(other.clone()); + *NUM_ALIVE.write().unwrap() += 1; + Arc::new(Self { + name: self.name.clone(), + other: new_other, + }) + } } impl Drop for Coveralls { @@ -134,6 +181,27 @@ impl Drop for Coveralls { *NUM_ALIVE.write().unwrap() -= 1; } } +#[derive(Debug, Clone, Copy)] +enum Color { + Red, + Blue, + Green, +} + +#[derive(Debug)] +struct Patch { + color: Color, +} + +impl Patch { + fn new(color: Color) -> Self { + Self { color } + } + + fn get_color(&self) -> Color { + self.color + } +} // This is a small implementation of a counter that allows waiting on one thread, // and counting on another thread. We use it to test that the UniFFI generated scaffolding diff --git a/fixtures/coverall/tests/bindings/test_coverall.kts b/fixtures/coverall/tests/bindings/test_coverall.kts index ea90e63c0e..5a6c6a33fd 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.kts +++ b/fixtures/coverall/tests/bindings/test_coverall.kts @@ -6,31 +6,74 @@ import java.util.concurrent.* import uniffi.coverall.* -// Test some_dict(). // TODO: use an actual test runner. -val d = createSomeDict(); -assert(d.text == "text"); -assert(d.maybeText == "maybe_text"); -assert(d.aBool); -assert(d.maybeABool == false); -assert(d.unsigned8 == 1.toUByte()) -assert(d.maybeUnsigned8 == 2.toUByte()) -assert(d.unsigned64 == 18446744073709551615UL) -assert(d.maybeUnsigned64 == 0UL) -assert(d.signed8 == 8.toByte()) -assert(d.maybeSigned8 == 0.toByte()) -assert(d.signed64 == 9223372036854775807L) -assert(d.maybeSigned64 == 0L) +// Test some_dict(). +// N.B. we need to `use` here to clean up the contained `Coveralls` reference. +createSomeDict().use { d -> + assert(d.text == "text"); + assert(d.maybeText == "maybe_text"); + assert(d.aBool); + assert(d.maybeABool == false); + assert(d.unsigned8 == 1.toUByte()) + assert(d.maybeUnsigned8 == 2.toUByte()) + assert(d.unsigned64 == 18446744073709551615UL) + assert(d.maybeUnsigned64 == 0UL) + assert(d.signed8 == 8.toByte()) + assert(d.maybeSigned8 == 0.toByte()) + assert(d.signed64 == 9223372036854775807L) + assert(d.maybeSigned64 == 0L) + + // floats should be "close enough". + fun Float.almostEquals(other: Float) = Math.abs(this - other) < 0.000001 + fun Double.almostEquals(other: Double) = Math.abs(this - other) < 0.000001 + + assert(d.float32.almostEquals(1.2345F)) + assert(d.maybeFloat32!!.almostEquals(22.0F/7.0F)) + assert(d.float64.almostEquals(0.0)) + assert(d.maybeFloat64!!.almostEquals(1.0)) + + assert(d.coveralls!!.getName() == "some_dict") +} + -// floats should be "close enough". -fun Float.almostEquals(other: Float) = Math.abs(this - other) < 0.000001 -fun Double.almostEquals(other: Double) = Math.abs(this - other) < 0.000001 +// Test arcs. + +Coveralls("test_arcs").use { coveralls -> + assert(getNumAlive() == 1UL); + // One ref held by the foreign-language code, one created for this method call. + assert(coveralls.strongCount() == 2UL); + assert(coveralls.getOther() == null); + coveralls.takeOther(coveralls); + // Should now be a new strong ref, held by the object's reference to itself. + assert(coveralls.strongCount() == 3UL); + // But the same number of instances. + assert(getNumAlive() == 1UL); + // Careful, this makes a new Kotlin object which must be separately destroyed. + coveralls.getOther()!!.use { other -> + // It's the same Rust object. + assert(other.getName() == "test_arcs") + // TODO: ideally, it would return the same Kotlin-level object. + // (but how does that interact with auto-closing..?) + // assert(coveralls.getOther()!! === coveralls); + } + try { + coveralls.takeOtherFallible() + throw RuntimeException("Should have thrown a IntegerOverflow exception!") + } catch (e: CoverallErrorException.TooManyHoles) { + // It's okay! + } + try { + coveralls.takeOtherPanic("expected panic: with an arc!") + throw RuntimeException("Should have thrown a IntegerOverflow exception!") + } catch (e: InternalException) { + // No problemo! + } + coveralls.takeOther(null); + assert(coveralls.strongCount() == 2UL); +} +assert(getNumAlive() == 0UL); -assert(d.float32.almostEquals(1.2345F)) -assert(d.maybeFloat32!!.almostEquals(22.0F/7.0F)) -assert(d.float64.almostEquals(0.0)) -assert(d.maybeFloat64!!.almostEquals(1.0)) // This tests that the UniFFI-generated scaffolding doesn't introduce any unexpected locking. // We have one thread busy-wait for a some period of time, while a second thread repeatedly diff --git a/fixtures/coverall/tests/bindings/test_coverall.py b/fixtures/coverall/tests/bindings/test_coverall.py index d7fb1accd4..e1d532711c 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.py +++ b/fixtures/coverall/tests/bindings/test_coverall.py @@ -6,6 +6,11 @@ from coverall import * class TestCoverall(unittest.TestCase): + # Any test not terminating with zero objects alive will cause others to + # fail - this helps us work out which test kept things alive. + def tearDown(self): + self.assertEqual(get_num_alive(), 0) + def test_some_dict(self): d = create_some_dict() self.assertEqual(d.text, "text") @@ -20,6 +25,7 @@ def test_some_dict(self): self.assertEqual(d.maybe_signed8, 0) self.assertEqual(d.signed64, 9223372036854775807) self.assertEqual(d.maybe_signed64, 0) + self.assertEqual(d.coveralls.get_name(), "some_dict") # floats should be "close enough" - although it's mildly surprising that # we need to specify `places=6` whereas the default is 7. @@ -47,6 +53,7 @@ def test_none_dict(self): self.assertIsNone(d.maybe_float32) self.assertAlmostEqual(d.float64, 0.0) self.assertIsNone(d.maybe_float64) + self.assertIsNone(d.coveralls) def test_constructors(self): self.assertEqual(get_num_alive(), 0) @@ -67,7 +74,7 @@ def test_constructors(self): with self.assertRaisesRegex(InternalError, "expected panic: woe is me"): Coveralls.panicing_new("expected panic: woe is me") - # in the absence of cycles Python is deterministic killing refs + # in the absence of cycles Python is deterministic in killing refs coveralls2 = None self.assertEqual(get_num_alive(), 1) coveralls = None @@ -89,5 +96,65 @@ def test_self_by_arc(self): # One reference is held by the handlemap, and one by the `Arc` method receiver. self.assertEqual(coveralls.strong_count(), 2) + def test_arcs(self): + coveralls = Coveralls("test_arcs") + self.assertEqual(get_num_alive(), 1) + self.assertEqual(coveralls.strong_count(), 2) + self.assertIsNone(coveralls.get_other()) + coveralls.take_other(coveralls) + # should now be a new strong ref. + self.assertEqual(coveralls.strong_count(), 3) + # but the same number of instances. + self.assertEqual(get_num_alive(), 1) + # and check it's the correct object. + self.assertEqual(coveralls.get_other().get_name(), "test_arcs") + # TODO: when we implement an identity map, it will even be the same Python object + # self.assertTrue(coveralls.get_other() is coveralls) + + with self.assertRaises(CoverallError.TooManyHoles): + coveralls.take_other_fallible() + + with self.assertRaisesRegex(InternalError, "expected panic: with an arc!"): + coveralls.take_other_panic("expected panic: with an arc!") + + coveralls.take_other(None) + self.assertEqual(coveralls.strong_count(), 2) + coveralls = None + self.assertEqual(get_num_alive(), 0) + + def test_return_objects(self): + coveralls = Coveralls("test_return_objects") + self.assertEqual(get_num_alive(), 1) + self.assertEqual(coveralls.strong_count(), 2) + c2 = coveralls.clone_me() + self.assertEqual(c2.get_name(), coveralls.get_name()) + self.assertEqual(get_num_alive(), 2) + self.assertEqual(c2.strong_count(), 2) + + coveralls.take_other(c2) + # same number alive but `c2` has an additional ref count. + self.assertEqual(get_num_alive(), 2) + self.assertEqual(coveralls.strong_count(), 2) + self.assertEqual(c2.strong_count(), 3) + + # and get_other() should return the exact same instance. + self.assertTrue(coveralls.get_other() is c2) + + # We can drop Python's reference to `c2`, but the rust struct will not + # be dropped as coveralls hold an `Arc<>` to it. + c2 = None + self.assertEqual(get_num_alive(), 2) + + # Dropping `coveralls` will kill both. + coveralls = None + self.assertEqual(get_num_alive(), 0) + + def test_bad_objects(self): + coveralls = Coveralls("test_return_objects") + patch = Patch(Color.RED) + # `coveralls.take_other` wants `Coveralls` not `Patch` + with self.assertRaisesRegex(TypeError, "Coveralls.*Patch"): + coveralls.take_other(patch) + if __name__=='__main__': unittest.main() diff --git a/fixtures/coverall/tests/bindings/test_coverall.rb b/fixtures/coverall/tests/bindings/test_coverall.rb index ae23faa1d5..e0d064a64e 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.rb +++ b/fixtures/coverall/tests/bindings/test_coverall.rb @@ -8,6 +8,7 @@ require 'coverall' class TestCoverall < Test::Unit::TestCase + def test_some_dict d = Coverall.create_some_dict assert_equal(d.text, 'text') @@ -28,6 +29,8 @@ def test_some_dict assert_equal(d.float64, 0.0) assert_equal(d.maybe_float64, 1.0) + + assert_equal(d.coveralls.get_name(), "some_dict") end def test_none_dict @@ -52,6 +55,7 @@ def test_none_dict end def test_constructors + GC.start assert_equal(Coverall.get_num_alive, 0) # must work. coveralls = Coverall::Coveralls.new 'c1' @@ -77,9 +81,9 @@ def test_constructors end begin - obejcts = 10.times.map { Coverall::Coveralls.new 'c1' } + objects = 10.times.map { Coverall::Coveralls.new 'c1' } assert_equal 12, Coverall.get_num_alive - obejcts = nil + objects = nil GC.start end @@ -109,4 +113,51 @@ def test_self_by_arc # One reference is held by the handlemap, and one by the `Arc` method receiver. assert_equal coveralls.strong_count, 2 end + + def test_arcs + GC.start + coveralls = Coverall::Coveralls.new 'test_arcs' + assert_equal 1, Coverall.get_num_alive + + assert_equal 2, coveralls.strong_count + assert_equal nil, coveralls.get_other + + coveralls.take_other coveralls + # should now be a new strong ref. + assert_equal 3, coveralls.strong_count + # but the same number of instances. + assert_equal 1, Coverall.get_num_alive + # and check it's the correct object. + assert_equal "test_arcs", coveralls.get_other.get_name + # TODO: when we implement an identity map, it will even be the same Ruby object + # self.assertTrue(coveralls.get_other() is coveralls) + + # TODO: using `assert_raise` seems to keep a reference to `coveralls` alive, + # breaking the reference-count checks further down. + begin + coveralls.take_other_fallible + rescue Coverall::CoverallError::TooManyHoles + # OK + else + raise 'should have thrown' + end + + begin + coveralls.take_other_panic "expected panic: with an arc!" + rescue Coverall::InternalError => err + assert_match /expected panic: with an arc!/, err.message + else + raise 'should have thrown' + end + + coveralls.take_other nil + GC.start + assert_equal 2, coveralls.strong_count + + # Reference cleanup includes the cached most recent exception. + coveralls = nil + GC.start + assert_equal 0, Coverall.get_num_alive + + end end diff --git a/fixtures/coverall/tests/bindings/test_coverall.swift b/fixtures/coverall/tests/bindings/test_coverall.swift new file mode 100644 index 0000000000..cc90cba76c --- /dev/null +++ b/fixtures/coverall/tests/bindings/test_coverall.swift @@ -0,0 +1,64 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import coverall + +// TODO: use an actual test runner. + +// Test some_dict(). +do { + let d = createSomeDict() + assert(d.text == "text"); + assert(d.maybeText == "maybe_text"); + assert(d.aBool); + assert(d.maybeABool == false); + assert(d.unsigned8 == 1) + assert(d.maybeUnsigned8 == 2) + assert(d.unsigned64 == 18446744073709551615) + assert(d.maybeUnsigned64 == 0) + assert(d.signed8 == 8) + assert(d.maybeSigned8 == 0) + assert(d.signed64 == 9223372036854775807) + assert(d.maybeSigned64 == 0) + // floats should be "close enough". + //fun Float.almostEquals(other: Float) = Math.abs(this - other) < 0.000001 + //fun Double.almostEquals(other: Double) = Math.abs(this - other) < 0.000001 + + //assert(d.float32.almostEquals(1.2345F)) + //assert(d.maybeFloat32!!.almostEquals(22.0F/7.0F)) + //assert(d.float64.almostEquals(0.0)) + //assert(d.maybeFloat64!!.almostEquals(1.0)) + + assert(d.coveralls!.getName() == "some_dict") +} + +// Test arcs. +do { + let coveralls: Coveralls = Coveralls(name: "test_arcs") + assert(getNumAlive() == 1); + // One ref held by the foreign-language code, one created for this method call. + assert(coveralls.strongCount() == 2); + assert(coveralls.getOther() == nil); + coveralls.takeOther(other: coveralls); + // Should now be a new strong ref, held by the object's reference to itself. + assert(coveralls.strongCount() == 3); + // But the same number of instances. + assert(getNumAlive() == 1); + // It's the same Rust object. + assert(coveralls.getOther()!.getName() == "test_arcs") + // TODO: ideally, it would return the same Swift-level object. + // assert(coveralls.getOther()!! === coveralls); + do { + try coveralls.takeOtherFallible() + fatalError("Should have thrown") + } catch CoverallError.TooManyHoles { + // It's okay! + } + // TODO: kinda hard to test this, as it triggers a fatal error. + // coveralls!.takeOtherPanic(message: "expected panic: with an arc!") + coveralls.takeOther(other: nil); + assert(coveralls.strongCount() == 2); +} +// Swift GC is deterministic, `coveralls` is freed when it goes out of scope. +assert(getNumAlive() == 0); diff --git a/fixtures/coverall/tests/test_generated_bindings.rs b/fixtures/coverall/tests/test_generated_bindings.rs index 4157820015..4786c8e656 100644 --- a/fixtures/coverall/tests/test_generated_bindings.rs +++ b/fixtures/coverall/tests/test_generated_bindings.rs @@ -3,6 +3,7 @@ uniffi_macros::build_foreign_language_testcases!( [ "tests/bindings/test_coverall.py", "tests/bindings/test_coverall.kts", + "tests/bindings/test_coverall.swift", "tests/bindings/test_coverall.rb" ] ); diff --git a/uniffi/Cargo.toml b/uniffi/Cargo.toml index a6d1f33d5e..c21256f97a 100644 --- a/uniffi/Cargo.toml +++ b/uniffi/Cargo.toml @@ -14,7 +14,8 @@ keywords = ["ffi", "bindgen"] # Re-exported dependencies used in generated Rust scaffolding files. anyhow = "1" bytes = "1.0" -ffi-support = "~0.4.2" +#ffi-support = "~0.4.2" +ffi-support = { git = "https://github.com/mozilla/ffi-support", branch="intoffi-c-void" } lazy_static = "1.4" log = "0.4" # Regular dependencies diff --git a/uniffi/src/ffi/handle_maps.rs b/uniffi/src/ffi/handle_maps.rs deleted file mode 100644 index a538ea79b6..0000000000 --- a/uniffi/src/ffi/handle_maps.rs +++ /dev/null @@ -1,363 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -use ffi_support::{ExternError, Handle, HandleError, HandleMap, IntoFfi}; -use std::sync::{Arc, RwLock}; - -/// `ArcHandleMap` is a relatively thin wrapper around `RwLock>>`. -/// This is only suitable for objects that implement `Sync` and `Send` and is -/// for objects that are able to look after their own locking, and need to be called -/// from multiple foreign language threads at the same time. -/// -/// This is in contrast to the `ConcurrentHandleMap` struct from `ffi-support`, which -/// allows objects with `mut` methods but which only lets the objects be accessed from one -/// thread at a time. -/// -// Some care is taken to protect that handlemap itself being read and written concurrently, and -// that the lock is held for the least amount of time; however, if it is ever poisoned, it will -// panic on both read and write. -pub struct ArcHandleMap -where - T: Sync + Send, -{ - /// The underlying map. Public so that more advanced use-cases - /// may use it as they please. - pub map: RwLock>>, -} - -impl ArcHandleMap { - /// Construct a new `ArcHandleMap`. - pub fn new() -> Self { - ArcHandleMap { - map: RwLock::new(HandleMap::new()), - } - } - - /// Get the number of entries in the `ArcHandleMap`. - /// - /// This takes the map's `read` lock. - #[inline] - pub fn len(&self) -> usize { - let map = self.map.read().unwrap(); - map.len() - } - - /// Returns true if the `ArcHandleMap` is empty. - /// - /// This takes the map's `read` lock. - #[inline] - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Insert an item into the map, returning the newly allocated handle to the - /// item. - /// - /// This takes the map's `write` lock which the object is inserted. - pub fn insert(&self, v: T) -> Handle { - let mut map = self.map.write().unwrap(); - map.insert(Arc::new(v)) - } - - /// Remove an item from the map. - /// - /// This takes the map's `write` lock while removing from the map, but - /// not while the object is being dropped. - pub fn delete(&self, h: Handle) -> Result<(), HandleError> { - // We use `remove` and not delete (and use the inner block) to ensure - // that if `v`'s destructor panics, we aren't holding the write lock - // when it happens, so that the map itself doesn't get poisoned. - let v = { - let mut map = self.map.write().unwrap(); - map.remove(h) - }; - v.map(drop) - } - - /// Convenient wrapper for `delete` which takes a `u64` that it will - /// convert to a handle. - /// - /// The main benefit (besides convenience) of this over the version - /// that takes a [`Handle`] is that it allows handling handle-related errors - /// in one place. - pub fn delete_u64(&self, h: u64) -> Result<(), HandleError> { - self.delete(Handle::from_u64(h)?) - } - - /// Remove an item from the map, returning either the item, - /// or None if its guard mutex got poisoned at some point. - /// - /// This takes the map's `write` lock, and unwraps the `Arc`. - pub fn remove(&self, h: Handle) -> Result, HandleError> { - let mut map = self.map.write().unwrap(); - let arc = map.remove(h)?; - match Arc::try_unwrap(arc) { - Ok(obj) => Ok(Some(obj)), - _ => Ok(None), - } - } - - /// Convenient wrapper for `remove` which takes a `u64` that it will - /// convert to a handle. - /// - /// The main benefit (besides convenience) of this over the version - /// that takes a [`Handle`] is that it allows handling handle-related errors - /// in one place. - pub fn remove_u64(&self, h: u64) -> Result, HandleError> { - self.remove(Handle::from_u64(h)?) - } - - /// Call `callback` with a non-mutable reference to the item from the map, - /// after acquiring the necessary locks. - /// - /// This takes the map's `read` lock for as long as needed to clone the inner `Arc`. - /// This is so the lock isn't held while the callback is in use. - pub fn get(&self, h: Handle, callback: F) -> Result - where - F: FnOnce(&T) -> Result, - E: From, - { - let obj = { - let map = self.map.read().unwrap(); - let obj = map.get(h)?; - Arc::clone(&obj) - }; - callback(&*obj) - } - - /// Convenient wrapper for `get` which takes a `u64` that it will convert to - /// a handle. - /// - /// The other benefit (besides convenience) of this over the version - /// that takes a [`Handle`] is that it allows handling handle-related errors - /// in one place. - /// - /// This takes the map's `read` lock for as long as needed to clone the inner `Arc`. - /// This is so the lock isn't held while the callback is in use. - pub fn get_u64(&self, u: u64, callback: F) -> Result - where - F: FnOnce(&T) -> Result, - E: From, - { - self.get(Handle::from_u64(u)?, callback) - } - - /// Helper that performs both a [`call_with_result`] and [`get`](ArcHandleMap::get). - /// - /// The callback is called with a clone of the inner `Arc`, returning a `Result`. - /// - /// This takes the map's `read` lock for as long as needed to clone the inner `Arc`. - /// This is so the lock isn't held while the callback is in use. - /// - /// This is the most general of the `call_` helper methods; all other call helpers - /// can be implemented in terms of this one. - pub fn call_by_arc_with_result( - &self, - out_error: &mut ExternError, - h: u64, - callback: F, - ) -> R::Value - where - F: std::panic::UnwindSafe + FnOnce(Arc) -> Result, - ExternError: From, - R: IntoFfi, - { - use ffi_support::call_with_result; - call_with_result(out_error, || -> Result<_, ExternError> { - // We can't reuse `get` here because it would require E: - // From, which is inconvenient... - let h = Handle::from_u64(h)?; - let obj_arc = { - let map = self.map.read().unwrap(); - let obj = map.get(h)?; - Arc::clone(&obj) - }; - Ok(callback(obj_arc)?) - }) - } - - /// Helper that performs both a [`call_with_result`] and [`get`](ArcHandleMap::get). - /// - /// The callback is called with a reference to the contained struct, returning a `Result`. - /// - /// This takes the map's `read` lock for as long as needed to clone the inner `Arc`. - /// This is so the lock isn't held while the callback is in use. - pub fn call_with_result( - &self, - out_error: &mut ExternError, - h: u64, - callback: F, - ) -> R::Value - where - F: std::panic::UnwindSafe + FnOnce(&T) -> Result, - ExternError: From, - R: IntoFfi, - { - self.call_by_arc_with_result(out_error, h, |obj_arc: Arc| -> Result<_, E> { - let obj = &*obj_arc; - callback(obj) - }) - } - - /// Helper that performs both a [`call_with_output`] and [`get`](ArcHandleMap::get). - /// - /// The callback is called with a clone of the inner `Arc`, returning a value. - pub fn call_by_arc_with_output( - &self, - out_error: &mut ExternError, - h: u64, - callback: F, - ) -> R::Value - where - F: std::panic::UnwindSafe + FnOnce(Arc) -> R, - R: IntoFfi, - { - self.call_by_arc_with_result(out_error, h, |obj_arc: Arc| -> Result<_, HandleError> { - Ok(callback(obj_arc)) - }) - } - - /// Helper that performs both a [`call_with_output`] and [`get`](ArcHandleMap::get). - /// - /// The callback is called with a reference to the contained struct, returning a value. - pub fn call_with_output( - &self, - out_error: &mut ExternError, - h: u64, - callback: F, - ) -> R::Value - where - F: std::panic::UnwindSafe + FnOnce(&T) -> R, - R: IntoFfi, - { - self.call_with_result(out_error, h, |obj| -> Result<_, HandleError> { - Ok(callback(obj)) - }) - } - - /// Use `constructor` to create and insert a `T`, while inside a - /// [`call_with_result`] call (to handle panics and map errors onto an - /// `ExternError`). - /// - /// This takes the map's `write` lock for as long as needed to insert into the map. - /// This is so the lock isn't held while the constructor is being called. - pub fn insert_with_result(&self, out_error: &mut ExternError, constructor: F) -> u64 - where - F: std::panic::UnwindSafe + FnOnce() -> Result, - ExternError: From, - { - use ffi_support::call_with_result; - call_with_result(out_error, || -> Result<_, ExternError> { - // Note: it's important that we don't call the constructor while - // we're holding the write lock, because we don't want to poison - // the entire map if it panics! - let to_insert = constructor()?; - Ok(self.insert(to_insert)) - }) - } - - /// Equivalent to - /// [`insert_with_result`](ArcHandleMap::insert_with_result) for the - /// case where the constructor cannot produce an error. - /// - /// The name is somewhat dubious, since there's no `output`, but it's intended to make it - /// clear that it contains a [`call_with_output`] internally. - pub fn insert_with_output(&self, out_error: &mut ExternError, constructor: F) -> u64 - where - F: std::panic::UnwindSafe + FnOnce() -> T, - { - // The Err type isn't important here beyond being convertable to ExternError - self.insert_with_result(out_error, || -> Result<_, HandleError> { - Ok(constructor()) - }) - } -} - -impl Default for ArcHandleMap { - #[inline] - fn default() -> Self { - Self::new() - } -} - -/// Tests that check our behavior when panicking. -/// -/// Naturally these require panic=unwind, which means we can't run them when -/// generating coverage (well, `-Zprofile`-based coverage can't -- although -/// ptrace-based coverage like tarpaulin can), and so we turn them off. -/// -/// (For clarity, `cfg(coverage)` is not a standard thing. We add it in -/// `automation/emit_coverage_info.sh`, and you can force it by adding -/// "--cfg coverage" to your RUSTFLAGS manually if you need to do so). -/// -/// Note: these tests are derived directly from ffi_support::ConcurrentHandleMap. -#[cfg(not(coverage))] -#[allow(unused_imports)] -mod panic_tests { - use super::ArcHandleMap; - use ffi_support::{call_with_result, ErrorCode, ExternError}; - - #[derive(PartialEq, Debug)] - pub(super) struct Foobar(usize); - - struct PanicOnDrop(()); - impl Drop for PanicOnDrop { - fn drop(&mut self) { - panic!("intentional panic (drop)"); - } - } - - #[test] - fn test_panicking_drop() { - let map = ArcHandleMap::new(); - let h = map.insert(PanicOnDrop(())).into_u64(); - let mut e = ExternError::success(); - call_with_result(&mut e, || map.delete_u64(h)); - assert_eq!(e.get_code(), ErrorCode::PANIC); - let _ = unsafe { e.get_and_consume_message() }; - assert!(!map.map.is_poisoned()); - let inner = map.map.read().unwrap(); - assert_eq!(inner.len(), 0); - } - - #[test] - fn test_panicking_call_with() { - let map = ArcHandleMap::new(); - let h = map.insert(Foobar(0)).into_u64(); - let mut e = ExternError::success(); - map.call_with_output(&mut e, h, |_thing| { - panic!("intentional panic (call_with_output)"); - }); - - assert_eq!(e.get_code(), ErrorCode::PANIC); - let _ = unsafe { e.get_and_consume_message() }; - { - assert!(!map.map.is_poisoned()); - let inner = map.map.read().unwrap(); - assert_eq!(inner.len(), 1); - } - assert!(map.delete_u64(h).is_ok()); - assert!(!map.map.is_poisoned()); - let inner = map.map.read().unwrap(); - assert_eq!(inner.len(), 0); - } - - #[test] - fn test_panicking_insert_with() { - let map = ArcHandleMap::new(); - let mut e = ExternError::success(); - let res = map.insert_with_output(&mut e, || { - panic!("intentional panic (insert_with_output)"); - }); - - assert_eq!(e.get_code(), ErrorCode::PANIC); - let _ = unsafe { e.get_and_consume_message() }; - - assert_eq!(res, 0); - - assert!(!map.map.is_poisoned()); - let inner = map.map.read().unwrap(); - assert_eq!(inner.len(), 0); - } -} diff --git a/uniffi/src/ffi/mod.rs b/uniffi/src/ffi/mod.rs index 3edf2eb4a8..03c12fabcb 100644 --- a/uniffi/src/ffi/mod.rs +++ b/uniffi/src/ffi/mod.rs @@ -4,10 +4,8 @@ pub mod foreignbytes; pub mod foreigncallbacks; -pub mod handle_maps; pub mod rustbuffer; pub use foreignbytes::*; pub use foreigncallbacks::*; -pub use handle_maps::*; pub use rustbuffer::*; diff --git a/uniffi/src/lib.rs b/uniffi/src/lib.rs index 7d87636e7d..88bb535fcc 100644 --- a/uniffi/src/lib.rs +++ b/uniffi/src/lib.rs @@ -49,7 +49,6 @@ pub mod deps { pub use anyhow; pub use bytes; pub use ffi_support; - pub use lazy_static; pub use log; pub use static_assertions; } @@ -566,6 +565,67 @@ unsafe impl ViaFfi for HashMap { } } +/// Support for passing reference-counted shared objects via the FFI. +/// +/// To avoid dealing with complex lifetime semantics over the FFI, any data passed +/// by reference must be encapsulated in an `Arc`, and must be safe to share +/// across threads. +unsafe impl ViaFfi for std::sync::Arc { + // Don't use a pointer to as that requires a `pub ` + type FfiType = *const std::os::raw::c_void; + + /// When lowering, we have an owned `Arc` and we transfer that ownership + /// to the foreign-language code, "leaking" it out of Rust's ownership system + /// as a raw pointer. This works safely because we have unique ownership of `self`. + /// The foreign-language code is responsible for freeing this by calling the + /// `ffi_object_free` FFI function provided by the corresponding UniFFI type. + /// + /// Safety: when freeing the resulting pointer, the foreign-language code must + /// call the destructor function specific to the type `T`. Calling the destructor + /// function for other types may lead to undefined behaviour. + fn lower(self) -> Self::FfiType { + std::sync::Arc::into_raw(self) as Self::FfiType + } + + /// When lifting, we receive a "borrow" of the `Arc` that is owned by + /// the foreign-language code, and make a clone of it for our own use. + /// + /// Safety: the provided value must be a pointer previously obtained by calling + /// the `lower()` or `write()` method of this impl. + fn try_lift(v: Self::FfiType) -> Result { + let v = v as *const T; + // We musn't drop the `Arc` that is owned by the foreign-language code. + let foreign_arc = std::mem::ManuallyDrop::new(unsafe { Self::from_raw(v) }); + // Take a clone for our own use. + Ok(std::sync::Arc::clone(&*foreign_arc)) + } + + /// When writing as a field of a complex structure, make a clone and transfer ownership + /// of it to the foreign-language code by writing its pointer into the buffer. + /// The foreign-language code is responsible for freeing this by calling the + /// `ffi_object_free` FFI function provided by the corresponding UniFFI type. + /// + /// Safety: when freeing the resulting pointer, the foreign-language code must + /// call the destructor function specific to the type `T`. Calling the destructor + /// function for other types may lead to undefined behaviour. + fn write(&self, buf: &mut B) { + static_assertions::const_assert!(std::mem::size_of::<*const std::ffi::c_void>() <= 8); + let ptr = std::sync::Arc::clone(self).lower(); + buf.put_u64(ptr as u64); + } + + /// When reading as a field of a complex structure, we receive a "borrow" of the `Arc` + /// that is owned by the foreign-language code, and make a clone for our own use. + /// + /// Safety: the buffer must contain a pointer previously obtained by calling + /// the `lower()` or `write()` method of this impl. + fn try_read(buf: &mut B) -> Result { + static_assertions::const_assert!(std::mem::size_of::<*const std::ffi::c_void>() <= 8); + check_remaining(buf, 8)?; + Self::try_lift(buf.get_u64() as Self::FfiType) + } +} + #[cfg(test)] mod test { use super::*; diff --git a/uniffi_bindgen/src/bindings/gecko_js/gen_gecko_js.rs b/uniffi_bindgen/src/bindings/gecko_js/gen_gecko_js.rs index dbf2da9cc3..6a3295f4c1 100644 --- a/uniffi_bindgen/src/bindings/gecko_js/gen_gecko_js.rs +++ b/uniffi_bindgen/src/bindings/gecko_js/gen_gecko_js.rs @@ -332,6 +332,7 @@ mod filters { FFIType::Float32 => "float".into(), FFIType::Float64 => "double".into(), FFIType::RustCString => "const char*".into(), + FFIType::RustArcPtr => unimplemented!("object pointers are not implemented"), FFIType::RustBuffer => context.ffi_rustbuffer_type(), FFIType::RustError => context.ffi_rusterror_type(), FFIType::ForeignBytes => context.ffi_foreignbytes_type(), diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin.rs index c62098fc10..c43de0efe1 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin.rs @@ -114,6 +114,8 @@ mod filters { FFIType::Float32 => "Float".to_string(), FFIType::Float64 => "Double".to_string(), FFIType::RustCString => "Pointer".to_string(), + // XXX - make this `Pointer` (but things like Helpers.kt need upgrading in non-obvious ways. ) + FFIType::RustArcPtr => "Long".to_string(), FFIType::RustBuffer => "RustBuffer.ByValue".to_string(), FFIType::RustError => "RustError".to_string(), FFIType::ForeignBytes => "ForeignBytes.ByValue".to_string(), diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt index b5b55b1095..332a881a2a 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/EnumTemplate.kt @@ -35,7 +35,7 @@ enum class {{ e.name()|class_name_kt }} { {% else %} -sealed class {{ e.name()|class_name_kt }} { +sealed class {{ e.name()|class_name_kt }}{% if e.contains_object_references(ci) %}: Destroyable {% endif %} { {% for variant in e.variants() -%} {% if !variant.has_fields() -%} object {{ variant.name()|class_name_kt }} : {{ e.name()|class_name_kt }}() @@ -78,11 +78,28 @@ sealed class {{ e.name()|class_name_kt }} { buf.putInt({{ loop.index }}) {% for field in variant.fields() -%} {{ "(this.{})"|format(field.name())|write_kt("buf", field.type_()) }} - {% endfor -%} + {% endfor %} } {%- endfor %} }.let { /* this makes the `when` an expression, which ensures it is exhaustive */ } } + + {% if e.contains_object_references(ci) %} + @Suppress("UNNECESSARY_SAFE_CALL") // codegen is much simpler if we unconditionally emit safe calls here + override fun destroy() { + when(this) { + {%- for variant in e.variants() %} + is {{ e.name()|class_name_kt }}.{{ variant.name()|class_name_kt }} -> { + {% for field in variant.fields() -%} + {%- if ci.type_contains_object_references(field.type_()) -%} + this.{{ field.name() }}?.destroy() + {% endif -%} + {%- endfor %} + } + {%- endfor %} + }.let { /* this makes the `when` an expression, which ensures it is exhaustive */ } + } + {% endif %} } {% endif %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt index d4e8a85307..20de7438b5 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt @@ -2,10 +2,34 @@ // This would be a good candidate for isolating in its own ffi-support lib. {% if ci.iter_object_definitions().len() > 0 %} + +// Interface implemented by anything that can contain an object reference. +// +// Such types expose a `destroy()` method that must be called to cleanly +// dispose of the contained objects. Failure to call this method may result +// in memory leaks. +// +// The easiest way to ensure this method is called is to use the `.use` +// helper method to execute a block and destroy the object at the end. +interface Destroyable { + fun destroy() +} + +inline fun T.use(block: (T) -> R) = + try { + block(this) + } finally { + try { + this?.destroy() + } catch (e: Throwable) { + // swallow + } + } + abstract class FFIObject( private val handle: AtomicLong -) { - open fun destroy() { +) : Destroyable { + override fun destroy() { this.handle.set(0L) } @@ -18,17 +42,6 @@ abstract class FFIObject( } } } - -inline fun T.use(block: (T) -> R) = - try { - block(this) - } finally { - try { - this.destroy() - } catch (e: Throwable) { - // swallow - } - } {% endif %} {% if ci.iter_callback_interface_definitions().len() > 0 %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index 90e9fd4074..730227b4da 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -40,6 +40,14 @@ class {{ obj.name()|class_name_kt }}( } } + internal fun lower(): Long { + callWithHandle { ptr -> return ptr } + } + + internal fun write(buf: RustBufferBuilder) { + buf.putLong(this.lower()) + } + {% for meth in obj.methods() -%} {%- match meth.return_type() -%} @@ -59,12 +67,19 @@ class {{ obj.name()|class_name_kt }}( {% endmatch %} {% endfor %} - {% if obj.constructors().len() > 1 -%} companion object { + internal fun lift(ptr: Long): {{ obj.name()|class_name_kt }} { + return {{ obj.name()|class_name_kt }}(ptr) + // TODO: identity map to smoosh clones together + } + + internal fun read(buf: ByteBuffer): {{ obj.name()|class_name_kt }} { + return {{ obj.name()|class_name_kt }}(buf.getLong()) + } + {% for cons in obj.alternate_constructors() -%} fun {{ cons.name()|fn_name_kt }}({% call kt::arg_list_decl(cons) %}): {{ obj.name()|class_name_kt }} = {{ obj.name()|class_name_kt }}({% call kt::to_ffi_call(cons) %}) {% endfor %} } - {%- endif %} } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/RecordTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/RecordTemplate.kt index 2455db2abf..ce6992850c 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/RecordTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/RecordTemplate.kt @@ -7,9 +7,8 @@ data class {{ rec.name()|class_name_kt }} ( {%- endmatch -%} {% if !loop.last %}, {% endif %} {%- endfor %} -) { +) {% if rec.contains_object_references(ci) %}: Destroyable {% endif %}{ companion object { - // XXX TODO: put this in a superclass maybe? internal fun lift(rbuf: RustBuffer.ByValue): {{ rec.name()|class_name_kt }} { return liftFromRustBuffer(rbuf) { buf -> {{ rec.name()|class_name_kt }}.read(buf) } } @@ -30,6 +29,17 @@ data class {{ rec.name()|class_name_kt }} ( internal fun write(buf: RustBufferBuilder) { {%- for field in rec.fields() %} {{ "(this.{})"|format(field.name())|write_kt("buf", field.type_()) }} + {% endfor %} + } + + {% if rec.contains_object_references(ci) %} + @Suppress("UNNECESSARY_SAFE_CALL") // codegen is much simpler if we unconditionally emit safe calls here + override fun destroy() { + {% for field in rec.fields() %} + {%- if ci.type_contains_object_references(field.type_()) -%} + this.{{ field.name() }}?.destroy() + {% endif -%} {%- endfor %} } -} \ No newline at end of file + {% endif %} +} diff --git a/uniffi_bindgen/src/bindings/python/gen_python.rs b/uniffi_bindgen/src/bindings/python/gen_python.rs index e41f67c2c0..8fa00c5259 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python.rs @@ -73,6 +73,7 @@ mod filters { FFIType::Float32 => "ctypes.c_float".to_string(), FFIType::Float64 => "ctypes.c_double".to_string(), FFIType::RustCString => "ctypes.c_voidp".to_string(), + FFIType::RustArcPtr => "ctypes.c_voidp".to_string(), FFIType::RustBuffer => "RustBuffer".to_string(), FFIType::RustError => "ctypes.POINTER(RustError)".to_string(), FFIType::ForeignBytes => "ForeignBytes".to_string(), @@ -204,7 +205,7 @@ mod filters { Type::Float32 | Type::Float64 => format!("float({})", nm), Type::Boolean => format!("(True if {} else False)", nm), Type::String => format!("{}.consumeIntoString()", nm), - Type::Object(_) => panic!("No support for lifting objects, yet"), + Type::Object(classname) => format!("{}._get_or_make_instance_({})", classname, nm), Type::CallbackInterface(_) => panic!("No support for lifting callback interfaces, yet"), Type::Error(_) => panic!("No support for lowering errors, yet"), Type::Enum(_) diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index cec5a695c0..c5e6e09fb9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -1,9 +1,15 @@ class {{ obj.name()|class_name_py }}(object): + _instances = WeakValueDictionary() {%- match obj.primary_constructor() %} {%- when Some with (cons) %} def __init__(self, {% call py::arg_list_decl(cons) -%}): {%- call py::coerce_args_extra_indent(cons) %} - self._handle = {% call py::to_ffi_call(cons) %}\ + self._handle = {% call py::to_ffi_call(cons) %} + {# A constructor, by definition, returns a new object. If we really + wanted to be thorough, we could consider asserting the handle isn't + already in the map, but that doesn't seem to offer much value. + #} + self.__class__._instances[self._handle] = self {%- when None %} {%- endmatch %} @@ -14,17 +20,37 @@ def __del__(self): self._handle ) - {% for cons in obj.alternate_constructors() -%} + # Used by alternative constructors or any methods which return this type. + # May return an existing instance, or create a new one if we can't find one. @classmethod - def {{ cons.name()|fn_name_py }}(cls, {% call py::arg_list_decl(cons) %}): - {%- call py::coerce_args_extra_indent(cons) %} - # Call the (fallible) function before creating any half-baked object instances. - handle = {% call py::to_ffi_call(cons) %} + def _get_or_make_instance_(cls, handle): + {# Look in our weak map for an instance already associated with this + pointer. If it exists we can return it, but the pointer we have + is a clone of an `Arc<>` so has a reference we need to destroy. + #} + existing = cls._instances.get(handle) + if existing is not None: + rust_call_with_error( + InternalError, + _UniFFILib.{{ obj.ffi_object_free().name() }}, + handle + ) + return existing + # Lightly yucky way to bypass the usual __init__ logic # and just create a new instance with the required handle. inst = cls.__new__(cls) inst._handle = handle + cls._instances[handle] = inst return inst + + {% for cons in obj.alternate_constructors() -%} + @classmethod + def {{ cons.name()|fn_name_py }}(cls, {% call py::arg_list_decl(cons) %}): + {%- call py::coerce_args_extra_indent(cons) %} + # Call the (fallible) function before creating any half-baked object instances. + handle = {% call py::to_ffi_call(cons) %} + return cls._get_or_make_instance_(handle) {% endfor %} {% for meth in obj.methods() -%} diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py index c4a74bff91..e7aeecb5ce 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferBuilder.py @@ -133,10 +133,12 @@ def write{{ canonical_type_name }}(self, v): {% when Type::Object with (object_name) -%} # The Object type {{ object_name }}. - # Objects cannot currently be serialized, but we can produce a helpful error. + # We write the pointer value directly - what could possibly go wrong? - def write{{ canonical_type_name }}(self): - raise InternalError("RustBufferStream.write() not implemented yet for {{ canonical_type_name }}") + def write{{ canonical_type_name }}(self, v): + if not isinstance(v, {{ object_name|class_name_py }}): + raise TypeError("Expected {{ object_name|class_name_py }} instance, {} found".format(v.__class__.__name__)) + self.writeU64(v._handle) # really should stop uncondionally using u64! {% when Type::CallbackInterface with (object_name) -%} # The Callback Interface type {{ object_name }}. diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py index 6daaca4309..a63e9e3e21 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferStream.py @@ -129,10 +129,10 @@ def read{{ canonical_type_name }}(self): {% when Type::Object with (object_name) -%} # The Object type {{ object_name }}. - # Objects cannot currently be serialized, but we can produce a helpful error. def read{{ canonical_type_name }}(self): - raise InternalError("RustBufferStream.read not implemented yet for {{ canonical_type_name }}") + handle = self._unpack_from(8, ">Q") + return {{ object_name|class_name_py }}._get_or_make_instance_(handle) {% when Type::CallbackInterface with (object_name) -%} # The Callback Interface type {{ object_name }}. diff --git a/uniffi_bindgen/src/bindings/python/templates/wrapper.py b/uniffi_bindgen/src/bindings/python/templates/wrapper.py index 606884b65a..3e1c606d24 100644 --- a/uniffi_bindgen/src/bindings/python/templates/wrapper.py +++ b/uniffi_bindgen/src/bindings/python/templates/wrapper.py @@ -20,6 +20,7 @@ import struct import contextlib import datetime +from weakref import WeakValueDictionary {% include "RustBufferTemplate.py" %} {% include "RustBufferStream.py" %} diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby.rs index 620a44560a..13b78506c6 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby.rs @@ -93,6 +93,7 @@ mod filters { FFIType::Float32 => ":float".to_string(), FFIType::Float64 => ":double".to_string(), FFIType::RustCString => ":string".to_string(), + FFIType::RustArcPtr => ":pointer".to_string(), FFIType::RustBuffer => "RustBuffer.by_value".to_string(), FFIType::RustError => "RustError.by_ref".to_string(), FFIType::ForeignBytes => "ForeignBytes".to_string(), @@ -210,7 +211,7 @@ mod filters { Type::String => format!("RustBuffer.allocFromString({})", nm), Type::Timestamp => panic!("No support for timestamps in Ruby, yet"), Type::Duration => panic!("No support for durations in Ruby, yet"), - Type::Object(_) => format!("({}._handle)", nm), + Type::Object(_) => format!("({}.pointer)", nm), Type::CallbackInterface(_) => panic!("No support for lowering callback interfaces yet"), Type::Error(_) => panic!("No support for lowering errors, yet"), Type::Enum(_) @@ -240,7 +241,7 @@ mod filters { Type::String => format!("{}.consumeIntoString", nm), Type::Timestamp => panic!("No support for timestamps in Ruby, yet"), Type::Duration => panic!("No support for durations in Ruby, yet"), - Type::Object(_) => panic!("No support for lifting objects, yet"), + Type::Object(name) => format!("{}({})", class_name_rb(name)?, nm), Type::CallbackInterface(_) => panic!("No support for lifting callback interfaces, yet"), Type::Error(_) => panic!("No support for lowering errors, yet"), Type::Enum(_) diff --git a/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb index af47374a89..3914278fac 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb @@ -1,22 +1,40 @@ class {{ obj.name()|class_name_rb }} - {%- match obj.primary_constructor() %} - {%- when Some with (cons) %} - def initialize({% call rb::arg_list_decl(cons) -%}) - {%- call rb::coerce_args_extra_indent(cons) %} - handle = {% call rb::to_ffi_call(cons) %} - ObjectSpace.define_finalizer(self, self.class.define_finalizer_by_handle(handle)) - @handle = handle + + # A private helper for initializing instances of the class from a raw pointer, + # bypassing any initialization logic and ensuring they are GC'd properly. + def self.uniffi_allocate(pointer) + pointer.autorelease = false + inst = allocate + inst.instance_variable_set :@pointer, pointer + ObjectSpace.define_finalizer(inst, uniffi_define_finalizer_by_pointer(pointer, inst.object_id)) + return inst end - def {{ obj.name()|class_name_rb }}.define_finalizer_by_handle(handle) + # A private helper for registering an object finalizer. + # N.B. it's important that this does not capture a reference + # to the actual instance, only its underlying pointer. + def self.uniffi_define_finalizer_by_pointer(pointer, object_id) Proc.new do |_id| {{ ci.namespace()|class_name_rb }}.rust_call_with_error( InternalError, :{{ obj.ffi_object_free().name() }}, - handle + pointer ) end end + + def uniffi_pointer + return @pointer + end + + {%- match obj.primary_constructor() %} + {%- when Some with (cons) %} + def initialize({% call rb::arg_list_decl(cons) -%}) + {%- call rb::coerce_args_extra_indent(cons) %} + pointer = {% call rb::to_ffi_call(cons) %} + @pointer = pointer + ObjectSpace.define_finalizer(self, self.class.uniffi_define_finalizer_by_pointer(pointer, self.object_id)) + end {%- when None %} {%- endmatch %} @@ -25,11 +43,8 @@ def self.{{ cons.name()|fn_name_rb }}({% call rb::arg_list_decl(cons) %}) {%- call rb::coerce_args_extra_indent(cons) %} # Call the (fallible) function before creating any half-baked object instances. # Lightly yucky way to bypass the usual "initialize" logic - # and just create a new instance with the required handle. - inst = allocate - inst.instance_variable_set :@handle, {% call rb::to_ffi_call(cons) %} - - return inst + # and just create a new instance with the required pointer. + return uniffi_allocate({% call rb::to_ffi_call(cons) %}) end {% endfor %} @@ -39,14 +54,14 @@ def self.{{ cons.name()|fn_name_rb }}({% call rb::arg_list_decl(cons) %}) {%- when Some with (return_type) -%} def {{ meth.name()|fn_name_rb }}({% call rb::arg_list_decl(meth) %}) {%- call rb::coerce_args_extra_indent(meth) %} - result = {% call rb::to_ffi_call_with_prefix("@handle", meth) %} + result = {% call rb::to_ffi_call_with_prefix("@pointer", meth) %} return {{ "result"|lift_rb(return_type) }} end {%- when None -%} def {{ meth.name()|fn_name_rb }}({% call rb::arg_list_decl(meth) %}) {%- call rb::coerce_args_extra_indent(meth) %} - {% call rb::to_ffi_call_with_prefix("@handle", meth) %} + {% call rb::to_ffi_call_with_prefix("@pointer", meth) %} end {% endmatch %} {% endfor %} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb index 530b833580..4f330dfc3d 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb @@ -123,10 +123,9 @@ def write_{{ canonical_type_name }} {% when Type::Object with (object_name) -%} # The Object type {{ object_name }}. - # Objects cannot currently be serialized, but we can produce a helpful error. - def write_{{ canonical_type_name }} - raise InternalError('RustBufferStream.write() not implemented yet for {{ canonical_type_name }}') + def write_{{ canonical_type_name }}(obj) + pack_into(8, 'Q>', obj.uniffi_pointer.address) end {% when Type::CallbackInterface with (object_name) -%} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb index 9e6e700398..59fca13757 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb @@ -124,10 +124,10 @@ def read{{ canonical_type_name }} {% when Type::Object with (object_name) -%} # The Object type {{ object_name }}. - # Objects cannot currently be serialized, but we can produce a helpful error. def read{{ canonical_type_name }} - raise InternalError, 'RustBufferStream.read not implemented yet for {{ canonical_type_name }}' + pointer = FFI::Pointer.new unpack_from 8, 'Q>' + return {{ object_name|class_name_rb }}.uniffi_allocate(pointer) end {% when Type::CallbackInterface with (object_name) -%} diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift.rs b/uniffi_bindgen/src/bindings/swift/gen_swift.rs index 6adde71a98..8d0f264e67 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift.rs @@ -166,6 +166,7 @@ mod filters { FFIType::Float32 => "float".into(), FFIType::Float64 => "double".into(), FFIType::RustCString => "const char*_Nonnull".into(), + FFIType::RustArcPtr => "void*_Nonnull".into(), FFIType::RustBuffer => "RustBuffer".into(), FFIType::RustError => "NativeRustError".into(), FFIType::ForeignBytes => "ForeignBytes".into(), diff --git a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift index 4532fce681..0ad77b379f 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift @@ -16,6 +16,7 @@ fileprivate enum UniffiInternalError: RustError { case incompleteData case unexpectedOptionalTag case unexpectedEnumCase + case unexpectedNullPointer case emptyResult case unknown(_ message: String) @@ -25,6 +26,7 @@ fileprivate enum UniffiInternalError: RustError { case .incompleteData: return "The buffer still has data after lifting its containing value" case .unexpectedOptionalTag: return "Unexpected optional tag; should be 0 or 1" case .unexpectedEnumCase: return "Raw enum value doesn't match any cases" + case .unexpectedNullPointer: return "Raw pointer value was null" case .emptyResult: return "Unexpected nil returned from FFI function" case let .unknown(message): return "FFI function returned unknown error: \(message)" } diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index 36ddcbb8b3..ceae77f8aa 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -9,10 +9,13 @@ public protocol {{ obj.name() }}Protocol { {% endfor %} } -public class {{ obj.name()|class_name_swift }}: {{ obj.name() }}Protocol { - private let handle: UInt64 +public class {{ obj.name()|class_name_swift }}: {{ obj.name() }}Protocol, Equatable, Hashable { + private let handle: UnsafeMutableRawPointer - private init(fromRawHandle handle: UInt64) { + // TODO: We'd like this to be `private` but for Swifty reasons, + // we can't implement `ViaFfi` without making this `required` and we can't + // make it `required` without making it `public`. + required init(fromRawHandle handle: UnsafeMutableRawPointer) { self.handle = handle } @@ -30,6 +33,14 @@ public class {{ obj.name()|class_name_swift }}: {{ obj.name() }}Protocol { } } + public static func ==(lhs: {{ obj.name()|class_name_swift }}, rhs: {{ obj.name()|class_name_swift }}) -> Bool { + return lhs.handle == rhs.handle + } + + public func hash(into hasher: inout Hasher) { + hasher.combine(self.handle) + } + {% for cons in obj.alternate_constructors() %} public static func {{ cons.name()|fn_name_swift }}({% call swift::arg_list_decl(cons) %}) {% call swift::throws(cons) %} -> {{ obj.name()|class_name_swift }} { return {{ obj.name()|class_name_swift }}(fromRawHandle: {% call swift::to_ffi_call(cons) %}) @@ -52,4 +63,32 @@ public class {{ obj.name()|class_name_swift }}: {{ obj.name() }}Protocol { } {%- endmatch %} {% endfor %} -} \ No newline at end of file +} + +fileprivate extension {{ obj.name()|class_name_swift }} { + fileprivate typealias FfiType = UnsafeMutableRawPointer + + fileprivate static func read(from buf: Reader) throws -> Self { + let v: UInt64 = try buf.readInt() + let ptr = UnsafeMutableRawPointer(bitPattern: UInt(truncatingIfNeeded: v)) + if (ptr == nil) { + throw UniffiInternalError.unexpectedNullPointer + } + return try self.lift(ptr!) + } + + fileprivate func write(into buf: Writer) { + // This fiddling is because `Int` it the thing that's the same size as a pointer. + buf.writeInt(UInt64(bitPattern: Int64(Int(bitPattern: self.lower())))) + } + + fileprivate static func lift(_ handle: UnsafeMutableRawPointer) throws -> Self { + return Self(fromRawHandle: handle) + } + + fileprivate func lower() -> UnsafeMutableRawPointer { + return self.handle + } +} + +extension {{ obj.name()|class_name_swift }} : ViaFfi, Serializable {} \ No newline at end of file diff --git a/uniffi_bindgen/src/interface/attributes.rs b/uniffi_bindgen/src/interface/attributes.rs index 7ca005c3f8..fb1c4de6a4 100644 --- a/uniffi_bindgen/src/interface/attributes.rs +++ b/uniffi_bindgen/src/interface/attributes.rs @@ -192,8 +192,8 @@ impl> TryFrom> /// Represents UDL attributes that might appear on a function argument. /// -/// This supports the `[ByRef]` attribute for arguments that should be passed -/// by reference in the generated Rust scaffolding. +/// This supports the `[ByRef]` attribute for arguments that should be +/// passed by reference in the generated Rust scaffolding. #[derive(Debug, Clone, Hash, Default)] pub(super) struct ArgumentAttributes(Vec); @@ -330,7 +330,7 @@ impl MethodAttributes { }) } - pub(super) fn get_by_arc(&self) -> bool { + pub(super) fn get_self_by_arc(&self) -> bool { self.0 .iter() .any(|attr| matches!(attr, Attribute::SelfType(SelfType::ByArc))) @@ -525,23 +525,23 @@ mod test { fn test_method_attributes() { let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Throws=Error]").unwrap(); let attrs = MethodAttributes::try_from(&node).unwrap(); - assert!(!attrs.get_by_arc()); + assert!(!attrs.get_self_by_arc()); assert!(matches!(attrs.get_throws_err(), Some("Error"))); let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[]").unwrap(); let attrs = MethodAttributes::try_from(&node).unwrap(); - assert!(!attrs.get_by_arc()); + assert!(!attrs.get_self_by_arc()); assert!(attrs.get_throws_err().is_none()); let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Self=ByArc, Throws=Error]").unwrap(); let attrs = MethodAttributes::try_from(&node).unwrap(); - assert!(attrs.get_by_arc()); + assert!(attrs.get_self_by_arc()); assert!(attrs.get_throws_err().is_some()); let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Self=ByArc]").unwrap(); let attrs = MethodAttributes::try_from(&node).unwrap(); - assert!(attrs.get_by_arc()); + assert!(attrs.get_self_by_arc()); assert!(attrs.get_throws_err().is_none()); } diff --git a/uniffi_bindgen/src/interface/enum_.rs b/uniffi_bindgen/src/interface/enum_.rs index e790f1e9cf..53418f13c0 100644 --- a/uniffi_bindgen/src/interface/enum_.rs +++ b/uniffi_bindgen/src/interface/enum_.rs @@ -99,6 +99,7 @@ impl Enum { pub fn name(&self) -> &str { &self.name } + pub fn variants(&self) -> Vec<&Variant> { self.variants.iter().collect() } @@ -106,6 +107,12 @@ impl Enum { pub fn is_flat(&self) -> bool { self.flat } + + pub fn contains_object_references(&self, ci: &ComponentInterface) -> bool { + // *sigh* at the clone here, the relationship between a ComponentInterace + // and its contained types could use a bit of a cleanup. + ci.type_contains_object_references(&Type::Enum(self.name.clone())) + } } // Note that we have two `APIConverter` impls here - one for the `enum` case diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index 24b4e24380..b84c99b4c7 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -35,6 +35,10 @@ pub enum FFIType { /// If you've got one of these, you must call the appropriate rust function to free it. /// This is currently only used for error messages, and may go away in future. RustCString, + /// A `*const c_void` pointer to a rust-owned `Arc`. + /// If you've got one of these, you must call the appropriate rust function to free it. + /// The templates will generate a unique `free` function for each T. + RustArcPtr, /// A byte buffer allocated by rust, and owned by whoever currently holds it. /// If you've got one of these, you must either call the appropriate rust function to free it /// or pass it to someone that will. diff --git a/uniffi_bindgen/src/interface/function.rs b/uniffi_bindgen/src/interface/function.rs index 04ab3c77ca..49b9ea1f65 100644 --- a/uniffi_bindgen/src/interface/function.rs +++ b/uniffi_bindgen/src/interface/function.rs @@ -61,12 +61,19 @@ impl Function { pub fn name(&self) -> &str { &self.name } + pub fn arguments(&self) -> Vec<&Argument> { self.arguments.iter().collect() } + + pub fn all_arguments(&self) -> Vec { + self.arguments.iter().cloned().collect() + } + pub fn return_type(&self) -> Option<&Type> { self.return_type.as_ref() } + pub fn ffi_func(&self) -> &FFIFunction { &self.ffi_func } @@ -112,9 +119,6 @@ impl APIConverter for weedle::namespace::NamespaceMember<'_> { impl APIConverter for weedle::namespace::OperationNamespaceMember<'_> { fn convert(&self, ci: &mut ComponentInterface) -> Result { let return_type = ci.resolve_return_type_expression(&self.return_type)?; - if let Some(Type::Object(_)) = return_type { - bail!("Objects cannot currently be returned from functions"); - } Ok(Function { name: match self.identifier { None => bail!("anonymous functions are not supported {:?}", self), @@ -176,9 +180,6 @@ impl APIConverter for weedle::argument::Argument<'_> { impl APIConverter for weedle::argument::SingleArgument<'_> { fn convert(&self, ci: &mut ComponentInterface) -> Result { let type_ = ci.resolve_type_expression(&self.type_)?; - if let Type::Object(_) = type_ { - bail!("Objects cannot currently be passed as arguments"); - } let default = match self.default { None => None, Some(v) => Some(convert_default_value(&v.value, &type_)?), diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 28ecc60c5b..5c8e2d97ec 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -204,10 +204,35 @@ impl<'ci> ComponentInterface { self.errors.iter().find(|e| e.name == name) } + /// Iterate over all known types in the interface. pub fn iter_types(&self) -> Vec { self.types.iter_known_types().collect() } + /// Check whether the given type contains any (possibly nested) Type::Object references. + /// + /// This is important to know in language bindings that cannot integrate object types + /// tightly with the host GC, and hence need to perform manual descruction of objects. + pub fn type_contains_object_references(&self, type_: &Type) -> bool { + match type_ { + Type::Object(_) => true, + Type::Optional(t) | Type::Sequence(t) | Type::Map(t) => self.type_contains_object_references(t), + Type::Record(name) => { + self.get_record_definition(name).map(|rec| { + rec.fields().iter().any(|f| self.type_contains_object_references(&f.type_)) + }).unwrap_or(false) + }, + Type::Enum(name) => { + self.get_enum_definition(name).map(|e| { + e.variants().iter().any(|v| { + v.fields().iter().any(|f| self.type_contains_object_references(&f.type_)) + }) + }).unwrap_or(false) + }, + _ => false, + } + } + /// Calculate a numeric checksum for this ComponentInterface. /// /// The checksum can be used to guard against accidentally using foreign-language bindings diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index ee69facef2..b0812bf5c7 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -95,6 +95,10 @@ impl Object { &self.name } + pub fn type_(&self) -> Type { + Type::Object(self.name.clone()) + } + pub fn constructors(&self) -> Vec<&Constructor> { self.constructors.iter().collect() } @@ -127,8 +131,8 @@ impl Object { pub fn derive_ffi_funcs(&mut self, ci_prefix: &str) -> Result<()> { self.ffi_func_free.name = format!("ffi_{}_{}_object_free", ci_prefix, self.name); self.ffi_func_free.arguments = vec![FFIArgument { - name: "handle".to_string(), - type_: FFIType::UInt64, + name: "ptr".to_string(), + type_: FFIType::RustArcPtr, }]; self.ffi_func_free.return_type = None; for cons in self.constructors.iter_mut() { @@ -198,7 +202,7 @@ impl APIConverter for weedle::InterfaceDefinition<'_> { // Represents a constructor for an object type. // -// In the FFI, this will be a function that returns a handle for an instance +// In the FFI, this will be a function that returns a pointer to an instance // of the corresponding object type. #[derive(Debug, Clone)] pub struct Constructor { @@ -217,6 +221,10 @@ impl Constructor { self.arguments.iter().collect() } + pub fn all_arguments(&self) -> Vec { + self.arguments.iter().cloned().collect() + } + pub fn ffi_func(&self) -> &FFIFunction { &self.ffi_func } @@ -232,7 +240,7 @@ impl Constructor { self.ffi_func.name.push('_'); self.ffi_func.name.push_str(&self.name); self.ffi_func.arguments = self.arguments.iter().map(Into::into).collect(); - self.ffi_func.return_type = Some(FFIType::UInt64); + self.ffi_func.return_type = Some(FFIType::RustArcPtr); } fn is_primary_constructor(&self) -> bool { @@ -282,8 +290,8 @@ impl APIConverter for weedle::interface::ConstructorInterfaceMember // Represents an instance method for an object type. // -// The in FFI, this will be a function whose first argument is a handle for an -// instance of the corresponding object type. +// The FFI will represent this as a function whose first/self argument is a +// `FFIType::RustArcPtr` to the instance. #[derive(Debug, Clone)] pub struct Method { pub(super) name: String, @@ -313,22 +321,26 @@ impl Method { pub fn first_argument(&self) -> Argument { Argument { - name: "handle".to_string(), + name: "ptr".to_string(), // TODO: ideally we'd get this via `ci.resolve_type_expression` so that it // is contained in the proper `TypeUniverse`, but this works for now. type_: Type::Object(self.object_name.clone()), - by_ref: false, + by_ref: ! self.attributes.get_self_by_arc(), optional: false, default: None, } } + pub fn all_arguments(&self) -> Vec { + vec![self.first_argument()].into_iter().chain(self.arguments.iter().cloned()).collect() + } + pub fn throws(&self) -> Option<&str> { self.attributes.get_throws_err() } pub fn takes_self_by_arc(&self) -> bool { - self.attributes.get_by_arc() + self.attributes.get_self_by_arc() } pub fn derive_ffi_func(&mut self, ci_prefix: &str, obj_prefix: &str) -> Result<()> { @@ -372,9 +384,6 @@ impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { bail!("method modifiers are not supported") } let return_type = ci.resolve_return_type_expression(&self.return_type)?; - if let Some(Type::Object(_)) = return_type { - bail!("Objects cannot currently be returned from functions"); - } Ok(Method { name: match self.identifier { None => bail!("anonymous methods are not supported {:?}", self), diff --git a/uniffi_bindgen/src/interface/record.rs b/uniffi_bindgen/src/interface/record.rs index a51b92a18b..07b417f7e3 100644 --- a/uniffi_bindgen/src/interface/record.rs +++ b/uniffi_bindgen/src/interface/record.rs @@ -65,9 +65,16 @@ impl Record { pub fn name(&self) -> &str { &self.name } + pub fn fields(&self) -> Vec<&Field> { self.fields.iter().collect() } + + pub fn contains_object_references(&self, ci: &ComponentInterface) -> bool { + // *sigh* at the clone here, the relationship between a ComponentInterace + // and its contained types could use a bit of a cleanup. + ci.type_contains_object_references(&Type::Record(self.name.clone())) + } } impl APIConverter for weedle::DictionaryDefinition<'_> { diff --git a/uniffi_bindgen/src/interface/types/mod.rs b/uniffi_bindgen/src/interface/types/mod.rs index 278e20afaa..be469a8d4f 100644 --- a/uniffi_bindgen/src/interface/types/mod.rs +++ b/uniffi_bindgen/src/interface/types/mod.rs @@ -133,8 +133,8 @@ impl Into for &Type { // Strings are always owned rust values. // We might add a separate type for borrowed strings in future. Type::String => FFIType::RustBuffer, - // Objects are passed as opaque integer handles. - Type::Object(_) => FFIType::UInt64, + // Objects are pointers to an Arc<> + Type::Object(_) => FFIType::RustArcPtr, // Callback interfaces are passed as opaque integer handles. Type::CallbackInterface(_) => FFIType::UInt64, // Errors have their own special type. diff --git a/uniffi_bindgen/src/scaffolding.rs b/uniffi_bindgen/src/scaffolding.rs index 1534b29de5..445f7ed3c0 100644 --- a/uniffi_bindgen/src/scaffolding.rs +++ b/uniffi_bindgen/src/scaffolding.rs @@ -42,9 +42,8 @@ mod filters { Type::String => "String".into(), Type::Timestamp => "std::time::SystemTime".into(), Type::Duration => "std::time::Duration".into(), - Type::Enum(name) | Type::Record(name) | Type::Object(name) | Type::Error(name) => { - name.clone() - } + Type::Enum(name) | Type::Record(name) | Type::Error(name) => name.clone(), + Type::Object(name) => format!("std::sync::Arc<{}>", name), Type::CallbackInterface(name) => format!("Box", name), Type::Optional(t) => format!("Option<{}>", type_rs(t)?), Type::Sequence(t) => format!("Vec<{}>", type_rs(t)?), @@ -65,6 +64,7 @@ mod filters { FFIType::Float32 => "f32".into(), FFIType::Float64 => "f64".into(), FFIType::RustCString => "*mut std::os::raw::c_char".into(), + FFIType::RustArcPtr => "*const std::os::raw::c_void".into(), FFIType::RustBuffer => "uniffi::RustBuffer".into(), FFIType::RustError => "uniffi::deps::ffi_support::ExternError".into(), FFIType::ForeignBytes => "uniffi::ForeignBytes".into(), diff --git a/uniffi_bindgen/src/templates/ObjectTemplate.rs b/uniffi_bindgen/src/templates/ObjectTemplate.rs index 8cd303a9f4..084f2176c5 100644 --- a/uniffi_bindgen/src/templates/ObjectTemplate.rs +++ b/uniffi_bindgen/src/templates/ObjectTemplate.rs @@ -1,13 +1,14 @@ -// For each Object definition, we assume the caller has provided an appropriately-shaped `struct` -// with an `impl` for each method on the object. We create a `ConcurrentHandleMap` for safely handing -// out references to these structs to foreign language code, and we provide a `pub extern "C"` function +// For each Object definition, we assume the caller has provided an appropriately-shaped `struct T` +// with an `impl` for each method on the object. We create an `Arc` for "safely" handing out +// references to these structs to foreign language code, and we provide a `pub extern "C"` function // corresponding to each method. // +// (Note that "safely" is in "scare quotes" - that's because we use functions on an `Arc` that +// that are inherently unsafe, but the code we generate is safe in practice.) +// // If the caller's implementation of the struct does not match with the methods or types specified // in the UDL, then the rust compiler will complain with a (hopefully at least somewhat helpful!) // error message when processing this generated code. -{% let handle_map = format!("UNIFFI_HANDLE_MAP_{}", obj.name().to_uppercase()) -%} - {% if obj.uses_deprecated_threadsafe_attribute() %} // We want to mark this as `deprecated` - long story short, the only way to @@ -23,25 +24,27 @@ fn uniffi_note_threadsafe_deprecation_{{ obj.name() }}() {} {% endif %} -uniffi::deps::lazy_static::lazy_static! { - #[doc(hidden)] - static ref {{ handle_map }}: uniffi::ffi::handle_maps::ArcHandleMap<{{ obj.name() }}> - = Default::default(); -} - - {% let ffi_free = obj.ffi_object_free() -%} - #[doc(hidden)] - #[no_mangle] - pub extern "C" fn {{ ffi_free.name() }}(handle: u64) { - let _ = {{ handle_map }}.delete_u64(handle); +{% let ffi_free = obj.ffi_object_free() -%} +#[doc(hidden)] +#[no_mangle] +pub extern "C" fn {{ ffi_free.name() }}(ptr: *const std::os::raw::c_void) { + // We musn't panic across the FFI, but also can't report it anywhere. + // The best we can do it catch, warn and ignore. + if let Err(e) = std::panic::catch_unwind(|| { + assert!(!ptr.is_null()); + {#- turn it into an Arc and explicitly drop it. #} + drop(unsafe { std::sync::Arc::from_raw(ptr as *const {{ obj.name() }}) }) + }) { + uniffi::deps::log::error!("{{ ffi_free.name() }} panicked: {:?}", e); } +} {%- for cons in obj.constructors() %} #[allow(clippy::all)] #[doc(hidden)] #[no_mangle] pub extern "C" fn {{ cons.ffi_func().name() }}( - {%- call rs::arg_list_ffi_decl(cons.ffi_func()) %}) -> u64 { + {%- call rs::arg_list_ffi_decl(cons.ffi_func()) %}) -> *const std::os::raw::c_void /* *const {{ obj.name() }} */ { uniffi::deps::log::debug!("{{ cons.ffi_func().name() }}"); {% if obj.uses_deprecated_threadsafe_attribute() %} uniffi_note_threadsafe_deprecation_{{ obj.name() }}(); diff --git a/uniffi_bindgen/src/templates/macros.rs b/uniffi_bindgen/src/templates/macros.rs index bc2e43cbfa..26527daf38 100644 --- a/uniffi_bindgen/src/templates/macros.rs +++ b/uniffi_bindgen/src/templates/macros.rs @@ -6,14 +6,8 @@ {{ func.name() }}({% call _arg_list_rs_call(func) -%}) {%- endmacro -%} -{%- macro to_rs_call_with_prefix(prefix, func) -%} - {{ func.name() }}( - {{- prefix }}{% if func.arguments().len() > 0 %}, {% call _arg_list_rs_call(func) -%}{% endif -%} -) -{%- endmacro -%} - {%- macro _arg_list_rs_call(func) %} - {%- for arg in func.arguments() %} + {%- for arg in func.all_arguments() %} {%- if arg.by_ref() %}&{% endif %} {{- arg.name()|lift_rs(arg.type_()) }} {%- if !loop.last %}, {% endif %} @@ -44,31 +38,37 @@ {% macro ret(func) %}{% match func.return_type() %}{% when Some with (return_type) %}{{ "_retval"|lower_rs(return_type) }}{% else %}_retval{% endmatch %}{% endmacro %} +{% macro construct(obj, cons) %} + {{- obj.name() }}::{% call to_rs_call(cons) -%} +{% endmacro %} + {% macro to_rs_constructor_call(obj, cons) %} {% match cons.throws() %} {% when Some with (e) %} -UNIFFI_HANDLE_MAP_{{ obj.name()|upper }}.insert_with_result(err, || -> Result<{{obj.name()}}, {{e}}> { - let _retval = {{ obj.name() }}::{% call to_rs_call(cons) %}?; - Ok(_retval) -}) + uniffi::deps::ffi_support::call_with_result(err, || -> Result<_, {{ e }}> { + let _new = {% call construct(obj, cons) %}?; + let _arc = std::sync::Arc::new(_new); + Ok({{ "_arc"|lower_rs(obj.type_()) }}) + }) {% else %} -UNIFFI_HANDLE_MAP_{{ obj.name()|upper }}.insert_with_output(err, || { - {{ obj.name() }}::{% call to_rs_call(cons) %} -}) + uniffi::deps::ffi_support::call_with_output(err, || { + let _new = {% call construct(obj, cons) %}; + let _arc = std::sync::Arc::new(_new); + {{ "_arc"|lower_rs(obj.type_()) }} + }) {% endmatch %} {% endmacro %} {% macro to_rs_method_call(obj, meth) -%} -{% let this_handle_map = format!("UNIFFI_HANDLE_MAP_{}", obj.name().to_uppercase()) -%} {% match meth.throws() -%} {% when Some with (e) -%} -{{ this_handle_map }}.call_{% if meth.takes_self_by_arc() %}by_arc_{% endif %}with_result(err, {{ meth.first_argument().name() }}, |obj| -> Result<{% call return_type_func(meth) %}, {{e}}> { - let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("obj", meth) -%}?; +uniffi::deps::ffi_support::call_with_result(err, || -> Result<{% call return_type_func(meth) %}, {{e}}> { + let _retval = {{ obj.name() }}::{% call to_rs_call(meth) %}?; Ok({% call ret(meth) %}) }) -{% else -%} -{{ this_handle_map }}.call_{% if meth.takes_self_by_arc() %}by_arc_{% endif %}with_output(err, {{ meth.first_argument().name() }}, |obj| { - let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("obj", meth) -%}; +{% else %} +uniffi::deps::ffi_support::call_with_output(err, || { + let _retval = {{ obj.name() }}::{% call to_rs_call(meth) %}; {% call ret(meth) %} }) {% endmatch -%}