diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a8482ae49..47a443f5ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,10 +14,18 @@ [All changes in [[UnreleasedUniFFIVersion]]](https://github.com/mozilla/uniffi-rs/compare/v0.25.0...HEAD). +### What's Fixed +- Fixed potential use-after-free error when lowering object handles (#1797) + ### What's changed? - The `rust_future_continuation_callback_set` FFI function was removed. `rust_future_poll` now inputs the callback pointer. External bindings authors will need to update their code. +- The object handle FFI has changed. External bindings generators will need to update their code to + use the new slab/handle system. +- Trait interfaces defined in UDL need to be wrapped with `#[uniffi::trait_interface]`. +- Trait interfaces performance has been improved. If a trait interface handle is passed across the + FFI multiple times, UniFFI will only wrap the object once rather than each time it's passed. ## v0.25.0 (backend crates: v0.25.0) - (_2023-10-18_) diff --git a/Cargo.lock b/Cargo.lock index a3a018f69b..f684928ee2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -93,6 +93,12 @@ version = "1.0.75" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" +[[package]] +name = "append-only-vec" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3cb8f874ecf419dd8165d0279746de966cb8966636d028845e3bd65d519812a" + [[package]] name = "askama" version = "0.12.0" @@ -470,6 +476,28 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "const-random" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "368a7a772ead6ce7e1de82bfb04c485f3db8ec744f72925af5735e29a22cc18e" +dependencies = [ + "const-random-macro", + "proc-macro-hack", +] + +[[package]] +name = "const-random-macro" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d7d6ab3c3a2282db210df5f02c4dab6e0a7057af0fb7ebd4070f30fe05c0ddb" +dependencies = [ + "getrandom", + "once_cell", + "proc-macro-hack", + "tiny-keccak", +] + [[package]] name = "criterion" version = "0.5.1" @@ -549,6 +577,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crunchy" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7" + [[package]] name = "either" version = "1.9.0" @@ -661,6 +695,17 @@ dependencies = [ "windows", ] +[[package]] +name = "getrandom" +version = "0.2.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be4136b2a15dd319360be1c07d9933517ccf0be8f16bf62a3bee4f0d618df427" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + [[package]] name = "gimli" version = "0.28.0" @@ -1074,6 +1119,18 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "ppv-lite86" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" + +[[package]] +name = "proc-macro-hack" +version = "0.5.20+deprecated" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" + [[package]] name = "proc-macro2" version = "1.0.66" @@ -1092,6 +1149,36 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + [[package]] name = "rayon" version = "1.7.0" @@ -1396,6 +1483,15 @@ dependencies = [ "once_cell", ] +[[package]] +name = "tiny-keccak" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c9d3793400a45f954c52e73d068316d76b6f4e36977e3fcebb13a2721e80237" +dependencies = [ + "crunchy", +] + [[package]] name = "tinytemplate" version = "1.2.1" @@ -1977,13 +2073,16 @@ name = "uniffi_core" version = "0.25.0" dependencies = [ "anyhow", + "append-only-vec", "async-compat", "bytes", "camino", + "const-random", "log", "once_cell", "oneshot", "paste", + "rand", "static_assertions", ] @@ -2096,6 +2195,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "wasi" +version = "0.11.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" + [[package]] name = "wasm-bindgen" version = "0.2.87" diff --git a/docs/manual/src/udl/interfaces.md b/docs/manual/src/udl/interfaces.md index 23db54a8d8..4a7fbd5a3d 100644 --- a/docs/manual/src/udl/interfaces.md +++ b/docs/manual/src/udl/interfaces.md @@ -87,9 +87,10 @@ interface Button { ``` -With the following Rust implementation: +The Rust implementation needs to be wrapped in `#[uniffi::trait_interface]`: ```rust +#[uniffi::trait_interface] pub trait Button: Send + Sync { fn name(&self) -> String; } diff --git a/examples/callbacks/src/lib.rs b/examples/callbacks/src/lib.rs index dac5653d1b..9e30bf1d86 100644 --- a/examples/callbacks/src/lib.rs +++ b/examples/callbacks/src/lib.rs @@ -21,6 +21,7 @@ impl From for TelephoneError { } // SIM cards. +#[uniffi::trait_interface] pub trait SimCard: Send + Sync { fn name(&self) -> String; } @@ -39,6 +40,7 @@ fn get_sim_cards() -> Vec> { } // The call-answer, callback interface. +#[uniffi::trait_interface] pub trait CallAnswerer { fn answer(&self) -> Result; } diff --git a/examples/sprites/tests/bindings/test_sprites.kts b/examples/sprites/tests/bindings/test_sprites.kts index 42451f28dd..ddd983351c 100644 --- a/examples/sprites/tests/bindings/test_sprites.kts +++ b/examples/sprites/tests/bindings/test_sprites.kts @@ -16,7 +16,7 @@ s.destroy() try { s.moveBy(Vector(0.0, 0.0)) assert(false) { "Should not be able to call anything after `destroy`" } -} catch(e: IllegalStateException) { +} catch(e: InternalException) { assert(true) } diff --git a/examples/traits/src/lib.rs b/examples/traits/src/lib.rs index 9d84bdae7f..51d81d83d8 100644 --- a/examples/traits/src/lib.rs +++ b/examples/traits/src/lib.rs @@ -13,6 +13,7 @@ fn press(button: Arc) -> Arc { button } +#[uniffi::trait_interface] pub trait Button: Send + Sync { fn name(&self) -> String; } diff --git a/fixtures/coverall/src/traits.rs b/fixtures/coverall/src/traits.rs index 15785ef0c6..a47a6f54eb 100644 --- a/fixtures/coverall/src/traits.rs +++ b/fixtures/coverall/src/traits.rs @@ -10,6 +10,7 @@ pub fn get_traits() -> Vec> { vec![Arc::new(Trait1::default()), Arc::new(Trait2::default())] } +#[uniffi::trait_interface] pub trait NodeTrait: Send + Sync + std::fmt::Debug { fn name(&self) -> String; @@ -35,6 +36,7 @@ pub fn ancestor_names(node: Arc) -> Vec { /// Test trait /// /// The goal here is to test all possible arg, return, and error types. +#[uniffi::trait_interface] pub trait Getters: Send + Sync { fn get_bool(&self, v: bool, arg2: bool) -> bool; fn get_string(&self, v: String, arg2: bool) -> Result; diff --git a/fixtures/coverall/tests/bindings/test_coverall.kts b/fixtures/coverall/tests/bindings/test_coverall.kts index 8bf3b0077b..b2b420bb18 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.kts +++ b/fixtures/coverall/tests/bindings/test_coverall.kts @@ -344,12 +344,10 @@ getTraits().let { traits -> assert(traits[1].name() == "node-2") assert(traits[1].strongCount() == 2UL) - // Note: this doesn't increase the Rust strong count, since we wrap the Rust impl with a - // Swift impl before passing it to `setParent()` traits[0].setParent(traits[1]) assert(ancestorNames(traits[0]) == listOf("node-2")) assert(ancestorNames(traits[1]).isEmpty()) - assert(traits[1].strongCount() == 2UL) + assert(traits[1].strongCount() == 3UL) assert(traits[0].getParent()!!.name() == "node-2") val ktNode = KotlinNode() @@ -358,6 +356,10 @@ getTraits().let { traits -> assert(ancestorNames(traits[1]) == listOf("node-kt")) assert(ancestorNames(ktNode) == listOf()) + // If we get the node back from Rust, we should get the original object, not an object that's + // been wrapped again. + assert(traits[1].getParent() === ktNode) + traits[1].setParent(null) ktNode.setParent(traits[0]) assert(ancestorNames(ktNode) == listOf("node-1", "node-2")) diff --git a/fixtures/coverall/tests/bindings/test_coverall.py b/fixtures/coverall/tests/bindings/test_coverall.py index 17593bc833..b4e27a450e 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.py +++ b/fixtures/coverall/tests/bindings/test_coverall.py @@ -332,14 +332,14 @@ def strong_count(self): class TraitsTest(unittest.TestCase): # Test traits implemented in Rust - # def test_rust_getters(self): - # test_getters(None) - # self.check_getters_from_python(make_rust_getters()) + def test_rust_getters(self): + test_getters(make_rust_getters()) + self.check_getters_from_python(make_rust_getters()) - # Test traits implemented in Rust + # Test traits implemented in Python def test_python_getters(self): test_getters(PyGetters()) - #self.check_getters_from_python(PyGetters()) + self.check_getters_from_python(PyGetters()) def check_getters_from_python(self, getters): self.assertEqual(getters.get_bool(True, True), False); @@ -370,7 +370,11 @@ def check_getters_from_python(self, getters): with self.assertRaises(ComplexError.UnknownError): getters.get_option("unknown-error", True) - with self.assertRaises(InternalError): + # If the trait is implmented in Rust, we should see an `InternalError`. + + # If it's implemented in Python, we see a `RuntimeError` since it's a direct call with no + # UniFFI wrapping. + with self.assertRaises((InternalError, RuntimeError)): getters.get_string("unexpected-error", True) def test_path(self): @@ -386,9 +390,7 @@ def test_path(self): # Let's try connecting them together traits[0].set_parent(traits[1]) - # Note: this doesn't increase the Rust strong count, since we wrap the Rust impl with a - # python impl before passing it to `set_parent()` - self.assertEqual(traits[1].strong_count(), 2) + self.assertEqual(traits[1].strong_count(), 3) self.assertEqual(ancestor_names(traits[0]), ["node-2"]) self.assertEqual(ancestor_names(traits[1]), []) self.assertEqual(traits[0].get_parent().name(), "node-2") @@ -401,6 +403,10 @@ def test_path(self): self.assertEqual(ancestor_names(traits[1]), ["node-py"]) self.assertEqual(ancestor_names(py_node), []) + # If we get the node back from Rust, we should get the original object, not an object that's + # been wrapped again. + self.assertIs(traits[1].get_parent(), py_node) + # Rotating things. # The ancestry chain now goes py_node -> traits[0] -> traits[1] traits[1].set_parent(None) diff --git a/fixtures/coverall/tests/bindings/test_coverall.swift b/fixtures/coverall/tests/bindings/test_coverall.swift index c6fcba4290..7d93ae7465 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.swift +++ b/fixtures/coverall/tests/bindings/test_coverall.swift @@ -384,12 +384,10 @@ do { assert(traits[1].name() == "node-2") assert(traits[1].strongCount() == 2) - // Note: this doesn't increase the Rust strong count, since we wrap the Rust impl with a - // Swift impl before passing it to `set_parent()` traits[0].setParent(parent: traits[1]) assert(ancestorNames(node: traits[0]) == ["node-2"]) assert(ancestorNames(node: traits[1]) == []) - assert(traits[1].strongCount() == 2) + assert(traits[1].strongCount() == 3) assert(traits[0].getParent()!.name() == "node-2") // Throw in a Swift implementation of the trait @@ -400,6 +398,10 @@ do { assert(ancestorNames(node: traits[1]) == ["node-swift"]) assert(ancestorNames(node: swiftNode) == []) + // If we get the node back from Rust, we should get the original object, not an object that's + // been wrapped again. + assert(traits[1].getParent() === swiftNode) + // Rotating things. // The ancestry chain now goes swiftNode -> traits[0] -> traits[1] traits[1].setParent(parent: nil) diff --git a/fixtures/coverall/tests/bindings/test_handlerace.kts b/fixtures/coverall/tests/bindings/test_handlerace.kts deleted file mode 100644 index 580099e6f1..0000000000 --- a/fixtures/coverall/tests/bindings/test_handlerace.kts +++ /dev/null @@ -1,52 +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/. */ - -import java.util.concurrent.* -import uniffi.coverall.* - -// This tests the thread-safety of operations on object handles. -// The idea is to have a method call running on one thread while -// we try to destroy the object on another thread. This should -// be handled correctly at the Kotlin level, rather than triggering -// any fail-safe checks (or worse, memory unsafety!) in the underlying -// Rust code. -// -// This test was able to reproduce the concurrency issue in -// https://github.com/mozilla/uniffi-rs/issues/457, and is kept -// to prevent regressions. - -// Give ourselves enough opportunity to trigger concurrency issues. -for (i in 1..1000) { - // So we can operate concurrently. - val executor = Executors.newFixedThreadPool(2) - try { - // On the main thread here, we're going to create and then destroy a `Coveralls` instance. - val concurrentMethodCall: Future = Coveralls("test_handlerace").use { coveralls -> - // Make a method call on a separate thread thread. - val concurrentMethodCall = executor.submit(Callable { - coveralls.getName() - }) - // Sleep a little, to give the background thread a chance to start operating. - Thread.sleep(1) - concurrentMethodCall - } - // At this point the object has been destroyed. - // The concurrent method call should either have failed to run at all (if the object was destroyed - // before it started) or should have completely succeeded (if it started before the object was - // destroyed). It should not fail in the Rust code (which currently fails cleanly on handlemap - // safety checks, but in future might be a use-after-free). - try { - concurrentMethodCall.get() - } catch (e: ExecutionException) { - if (e.cause is IllegalStateException && e.message!!.endsWith("has already been destroyed")) { - // This is fine, we rejected the call after the object had been destroyed. - } else { - // Any other behaiour signals a possible problem. - throw e - } - } - } finally { - executor.shutdown() - } -} diff --git a/fixtures/coverall/tests/test_generated_bindings.rs b/fixtures/coverall/tests/test_generated_bindings.rs index 962f767964..ee5d69dfb2 100644 --- a/fixtures/coverall/tests/test_generated_bindings.rs +++ b/fixtures/coverall/tests/test_generated_bindings.rs @@ -3,5 +3,4 @@ uniffi::build_foreign_language_testcases!( "tests/bindings/test_coverall.kts", "tests/bindings/test_coverall.rb", "tests/bindings/test_coverall.swift", - "tests/bindings/test_handlerace.kts", ); diff --git a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts index 38ee5a7abd..404c06a463 100644 --- a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts +++ b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts @@ -37,11 +37,3 @@ runBlocking { assert(result.delayMs <= 200U) tester.close() } - -// Test that we cleanup when dropping a ForeignExecutor handles -assert(FfiConverterForeignExecutor.handleCount() == 0) -val tester = ForeignExecutorTester(coroutineScope) -val tester2 = ForeignExecutorTester.newFromSequence(listOf(coroutineScope)) -tester.close() -tester2.close() -assert(FfiConverterForeignExecutor.handleCount() == 0) diff --git a/fixtures/reexport-scaffolding-macro/src/lib.rs b/fixtures/reexport-scaffolding-macro/src/lib.rs index 6bd04f2ccd..860fe2d019 100644 --- a/fixtures/reexport-scaffolding-macro/src/lib.rs +++ b/fixtures/reexport-scaffolding-macro/src/lib.rs @@ -6,9 +6,10 @@ mod tests { use cargo_metadata::Message; use libloading::{Library, Symbol}; use std::ffi::CString; - use std::os::raw::c_void; use std::process::{Command, Stdio}; - use uniffi::{FfiConverter, ForeignCallback, RustBuffer, RustCallStatus, RustCallStatusCode}; + use uniffi::{ + FfiConverter, ForeignCallback, Handle, RustBuffer, RustCallStatus, RustCallStatusCode, + }; use uniffi_bindgen::ComponentInterface; struct UniFfiTag; @@ -149,26 +150,27 @@ mod tests { .ffi_func() .name(), ); - let coveralls_new: Symbol< - unsafe extern "C" fn(RustBuffer, &mut RustCallStatus) -> *const c_void, - > = get_symbol( - &library, - object_def.primary_constructor().unwrap().ffi_func().name(), - ); + let coveralls_new: Symbol Handle> = + get_symbol( + &library, + object_def.primary_constructor().unwrap().ffi_func().name(), + ); let coveralls_get_name: Symbol< - unsafe extern "C" fn(*const c_void, &mut RustCallStatus) -> RustBuffer, + unsafe extern "C" fn(Handle, &mut RustCallStatus) -> RustBuffer, > = get_symbol( &library, object_def.get_method("get_name").ffi_func().name(), ); - let coveralls_free: Symbol ()> = + let coveralls_inc_ref: Symbol ()> = + get_symbol(&library, object_def.ffi_object_inc_ref().name()); + let coveralls_free: Symbol ()> = get_symbol(&library, object_def.ffi_object_free().name()); let num_alive = unsafe { get_num_alive(&mut call_status) }; assert_eq!(call_status.code, RustCallStatusCode::Success); assert_eq!(num_alive, 0); - let obj_id = unsafe { + let obj_handle = unsafe { coveralls_new( >::lower("TestName".into()), &mut call_status, @@ -176,7 +178,10 @@ mod tests { }; assert_eq!(call_status.code, RustCallStatusCode::Success); - let name_buf = unsafe { coveralls_get_name(obj_id, &mut call_status) }; + unsafe { coveralls_inc_ref(obj_handle, &mut call_status) }; + assert_eq!(call_status.code, RustCallStatusCode::Success); + + let name_buf = unsafe { coveralls_get_name(obj_handle, &mut call_status) }; assert_eq!(call_status.code, RustCallStatusCode::Success); assert_eq!( >::try_lift(name_buf).unwrap(), @@ -187,7 +192,7 @@ mod tests { assert_eq!(call_status.code, RustCallStatusCode::Success); assert_eq!(num_alive, 1); - unsafe { coveralls_free(obj_id, &mut call_status) }; + unsafe { coveralls_free(obj_handle, &mut call_status) }; assert_eq!(call_status.code, RustCallStatusCode::Success); let num_alive = unsafe { get_num_alive(&mut call_status) }; diff --git a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs index 1bcd3a4237..6671a9624a 100644 --- a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs +++ b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs @@ -4,6 +4,7 @@ uniffi_macros::generate_and_include_scaffolding!("../../../../fixtures/uitests/s fn main() { /* empty main required by `trybuild` */} // This will fail to compile, because the trait is not explicit Send+Sync +#[uniffi::trait_interface] pub trait Trait { } diff --git a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr index a8e7dc90a1..3925745647 100644 --- a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr +++ b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr @@ -27,9 +27,9 @@ note: required by a bound in `FfiConverterArc` = note: this error originates in the attribute macro `::uniffi::export_for_udl` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: `(dyn ProcMacroTrait + 'static)` cannot be shared between threads safely - --> tests/ui/interface_trait_not_sync_and_send.rs:11:1 + --> tests/ui/interface_trait_not_sync_and_send.rs:12:1 | -11 | #[uniffi::export] +12 | #[uniffi::export] | ^^^^^^^^^^^^^^^^^ `(dyn ProcMacroTrait + 'static)` cannot be shared between threads safely | = help: the trait `Sync` is not implemented for `(dyn ProcMacroTrait + 'static)` @@ -41,9 +41,9 @@ note: required by a bound in `FfiConverterArc` = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: `(dyn ProcMacroTrait + 'static)` cannot be sent between threads safely - --> tests/ui/interface_trait_not_sync_and_send.rs:11:1 + --> tests/ui/interface_trait_not_sync_and_send.rs:12:1 | -11 | #[uniffi::export] +12 | #[uniffi::export] | ^^^^^^^^^^^^^^^^^ `(dyn ProcMacroTrait + 'static)` cannot be sent between threads safely | = help: the trait `Send` is not implemented for `(dyn ProcMacroTrait + 'static)` diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index f36a5bda2f..7cd8867d14 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -292,19 +292,16 @@ impl KotlinCodeOracle { FfiType::Int64 | FfiType::UInt64 => "Long".to_string(), FfiType::Float32 => "Float".to_string(), FfiType::Float64 => "Double".to_string(), - FfiType::RustArcPtr(_) => "Pointer".to_string(), + FfiType::Handle => "UniffiHandle".to_string(), FfiType::RustBuffer(maybe_suffix) => { format!("RustBuffer{}", maybe_suffix.as_deref().unwrap_or_default()) } FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(), FfiType::ForeignCallback => "ForeignCallback".to_string(), - FfiType::ForeignExecutorHandle => "USize".to_string(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback".to_string(), - FfiType::RustFutureHandle => "Pointer".to_string(), FfiType::RustFutureContinuationCallback => { "UniFffiRustFutureContinuationCallbackType".to_string() } - FfiType::RustFutureContinuationData => "USize".to_string(), } } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt index 4d1c099a02..5aef545aec 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt @@ -3,20 +3,20 @@ internal const val UNIFFI_RUST_FUTURE_POLL_READY = 0.toShort() internal const val UNIFFI_RUST_FUTURE_POLL_MAYBE_READY = 1.toShort() -internal val uniffiContinuationHandleMap = UniFfiHandleMap>() +internal val uniffiContinuationSlab = UniffiSlab>() // FFI type for Rust future continuations internal object uniffiRustFutureContinuationCallback: UniFffiRustFutureContinuationCallbackType { - override fun callback(continuationHandle: USize, pollResult: Short) { - uniffiContinuationHandleMap.remove(continuationHandle)?.resume(pollResult) + override fun callback(continuationHandle: UniffiHandle, pollResult: Short) { + uniffiContinuationSlab.remove(continuationHandle).resume(pollResult) } } internal suspend fun uniffiRustCallAsync( - rustFuture: Pointer, - pollFunc: (Pointer, UniFffiRustFutureContinuationCallbackType, USize) -> Unit, - completeFunc: (Pointer, RustCallStatus) -> F, - freeFunc: (Pointer) -> Unit, + rustFuture: UniffiHandle, + pollFunc: (UniffiHandle, UniFffiRustFutureContinuationCallbackType, UniffiHandle) -> Unit, + completeFunc: (UniffiHandle, RustCallStatus) -> F, + freeFunc: (UniffiHandle) -> Unit, liftFunc: (F) -> T, errorHandler: CallStatusErrorHandler ): T { @@ -26,7 +26,7 @@ internal suspend fun uniffiRustCallAsync( pollFunc( rustFuture, uniffiRustFutureContinuationCallback, - uniffiContinuationHandleMap.insert(continuation) + uniffiContinuationSlab.insert(continuation) ) } } while (pollResult != UNIFFI_RUST_FUTURE_POLL_READY); diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt index f1c58ee971..2eaa741a74 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt @@ -3,14 +3,15 @@ // Implement the foreign callback handler for {{ interface_name }} internal class {{ callback_handler_class }} : ForeignCallback { @Suppress("TooGenericExceptionCaught") - override fun invoke(handle: Handle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int { - val cb = {{ ffi_converter_name }}.handleMap.get(handle) + override fun invoke(handle: UniffiHandle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int { + val cb = {{ ffi_converter_name }}.slab.get(handle) return when (method) { IDX_CALLBACK_FREE -> { - {{ ffi_converter_name }}.handleMap.remove(handle) - - // Successful return - // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` + {{ ffi_converter_name }}.slab.remove(handle) + UNIFFI_CALLBACK_SUCCESS + } + IDX_CALLBACK_INC_REF -> { + {{ ffi_converter_name }}.slab.incRef(handle) UNIFFI_CALLBACK_SUCCESS } {% for meth in methods.iter() -%} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt index d0e0686322..5db0f0caad 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt @@ -1,63 +1,32 @@ {{- self.add_import("java.util.concurrent.atomic.AtomicLong") }} {{- self.add_import("java.util.concurrent.locks.ReentrantLock") }} -{{- self.add_import("kotlin.concurrent.withLock") }} - -internal typealias Handle = Long -internal class ConcurrentHandleMap( - private val leftMap: MutableMap = mutableMapOf(), -) { - private val lock = java.util.concurrent.locks.ReentrantLock() - private val currentHandle = AtomicLong(0L) - private val stride = 1L - - fun insert(obj: T): Handle = - lock.withLock { - currentHandle.getAndAdd(stride) - .also { handle -> - leftMap[handle] = obj - } - } - - fun get(handle: Handle) = lock.withLock { - leftMap[handle] ?: throw InternalException("No callback in handlemap; this is a Uniffi bug") - } - - fun delete(handle: Handle) { - this.remove(handle) - } - - fun remove(handle: Handle): T? = - lock.withLock { - leftMap.remove(handle) - } -} interface ForeignCallback : com.sun.jna.Callback { - public fun invoke(handle: Handle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int + public fun invoke(handle: UniffiHandle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int } -// Magic number for the Rust proxy to call using the same mechanism as every other method, -// to free the callback once it's dropped by Rust. +// Magic numbers for the Rust proxy to call using the same mechanism as every other method. + +// Dec-ref the callback object internal const val IDX_CALLBACK_FREE = 0 -// Callback return codes +// Inc-ref the callback object +internal const val IDX_CALLBACK_INC_REF = 0x7FFF_FFFF; + +// Callback return values internal const val UNIFFI_CALLBACK_SUCCESS = 0 internal const val UNIFFI_CALLBACK_ERROR = 1 internal const val UNIFFI_CALLBACK_UNEXPECTED_ERROR = 2 -public abstract class FfiConverterCallbackInterface: FfiConverter { - internal val handleMap = ConcurrentHandleMap() - - internal fun drop(handle: Handle) { - handleMap.remove(handle) - } +public abstract class FfiConverterCallbackInterface: FfiConverter { + internal val slab = UniffiSlab() - override fun lift(value: Handle): CallbackInterface { - return handleMap.get(value) + override fun lift(value: UniffiHandle): CallbackInterface { + return slab.get(value) } override fun read(buf: ByteBuffer) = lift(buf.getLong()) - override fun lower(value: CallbackInterface) = handleMap.insert(value) + override fun lower(value: CallbackInterface) = slab.insert(value) override fun allocationSize(value: CallbackInterface) = 8 diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt index 3544b2f9e6..b5795f9151 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt @@ -16,7 +16,7 @@ internal interface UniFfiRustTaskCallback : com.sun.jna.Callback { } internal object UniFfiForeignExecutorCallback : com.sun.jna.Callback { - fun callback(handle: USize, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) : Byte { + fun callback(handle: UniffiHandle, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) : Byte { if (rustTask == null) { FfiConverterForeignExecutor.drop(handle) return UNIFFI_FOREIGN_EXECUTOR_CALLBACK_SUCCESS @@ -42,11 +42,11 @@ internal object UniFfiForeignExecutorCallback : com.sun.jna.Callback { } } -public object FfiConverterForeignExecutor: FfiConverter { - internal val handleMap = UniFfiHandleMap() +public object FfiConverterForeignExecutor: FfiConverter { + internal val slab = UniffiSlab() - internal fun drop(handle: USize) { - handleMap.remove(handle) + internal fun drop(handle: UniffiHandle) { + slab.remove(handle) } internal fun register(lib: _UniFFILib) { @@ -58,26 +58,21 @@ public object FfiConverterForeignExecutor: FfiConverter { {% endmatch %} } - // Number of live handles, exposed so we can test the memory management - public fun handleCount() : Int { - return handleMap.size - } - - override fun allocationSize(value: CoroutineScope) = USize.size + override fun allocationSize(value: CoroutineScope) = 8 - override fun lift(value: USize): CoroutineScope { - return handleMap.get(value) ?: throw RuntimeException("unknown handle in FfiConverterForeignExecutor.lift") + override fun lift(value: UniffiHandle): CoroutineScope { + return slab.get(value) } override fun read(buf: ByteBuffer): CoroutineScope { - return lift(USize.readFromBuffer(buf)) + return lift(buf.getLong()) } - override fun lower(value: CoroutineScope): USize { - return handleMap.insert(value) + override fun lower(value: CoroutineScope): UniffiHandle { + return slab.insert(value) } override fun write(value: CoroutineScope, buf: ByteBuffer) { - lower(value).writeToBuffer(buf) + buf.putLong(lower(value)) } } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt index 382a5f7413..7dfa38bcd5 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt @@ -1,3 +1,5 @@ +typealias UniffiHandle = Long + // A handful of classes and functions to support the generated data structures. // This would be a good candidate for isolating in its own ffi-support lib. // Error runtime. @@ -73,89 +75,7 @@ private inline fun rustCall(callback: (RustCallStatus) -> U): U { return rustCallWithError(NullCallStatusErrorHandler, callback); } -// IntegerType that matches Rust's `usize` / C's `size_t` -public class USize(value: Long = 0) : IntegerType(Native.SIZE_T_SIZE, value, true) { - // This is needed to fill in the gaps of IntegerType's implementation of Number for Kotlin. - override fun toByte() = toInt().toByte() - // Needed until https://youtrack.jetbrains.com/issue/KT-47902 is fixed. - @Deprecated("`toInt().toChar()` is deprecated") - override fun toChar() = toInt().toChar() - override fun toShort() = toInt().toShort() - - fun writeToBuffer(buf: ByteBuffer) { - // Make sure we always write usize integers using native byte-order, since they may be - // casted to pointer values - buf.order(ByteOrder.nativeOrder()) - try { - when (Native.SIZE_T_SIZE) { - 4 -> buf.putInt(toInt()) - 8 -> buf.putLong(toLong()) - else -> throw RuntimeException("Invalid SIZE_T_SIZE: ${Native.SIZE_T_SIZE}") - } - } finally { - buf.order(ByteOrder.BIG_ENDIAN) - } - } - - companion object { - val size: Int - get() = Native.SIZE_T_SIZE - - fun readFromBuffer(buf: ByteBuffer) : USize { - // Make sure we always read usize integers using native byte-order, since they may be - // casted from pointer values - buf.order(ByteOrder.nativeOrder()) - try { - return when (Native.SIZE_T_SIZE) { - 4 -> USize(buf.getInt().toLong()) - 8 -> USize(buf.getLong()) - else -> throw RuntimeException("Invalid SIZE_T_SIZE: ${Native.SIZE_T_SIZE}") - } - } finally { - buf.order(ByteOrder.BIG_ENDIAN) - } - } - } -} - - -// Map handles to objects -// -// This is used when the Rust code expects an opaque pointer to represent some foreign object. -// Normally we would pass a pointer to the object, but JNA doesn't support getting a pointer from an -// object reference , nor does it support leaking a reference to Rust. -// -// Instead, this class maps USize values to objects so that we can pass a pointer-sized type to -// Rust when it needs an opaque pointer. -// -// TODO: refactor callbacks to use this class -internal class UniFfiHandleMap { - private val map = ConcurrentHashMap() - // Use AtomicInteger for our counter, since we may be on a 32-bit system. 4 billion possible - // values seems like enough. If somehow we generate 4 billion handles, then this will wrap - // around back to zero and we can assume the first handle generated will have been dropped by - // then. - private val counter = java.util.concurrent.atomic.AtomicInteger(0) - - val size: Int - get() = map.size - - fun insert(obj: T): USize { - val handle = USize(counter.getAndAdd(1).toLong()) - map.put(handle, obj) - return handle - } - - fun get(handle: USize): T? { - return map.get(handle) - } - - fun remove(handle: USize): T? { - return map.remove(handle) - } -} - // FFI type for Rust future continuations internal interface UniFffiRustFutureContinuationCallbackType : com.sun.jna.Callback { - fun callback(continuationHandle: USize, pollResult: Short); + fun callback(continuationHandle: Long, pollResult: Short); } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt index 1e3ead5a7e..79d578da4c 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt @@ -30,106 +30,30 @@ inline fun T.use(block: (T) -> R) = } } -// The base class for all UniFFI Object types. -// -// This class provides core operations for working with the Rust `Arc` pointer to -// the live Rust struct on the other side of the FFI. -// -// There's some subtlety here, because we have to be careful not to operate on a Rust -// struct after it has been dropped, and because we must expose a public API for freeing -// the Kotlin wrapper object in lieu of reliable finalizers. The core requirements are: -// -// * Each `FFIObject` instance holds an opaque pointer to the underlying Rust struct. -// Method calls need to read this pointer from the object's state and pass it in to -// the Rust FFI. -// -// * When an `FFIObject` is no longer needed, its pointer should be passed to a -// special destructor function provided by the Rust FFI, which will drop the -// underlying Rust struct. -// -// * Given an `FFIObject` instance, calling code is expected to call the special -// `destroy` method in order to free it after use, either by calling it explicitly -// or by using a higher-level helper like the `use` method. Failing to do so will -// leak the underlying Rust struct. -// -// * We can't assume that calling code will do the right thing, and must be prepared -// to handle Kotlin method calls executing concurrently with or even after a call to -// `destroy`, and to handle multiple (possibly concurrent!) calls to `destroy`. -// -// * We must never allow Rust code to operate on the underlying Rust struct after -// the destructor has been called, and must never call the destructor more than once. -// Doing so may trigger memory unsafety. -// -// If we try to implement this with mutual exclusion on access to the pointer, there is the -// possibility of a race between a method call and a concurrent call to `destroy`: -// -// * Thread A starts a method call, reads the value of the pointer, but is interrupted -// before it can pass the pointer over the FFI to Rust. -// * Thread B calls `destroy` and frees the underlying Rust struct. -// * Thread A resumes, passing the already-read pointer value to Rust and triggering -// a use-after-free. -// -// One possible solution would be to use a `ReadWriteLock`, with each method call taking -// a read lock (and thus allowed to run concurrently) and the special `destroy` method -// taking a write lock (and thus blocking on live method calls). However, we aim not to -// generate methods with any hidden blocking semantics, and a `destroy` method that might -// block if called incorrectly seems to meet that bar. -// -// So, we achieve our goals by giving each `FFIObject` an associated `AtomicLong` counter to track -// the number of in-flight method calls, and an `AtomicBoolean` flag to indicate whether `destroy` -// has been called. These are updated according to the following rules: -// -// * The initial value of the counter is 1, indicating a live object with no in-flight calls. -// The initial value for the flag is false. +// Wraps `UniffiHandle` to pass to object constructors. // -// * At the start of each method call, we atomically check the counter. -// If it is 0 then the underlying Rust struct has already been destroyed and the call is aborted. -// If it is nonzero them we atomically increment it by 1 and proceed with the method call. +// This class exists because `UniffiHandle` is a typealias to `Long`. If the object constructor +// inputs `UniffiHandle` directly and the user defines a primary constructor than inputs a single +// `Long` or `ULong` input, then we get JVM signature conflicts. To avoid this, we pass this type +// in instead. // -// * At the end of each method call, we atomically decrement and check the counter. -// If it has reached zero then we destroy the underlying Rust struct. -// -// * When `destroy` is called, we atomically flip the flag from false to true. -// If the flag was already true we silently fail. -// Otherwise we atomically decrement and check the counter. -// If it has reached zero then we destroy the underlying Rust struct. -// -// Astute readers may observe that this all sounds very similar to the way that Rust's `Arc` works, -// and indeed it is, with the addition of a flag to guard against multiple calls to `destroy`. -// -// The overall effect is that the underlying Rust struct is destroyed only when `destroy` has been -// called *and* all in-flight method calls have completed, avoiding violating any of the expectations -// of the underlying Rust code. -// -// In the future we may be able to replace some of this with automatic finalization logic, such as using -// the new "Cleaner" functionaility in Java 9. The above scheme has been designed to work even if `destroy` is -// invoked by garbage-collection machinery rather than by calling code (which by the way, it's apparently also -// possible for the JVM to finalize an object while there is an in-flight call to one of its methods [1], -// so there would still be some complexity here). -// -// Sigh...all of this for want of a robust finalization mechanism. -// -// [1] https://stackoverflow.com/questions/24376768/can-java-finalize-an-object-when-it-is-still-in-scope/24380219 -// -abstract class FFIObject( - protected val pointer: Pointer -): Disposable, AutoCloseable { +// Let's try to remove this when we update the code based on ADR-0008. +data class UniffiHandleWrapper(val handle: UniffiHandle) +// The base class for all UniFFI Object types. +// +// This class provides core operations for working with the Rust handle to the live Rust struct on +// the other side of the FFI. +abstract class FFIObject(): Disposable, AutoCloseable { private val wasDestroyed = AtomicBoolean(false) - private val callCounter = AtomicLong(1) open protected fun freeRustArcPtr() { // To be overridden in subclasses. } override fun destroy() { - // Only allow a single call to this method. - // TODO: maybe we should log a warning if called more than once? if (this.wasDestroyed.compareAndSet(false, true)) { - // This decrement always matches the initial count of 1 given at creation time. - if (this.callCounter.decrementAndGet() == 0L) { - this.freeRustArcPtr() - } + this.freeRustArcPtr() } } @@ -137,27 +61,4 @@ abstract class FFIObject( override fun close() { this.destroy() } - - internal inline fun callWithPointer(block: (ptr: Pointer) -> R): R { - // Check and increment the call counter, to keep the object alive. - // This needs a compare-and-set retry loop in case of concurrent updates. - do { - val c = this.callCounter.get() - if (c == 0L) { - throw IllegalStateException("${this.javaClass.simpleName} object has already been destroyed") - } - if (c == Long.MAX_VALUE) { - throw IllegalStateException("${this.javaClass.simpleName} call counter would overflow") - } - } while (! this.callCounter.compareAndSet(c, c + 1L)) - // Now we can safely do the method call without the pointer being freed concurrently. - try { - return block(this.pointer) - } finally { - // This decrement always matches the increment we performed above. - if (this.callCounter.decrementAndGet() == 0L) { - this.freeRustArcPtr() - } - } - } } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index cef6be881f..fb7a09c8f2 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -5,14 +5,14 @@ {% include "Interface.kt" %} -class {{ impl_class_name }}( - pointer: Pointer -) : FFIObject(pointer), {{ interface_name }}{ +class {{ impl_class_name }} internal constructor(handleWrapper: UniffiHandleWrapper) + : FFIObject(), {{ interface_name }} { + val handle = handleWrapper.handle {%- match obj.primary_constructor() %} {%- when Some with (cons) %} constructor({% call kt::arg_list_decl(cons) -%}) : - this({% call kt::to_ffi_call(cons) %}) + this(UniffiHandleWrapper({% call kt::to_ffi_call(cons) %})) {%- when None %} {%- endmatch %} @@ -26,10 +26,17 @@ class {{ impl_class_name }}( */ override protected fun freeRustArcPtr() { rustCall() { status -> - _UniFFILib.INSTANCE.{{ obj.ffi_object_free().name() }}(this.pointer, status) + _UniFFILib.INSTANCE.{{ obj.ffi_object_free().name() }}(this.handle, status) } } + internal fun uniffiCloneHandle(): UniffiHandle { + rustCall() { status -> + _UniFFILib.INSTANCE.{{ obj.ffi_object_inc_ref().name() }}(handle, status) + } + return handle + } + {% for meth in obj.methods() -%} {%- match meth.throws_type() -%} {%- when Some with (throwable) %} @@ -40,12 +47,10 @@ class {{ impl_class_name }}( @Suppress("ASSIGNED_BUT_NEVER_ACCESSED_VARIABLE") override suspend fun {{ meth.name()|fn_name }}({%- call kt::arg_list_decl(meth) -%}){% match meth.return_type() %}{% when Some with (return_type) %} : {{ return_type|type_name }}{% when None %}{%- endmatch %} { return uniffiRustCallAsync( - callWithPointer { thisPtr -> - _UniFFILib.INSTANCE.{{ meth.ffi_func().name() }}( - thisPtr, - {% call kt::arg_list_lowered(meth) %} - ) - }, + _UniFFILib.INSTANCE.{{ meth.ffi_func().name() }}( + uniffiCloneHandle(), + {% call kt::arg_list_lowered(meth) %} + ), {{ meth|async_poll(ci) }}, {{ meth|async_complete(ci) }}, {{ meth|async_free(ci) }}, @@ -69,17 +74,13 @@ class {{ impl_class_name }}( {%- match meth.return_type() -%} {%- when Some with (return_type) -%} override fun {{ meth.name()|fn_name }}({% call kt::arg_list_protocol(meth) %}): {{ return_type|type_name }} = - callWithPointer { - {%- call kt::to_ffi_call_with_prefix("it", meth) %} - }.let { + {%- call kt::to_ffi_call_with_prefix("uniffiCloneHandle()", meth) %}.let { {{ return_type|lift_fn }}(it) } {%- when None -%} override fun {{ meth.name()|fn_name }}({% call kt::arg_list_protocol(meth) %}) = - callWithPointer { - {%- call kt::to_ffi_call_with_prefix("it", meth) %} - } + {%- call kt::to_ffi_call_with_prefix("uniffiCloneHandle()", meth) %} {% endmatch %} {% endif %} {% endfor %} @@ -88,7 +89,7 @@ class {{ impl_class_name }}( companion object { {% for cons in obj.alternate_constructors() -%} fun {{ cons.name()|fn_name }}({% call kt::arg_list_decl(cons) %}): {{ impl_class_name }} = - {{ impl_class_name }}({% call kt::to_ffi_call(cons) %}) + {{ impl_class_name }}(UniffiHandleWrapper({% call kt::to_ffi_call(cons) %})) {% endfor %} } {% else %} @@ -103,35 +104,44 @@ class {{ impl_class_name }}( {% include "CallbackInterfaceImpl.kt" %} {%- endif %} -public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, Pointer> { +public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, UniffiHandle> { {%- if obj.is_trait_interface() %} - internal val handleMap = ConcurrentHandleMap<{{ interface_name }}>() + internal val slab = UniffiSlab<{{ type_name }}>() {%- endif %} - override fun lower(value: {{ type_name }}): Pointer { - {%- match obj.imp() %} - {%- when ObjectImpl::Struct %} - return value.callWithPointer { it } - {%- when ObjectImpl::Trait %} - return Pointer(handleMap.insert(value)) - {%- endmatch %} + override fun lower(value: {{ type_name }}): UniffiHandle { + {%- if obj.is_trait_interface() %} + if (value is {{ impl_class_name }}) { + // If we're wrapping a trait implemented in Rust, return that handle directly rather + // than wrapping it again in Kotlin. + return value.uniffiCloneHandle() + } else { + return slab.insert(value) + } + {%- else %} + return value.uniffiCloneHandle() + {%- endif %} } - override fun lift(value: Pointer): {{ type_name }} { - return {{ impl_class_name }}(value) + override fun lift(value: UniffiHandle): {{ type_name }} { + {%- if obj.is_trait_interface() %} + if (uniffiHandleIsFromRust(value)) { + return {{ impl_class_name }}(UniffiHandleWrapper(value)) + } else { + return slab.remove(value) + } + {%- else %} + return {{ impl_class_name }}(UniffiHandleWrapper(value)) + {%- endif %} } override fun read(buf: ByteBuffer): {{ type_name }} { - // The Rust code always writes pointers as 8 bytes, and will - // fail to compile if they don't fit. - return lift(Pointer(buf.getLong())) + return lift(buf.getLong()) } override fun allocationSize(value: {{ type_name }}) = 8 override fun write(value: {{ type_name }}, buf: ByteBuffer) { - // The Rust code always expects pointers written as 8 bytes, - // and will fail to compile if they don't fit. - buf.putLong(Pointer.nativeValue(lower(value))) + buf.putLong(lower(value)) } } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Slab.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Slab.kt new file mode 100644 index 0000000000..1e78985589 --- /dev/null +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Slab.kt @@ -0,0 +1,56 @@ +fun uniffiHandleIsFromRust(handle: UniffiHandle): Boolean { + return (handle and 0x0001_0000_0000) == 0.toLong() +} + +internal class UniffiSlab { + val slabHandle = _UniFFILib.INSTANCE.{{ ci.ffi_slab_new().name() }}() + var lock = ReentrantReadWriteLock() + var items = mutableListOf() + + private fun index(handle: UniffiHandle): Int = (handle and 0xFFFF).toInt() + + internal fun insert(value: T): UniffiHandle { + val handle = _UniFFILib.INSTANCE.{{ ci.ffi_slab_insert().name() }}(slabHandle) + if (handle < 0) { + throw InternalException("Slab insert error") + } + val index = index(handle) + return lock.writeLock().withLock { + while (items.size <= index) { + items.add(null) + } + items[index] = value + handle + } + } + + internal fun get(handle: UniffiHandle): T { + val result = _UniFFILib.INSTANCE.{{ ci.ffi_slab_check_handle().name() }}(slabHandle, handle) + if (result < 0) { + throw InternalException("Slab get error") + } + return lock.readLock().withLock { items[index(handle)]!! } + } + + internal fun incRef(handle: UniffiHandle) { + val result = _UniFFILib.INSTANCE.{{ ci.ffi_slab_inc_ref().name() }}(slabHandle, handle) + if (result < 0) { + throw InternalException("Slab inc-ref error") + } + } + + internal fun remove(handle: UniffiHandle): T { + val result = _UniFFILib.INSTANCE.{{ ci.ffi_slab_dec_ref().name() }}(slabHandle, handle) + if (result < 0) { + throw InternalException("Slab remove error") + } + val index = index(handle) + return lock.writeLock().withLock { + items[index]!!.also { + if (result == 1.toByte()) { + items[index] = null + } + } + } + } +} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt index 9ee4229018..51752ff7a9 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt @@ -29,12 +29,15 @@ import java.nio.ByteOrder import java.nio.CharBuffer import java.nio.charset.CodingErrorAction import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.withLock {%- for req in self.imports() %} {{ req.render() }} {%- endfor %} {% include "RustBufferTemplate.kt" %} +{% include "Slab.kt" %} {% include "FfiConverterTemplate.kt" %} {% include "Helpers.kt" %} diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index 9199bbefd0..700dfc3ece 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -311,19 +311,15 @@ impl PythonCodeOracle { FfiType::UInt64 => "ctypes.c_uint64".to_string(), FfiType::Float32 => "ctypes.c_float".to_string(), FfiType::Float64 => "ctypes.c_double".to_string(), - FfiType::RustArcPtr(_) => "ctypes.c_void_p".to_string(), + FfiType::Handle => "ctypes.c_int64".to_string(), FfiType::RustBuffer(maybe_suffix) => match maybe_suffix { Some(suffix) => format!("_UniffiRustBuffer{suffix}"), None => "_UniffiRustBuffer".to_string(), }, FfiType::ForeignBytes => "_UniffiForeignBytes".to_string(), FfiType::ForeignCallback => "_UNIFFI_FOREIGN_CALLBACK_T".to_string(), - // Pointer to an `asyncio.EventLoop` instance - FfiType::ForeignExecutorHandle => "ctypes.c_size_t".to_string(), FfiType::ForeignExecutorCallback => "_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T".to_string(), - FfiType::RustFutureHandle => "ctypes.c_void_p".to_string(), FfiType::RustFutureContinuationCallback => "_UNIFFI_FUTURE_CONTINUATION_T".to_string(), - FfiType::RustFutureContinuationData => "ctypes.c_size_t".to_string(), } } @@ -417,6 +413,10 @@ pub mod filters { Ok(format!("{}.lift", ffi_converter_name(as_ct)?)) } + pub fn check_fn(as_ct: &impl AsCodeType) -> Result { + Ok(format!("{}.check", ffi_converter_name(as_ct)?)) + } + pub fn lower_fn(as_ct: &impl AsCodeType) -> Result { Ok(format!("{}.lower", ffi_converter_name(as_ct)?)) } diff --git a/uniffi_bindgen/src/bindings/python/templates/Async.py b/uniffi_bindgen/src/bindings/python/templates/Async.py index 51bc31b595..d953999d0f 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Async.py +++ b/uniffi_bindgen/src/bindings/python/templates/Async.py @@ -3,13 +3,13 @@ _UNIFFI_RUST_FUTURE_POLL_MAYBE_READY = 1 # Stores futures for _uniffi_continuation_callback -_UniffiContinuationPointerManager = _UniffiPointerManager() +_UNIFFI_CONTINUATION_SLAB = UniffiSlab() # Continuation callback for async functions # lift the return value or error and resolve the future, causing the async function to resume. @_UNIFFI_FUTURE_CONTINUATION_T def _uniffi_continuation_callback(future_ptr, poll_code): - (eventloop, future) = _UniffiContinuationPointerManager.release_pointer(future_ptr) + (eventloop, future) = _UNIFFI_CONTINUATION_SLAB.remove(future_ptr) eventloop.call_soon_threadsafe(_uniffi_set_future_result, future, poll_code) def _uniffi_set_future_result(future, poll_code): @@ -26,7 +26,7 @@ async def _uniffi_rust_call_async(rust_future, ffi_poll, ffi_complete, ffi_free, ffi_poll( rust_future, _uniffi_continuation_callback, - _UniffiContinuationPointerManager.new_pointer((eventloop, future)), + _UNIFFI_CONTINUATION_SLAB.insert((eventloop, future)), ) poll_code = await future if poll_code == _UNIFFI_RUST_FUTURE_POLL_READY: diff --git a/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py b/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py index 6775e9e132..52ca6a2646 100644 --- a/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py @@ -1,16 +1,20 @@ -class _UniffiConverterBool(_UniffiConverterPrimitive): +class _UniffiConverterBool: @classmethod def check(cls, value): - return not not value + pass + + @classmethod + def lower(cls, value): + return 1 if value else 0 + + @staticmethod + def lift(value): + return value != 0 @classmethod def read(cls, buf): return cls.lift(buf.read_u8()) @classmethod - def write_unchecked(cls, value, buf): + def write(cls, value, buf): buf.write_u8(value) - - @staticmethod - def lift(value): - return value != 0 diff --git a/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py b/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py index 196b5b29fa..f649b5e41a 100644 --- a/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py @@ -7,10 +7,13 @@ def read(buf): return buf.read(size) @staticmethod - def write(value, buf): + def check(value): try: memoryview(value) except TypeError: raise TypeError("a bytes-like object is required, not {!r}".format(type(value).__name__)) + + @staticmethod + def write(value, buf): buf.write_i32(len(value)) buf.write(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py index f9a4768a5c..d0b1264ea4 100644 --- a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py +++ b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py @@ -48,13 +48,13 @@ def makeCallAndHandleReturn(): {% endfor %} - cb = {{ ffi_converter_name }}._handle_map.get(handle) + cb = {{ ffi_converter_name }}._slab.get(handle) if method == IDX_CALLBACK_FREE: - {{ ffi_converter_name }}._handle_map.remove(handle) - - # Successfull return - # See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` + {{ ffi_converter_name }}._slab.remove(handle) + return _UNIFFI_CALLBACK_SUCCESS + if method == IDX_CALLBACK_INC_REF: + {{ ffi_converter_name }}._slab.inc_ref(handle) return _UNIFFI_CALLBACK_SUCCESS {% for meth in methods.iter() -%} diff --git a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py index 1b7346ba4c..6257407c22 100644 --- a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py +++ b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py @@ -1,61 +1,36 @@ import threading -class ConcurrentHandleMap: - """ - A map where inserting, getting and removing data is synchronized with a lock. - """ - - def __init__(self): - # type Handle = int - self._left_map = {} # type: Dict[Handle, Any] - - self._lock = threading.Lock() - self._current_handle = 0 - self._stride = 1 - - def insert(self, obj): - with self._lock: - handle = self._current_handle - self._current_handle += self._stride - self._left_map[handle] = obj - return handle - - def get(self, handle): - with self._lock: - obj = self._left_map.get(handle) - if not obj: - raise InternalError("No callback in handlemap; this is a uniffi bug") - return obj - - def remove(self, handle): - with self._lock: - if handle in self._left_map: - obj = self._left_map.pop(handle) - return obj - -# Magic number for the Rust proxy to call using the same mechanism as every other method, -# to free the callback once it's dropped by Rust. +# Magic numbers for the Rust proxy to call using the same mechanism as every other method. + +# Dec-ref the callback object IDX_CALLBACK_FREE = 0 +# Inc-ref the callback object +IDX_CALLBACK_INC_REF = 0x7FFF_FFFF + # Return codes for callback calls _UNIFFI_CALLBACK_SUCCESS = 0 _UNIFFI_CALLBACK_ERROR = 1 _UNIFFI_CALLBACK_UNEXPECTED_ERROR = 2 class UniffiCallbackInterfaceFfiConverter: - _handle_map = ConcurrentHandleMap() + _slab = UniffiSlab() @classmethod def lift(cls, handle): - return cls._handle_map.get(handle) + return cls._slab.get(handle) @classmethod def read(cls, buf): handle = buf.read_u64() cls.lift(handle) + @classmethod + def check(cls, cb): + pass + @classmethod def lower(cls, cb): - handle = cls._handle_map.insert(cb) + handle = cls._slab.insert(cb) return handle @classmethod diff --git a/uniffi_bindgen/src/bindings/python/templates/CustomType.py b/uniffi_bindgen/src/bindings/python/templates/CustomType.py index 5be6155b84..f7e626c163 100644 --- a/uniffi_bindgen/src/bindings/python/templates/CustomType.py +++ b/uniffi_bindgen/src/bindings/python/templates/CustomType.py @@ -17,6 +17,10 @@ def read(buf): def lift(value): return {{ builtin|ffi_converter_name }}.lift(value) + @staticmethod + def check(value): + return {{ builtin|ffi_converter_name }}.check(value) + @staticmethod def lower(value): return {{ builtin|ffi_converter_name }}.lower(value) @@ -51,6 +55,11 @@ def lift(value): builtin_value = {{ builtin|lift_fn }}(value) return {{ config.into_custom.render("builtin_value") }} + @staticmethod + def check(value): + builtin_value = {{ config.from_custom.render("value") }} + return {{ builtin|check_fn }}(builtin_value) + @staticmethod def lower(value): builtin_value = {{ config.from_custom.render("value") }} diff --git a/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py b/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py index 10974e009d..3bb06e1099 100644 --- a/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py @@ -12,10 +12,14 @@ def read(buf): return datetime.timedelta(seconds=seconds, microseconds=microseconds) @staticmethod - def write(value, buf): + def check(value): seconds = value.seconds + value.days * 24 * 3600 - nanoseconds = value.microseconds * 1000 if seconds < 0: raise ValueError("Invalid duration, must be non-negative") + + @staticmethod + def write(value, buf): + seconds = value.seconds + value.days * 24 * 3600 + nanoseconds = value.microseconds * 1000 buf.write_i64(seconds) buf.write_u32(nanoseconds) diff --git a/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py b/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py index 84d089baf9..2a347f873e 100644 --- a/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py @@ -81,6 +81,25 @@ def read(buf): {%- endfor %} raise InternalError("Raw enum value doesn't match any cases") + @staticmethod + def check(value): + {%- if e.variants().is_empty() %} + pass + {%- else %} + {%- for variant in e.variants() %} + {%- if e.is_flat() %} + if value == {{ type_name }}.{{ variant.name()|enum_variant_py }}: + {%- else %} + if value.is_{{ variant.name()|var_name }}(): + {%- endif %} + {%- for field in variant.fields() %} + {{ field|check_fn }}(value.{{ field.name()|var_name }}) + {%- endfor %} + return + {%- endfor %} + {%- endif %} + + @staticmethod def write(value, buf): {%- for variant in e.variants() %} {%- if e.is_flat() %} diff --git a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py index 26a1e6452a..b4c9849626 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py @@ -59,6 +59,20 @@ def read(buf): {%- endfor %} raise InternalError("Raw enum value doesn't match any cases") + @staticmethod + def check(value): + {%- if e.variants().is_empty() %} + pass + {%- else %} + {%- for variant in e.variants() %} + if isinstance(value, {{ type_name }}.{{ variant.name()|class_name }}): + {%- for field in variant.fields() %} + {{ field|check_fn }}(value.{{ field.name()|var_name }}) + {%- endfor %} + return + {%- endfor %} + {%- endif %} + @staticmethod def write(value, buf): {%- for variant in e.variants() %} diff --git a/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py b/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py index a52107a638..49a1a7286e 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py @@ -4,5 +4,5 @@ def read(buf): return buf.read_float() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_float(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py b/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py index 772f5080e9..e2084c7b13 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py @@ -4,5 +4,5 @@ def read(buf): return buf.read_double() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_double(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py index 6a6932fed0..166fa95b42 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py @@ -9,33 +9,36 @@ _UNIFFI_FOREIGN_EXECUTOR_CALLBACK_ERROR = 2 class {{ ffi_converter_name }}: - _pointer_manager = _UniffiPointerManager() + _slab = UniffiSlab() @classmethod - def lower(cls, eventloop): + def check(cls, eventloop): if not isinstance(eventloop, asyncio.BaseEventLoop): raise TypeError("_uniffi_executor_callback: Expected EventLoop instance") - return cls._pointer_manager.new_pointer(eventloop) + + @classmethod + def lower(cls, eventloop): + return cls._slab.insert(eventloop) @classmethod def write(cls, eventloop, buf): - buf.write_c_size_t(cls.lower(eventloop)) + buf.write_i64(cls.lower(eventloop)) @classmethod def read(cls, buf): - return cls.lift(buf.read_c_size_t()) + return cls.lift(buf.read_i64()) @classmethod def lift(cls, value): - return cls._pointer_manager.lookup(value) + return cls._slab.get(value) @_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T -def _uniffi_executor_callback(eventloop_address, delay, task_ptr, task_data): +def _uniffi_executor_callback(handle, delay, task_ptr, task_data): if task_ptr is None: - {{ ffi_converter_name }}._pointer_manager.release_pointer(eventloop_address) + {{ ffi_converter_name }}._slab.remove(handle) return _UNIFFI_FOREIGN_EXECUTOR_CALLBACK_SUCCESS else: - eventloop = {{ ffi_converter_name }}._pointer_manager.lookup(eventloop_address) + eventloop = {{ ffi_converter_name }}._slab.get(handle) if eventloop.is_closed(): return _UNIFFI_FOREIGN_EXECUTOR_CALLBACK_CANCELED diff --git a/uniffi_bindgen/src/bindings/python/templates/Helpers.py b/uniffi_bindgen/src/bindings/python/templates/Helpers.py index dca962f176..9d1c202e40 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Helpers.py +++ b/uniffi_bindgen/src/bindings/python/templates/Helpers.py @@ -71,5 +71,5 @@ def _uniffi_check_call_status(error_ffi_converter, call_status): _UNIFFI_FOREIGN_CALLBACK_T = ctypes.CFUNCTYPE(ctypes.c_int, ctypes.c_ulonglong, ctypes.c_ulong, ctypes.POINTER(ctypes.c_char), ctypes.c_int, ctypes.POINTER(_UniffiRustBuffer)) # UniFFI future continuation -_UNIFFI_FUTURE_CONTINUATION_T = ctypes.CFUNCTYPE(None, ctypes.c_size_t, ctypes.c_int8) +_UNIFFI_FUTURE_CONTINUATION_T = ctypes.CFUNCTYPE(None, ctypes.c_int64, ctypes.c_int8) diff --git a/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py index 99f19dc1c0..befa563384 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i16() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i16(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py index 3b142c8749..70644f6717 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i32() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i32(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py index 6e94379cbf..232f127bd6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i64() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i64(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py index 732530e3cb..c1de1625e7 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i8() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i8(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py b/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py index 387227ed09..6f413bb7e6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py @@ -2,6 +2,12 @@ {%- let value_ffi_converter = value_type|ffi_converter_name %} class {{ ffi_converter_name }}(_UniffiConverterRustBuffer): + @classmethod + def check(cls, items): + for (key, value) in items.items(): + {{ key_ffi_converter }}.check(key) + {{ value_ffi_converter }}.check(value) + @classmethod def write(cls, items, buf): buf.write_i32(len(items)) diff --git a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py index fac6cd5564..581a723f5c 100644 --- a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py @@ -14,7 +14,7 @@ However, when task is NULL this indicates that Rust has dropped the ForeignExecutor and we should decrease the EventLoop refcount. """ -_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T = ctypes.CFUNCTYPE(ctypes.c_int8, ctypes.c_size_t, ctypes.c_uint32, ctypes.c_void_p, ctypes.c_void_p) +_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T = ctypes.CFUNCTYPE(ctypes.c_int8, ctypes.c_int64, ctypes.c_uint32, ctypes.c_void_p, ctypes.c_void_p) """ Function pointer for a Rust task, which a callback function that takes a opaque pointer diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index 097fcd4d1d..e23970eaf6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -5,29 +5,33 @@ {% include "Protocol.py" %} class {{ impl_name }}: - _pointer: ctypes.c_void_p + _uniffi_handle: ctypes.c_int64 {%- match obj.primary_constructor() %} {%- when Some with (cons) %} def __init__(self, {% call py::arg_list_decl(cons) -%}): {%- call py::setup_args_extra_indent(cons) %} - self._pointer = {% call py::to_ffi_call(cons) %} + self._uniffi_handle = {% call py::to_ffi_call(cons) %} {%- when None %} {%- endmatch %} def __del__(self): # In case of partial initialization of instances. - pointer = getattr(self, "_pointer", None) - if pointer is not None: - _rust_call(_UniffiLib.{{ obj.ffi_object_free().name() }}, pointer) + handle = getattr(self, "_uniffi_handle", None) + if handle is not None: + _rust_call(_UniffiLib.{{ obj.ffi_object_free().name() }}, handle) + + def _uniffi_clone_handle(self): + _rust_call(_UniffiLib.{{ obj.ffi_object_inc_ref().name() }}, self._uniffi_handle) + return self._uniffi_handle # Used by alternative constructors or any methods which return this type. @classmethod - def _make_instance_(cls, pointer): + def _make_instance_(cls, handle): # Lightly yucky way to bypass the usual __init__ logic - # and just create a new instance with the required pointer. + # and just create a new instance with the required handle. inst = cls.__new__(cls) - inst._pointer = pointer + inst._uniffi_handle = handle return inst {%- for cons in obj.alternate_constructors() %} @@ -35,9 +39,8 @@ def _make_instance_(cls, pointer): @classmethod def {{ cons.name()|fn_name }}(cls, {% call py::arg_list_decl(cons) %}): {%- call py::setup_args_extra_indent(cons) %} - # Call the (fallible) function before creating any half-baked object instances. - pointer = {% call py::to_ffi_call(cons) %} - return cls._make_instance_(pointer) + uniffi_handle = {% call py::to_ffi_call(cons) %} + return cls._make_instance_(uniffi_handle) {% endfor %} {%- for meth in obj.methods() -%} @@ -55,13 +58,13 @@ def __eq__(self, other: object) -> {{ eq.return_type().unwrap()|type_name }}: if not isinstance(other, {{ type_name }}): return NotImplemented - return {{ eq.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._pointer", eq) %}) + return {{ eq.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._uniffi_handle", eq) %}) def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}: if not isinstance(other, {{ type_name }}): return NotImplemented - return {{ ne.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._pointer", ne) %}) + return {{ ne.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._uniffi_handle", ne) %}) {%- when UniffiTrait::Hash { hash } %} {%- call py::method_decl("__hash__", hash) %} {% endmatch %} @@ -76,31 +79,48 @@ def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}: class {{ ffi_converter_name }}: {%- if obj.is_trait_interface() %} - _handle_map = ConcurrentHandleMap() + _slab = UniffiSlab() {%- endif %} @staticmethod - def lift(value: int): + def lift(value: UniffiHandle): + {%- if obj.is_trait_interface() %} + if uniffi_handle_is_from_rust(value): + return {{ impl_name }}._make_instance_(value) + else: + return {{ ffi_converter_name }}._slab.remove(value) + {%- else %} return {{ impl_name }}._make_instance_(value) + {%- endif %} @staticmethod - def lower(value: {{ protocol_name }}): - {%- match obj.imp() %} - {%- when ObjectImpl::Struct %} + def check(value: {{ type_name }}): + {%- if obj.is_trait_interface() %} + pass + {%- else %} if not isinstance(value, {{ impl_name }}): raise TypeError("Expected {{ impl_name }} instance, {} found".format(type(value).__name__)) - return value._pointer - {%- when ObjectImpl::Trait %} - return {{ ffi_converter_name }}._handle_map.insert(value) - {%- endmatch %} + {%- endif %} + + @staticmethod + def lower(value: {{ type_name }}): + {%- if obj.is_trait_interface() %} + _uniffi_clone_handle = getattr(value, '_uniffi_clone_handle', None) + if _uniffi_clone_handle is not None: + # If we're wrapping a trait implemented in Rust, return that handle directly rather than + # wrapping it again in Python. + return _uniffi_clone_handle() + else: + return {{ ffi_converter_name }}._slab.insert(value) + {%- else %} + return value._uniffi_clone_handle() + {%- endif %} @classmethod def read(cls, buf: _UniffiRustBuffer): - ptr = buf.read_u64() - if ptr == 0: - raise InternalError("Raw pointer value was null") + ptr = buf.read_i64() return cls.lift(ptr) @classmethod - def write(cls, value: {{ protocol_name }}, buf: _UniffiRustBuffer): - buf.write_u64(cls.lower(value)) + def write(cls, value: {{ type_name }}, buf: _UniffiRustBuffer): + buf.write_i64(cls.lower(value)) diff --git a/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py b/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py index e406c51d49..14b7a0f3af 100644 --- a/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py @@ -1,6 +1,11 @@ {%- let inner_ffi_converter = inner_type|ffi_converter_name %} class {{ ffi_converter_name }}(_UniffiConverterRustBuffer): + @classmethod + def check(cls, value): + if value is not None: + {{ inner_ffi_converter }}.check(value) + @classmethod def write(cls, value, buf): if value is None: diff --git a/uniffi_bindgen/src/bindings/python/templates/PointerManager.py b/uniffi_bindgen/src/bindings/python/templates/PointerManager.py deleted file mode 100644 index 23aa28eab4..0000000000 --- a/uniffi_bindgen/src/bindings/python/templates/PointerManager.py +++ /dev/null @@ -1,68 +0,0 @@ -class _UniffiPointerManagerCPython: - """ - Manage giving out pointers to Python objects on CPython - - This class is used to generate opaque pointers that reference Python objects to pass to Rust. - It assumes a CPython platform. See _UniffiPointerManagerGeneral for the alternative. - """ - - def new_pointer(self, obj): - """ - Get a pointer for an object as a ctypes.c_size_t instance - - Each call to new_pointer() must be balanced with exactly one call to release_pointer() - - This returns a ctypes.c_size_t. This is always the same size as a pointer and can be - interchanged with pointers for FFI function arguments and return values. - """ - # IncRef the object since we're going to pass a pointer to Rust - ctypes.pythonapi.Py_IncRef(ctypes.py_object(obj)) - # id() is the object address on CPython - # (https://docs.python.org/3/library/functions.html#id) - return id(obj) - - def release_pointer(self, address): - py_obj = ctypes.cast(address, ctypes.py_object) - obj = py_obj.value - ctypes.pythonapi.Py_DecRef(py_obj) - return obj - - def lookup(self, address): - return ctypes.cast(address, ctypes.py_object).value - -class _UniffiPointerManagerGeneral: - """ - Manage giving out pointers to Python objects on non-CPython platforms - - This has the same API as _UniffiPointerManagerCPython, but doesn't assume we're running on - CPython and is slightly slower. - - Instead of using real pointers, it maps integer values to objects and returns the keys as - c_size_t values. - """ - - def __init__(self): - self._map = {} - self._lock = threading.Lock() - self._current_handle = 0 - - def new_pointer(self, obj): - with self._lock: - handle = self._current_handle - self._current_handle += 1 - self._map[handle] = obj - return handle - - def release_pointer(self, handle): - with self._lock: - return self._map.pop(handle) - - def lookup(self, handle): - with self._lock: - return self._map[handle] - -# Pick an pointer manager implementation based on the platform -if platform.python_implementation() == 'CPython': - _UniffiPointerManager = _UniffiPointerManagerCPython # type: ignore -else: - _UniffiPointerManager = _UniffiPointerManagerGeneral # type: ignore diff --git a/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py index 99a30e120f..b2420b6671 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py @@ -42,6 +42,16 @@ def read(buf): {%- endfor %} ) + @staticmethod + def check(value): + {%- if rec.fields().is_empty() %} + pass + {%- else %} + {%- for field in rec.fields() %} + {{ field|check_fn }}(value.{{ field.name()|var_name }}) + {%- endfor %} + {%- endif %} + @staticmethod def write(value, buf): {%- for field in rec.fields() %} diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py index daabd5b4b9..88cc96b6fb 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py @@ -1,25 +1,13 @@ # Types conforming to `_UniffiConverterPrimitive` pass themselves directly over the FFI. class _UniffiConverterPrimitive: - @classmethod - def check(cls, value): - return value - @classmethod def lift(cls, value): return value - + @classmethod def lower(cls, value): - return cls.lowerUnchecked(cls.check(value)) - - @classmethod - def lowerUnchecked(cls, value): return value - @classmethod - def write(cls, value, buf): - cls.write_unchecked(cls.check(value), buf) - class _UniffiConverterPrimitiveInt(_UniffiConverterPrimitive): @classmethod def check(cls, value): @@ -31,7 +19,6 @@ def check(cls, value): raise TypeError("__index__ returned non-int (type {})".format(type(value).__name__)) if not cls.VALUE_MIN <= value < cls.VALUE_MAX: raise ValueError("{} requires {} <= value < {}".format(cls.CLASS_NAME, cls.VALUE_MIN, cls.VALUE_MAX)) - return super().check(value) class _UniffiConverterPrimitiveFloat(_UniffiConverterPrimitive): @classmethod @@ -42,7 +29,6 @@ def check(cls, value): raise TypeError("must be real number, not {}".format(type(value).__name__)) if not isinstance(value, float): raise TypeError("__float__ returned non-float (type {})".format(type(value).__name__)) - return super().check(value) # Helper class for wrapper types that will always go through a _UniffiRustBuffer. # Classes should inherit from this and implement the `read` and `write` static methods. diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py index c317a632fc..35efeb2ab9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py @@ -137,9 +137,6 @@ def read_float(self): def read_double(self): return self._unpack_from(8, ">d") - def read_c_size_t(self): - return self._unpack_from(ctypes.sizeof(ctypes.c_size_t) , "@N") - class _UniffiRustBufferBuilder: """ Helper for structured writing of bytes into a _UniffiRustBuffer. @@ -206,6 +203,3 @@ def write_float(self, v): def write_double(self, v): self._pack_into(8, ">d", v) - - def write_c_size_t(self, v): - self._pack_into(ctypes.sizeof(ctypes.c_size_t) , "@N", v) diff --git a/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py b/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py index 3c9f5a4596..c69a36f178 100644 --- a/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py @@ -1,6 +1,11 @@ {%- let inner_ffi_converter = inner_type|ffi_converter_name %} class {{ ffi_converter_name}}(_UniffiConverterRustBuffer): + @classmethod + def check(cls, value): + for item in value: + {{ inner_ffi_converter }}.check(item) + @classmethod def write(cls, value, buf): items = len(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Slab.py b/uniffi_bindgen/src/bindings/python/templates/Slab.py new file mode 100644 index 0000000000..59a80cd171 --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/templates/Slab.py @@ -0,0 +1,52 @@ +UniffiHandle = typing.NewType('UniffiHandle', int) + +def uniffi_handle_is_from_rust(handle: int) -> bool: + return (handle & 0x0001_0000_0000) == 0 + +# TODO: it would be nice to make this a generic class, however let's wait until python 3.11 is the +# minimum version, so we can do that without having to add a `TypeVar` to the top-level namespace. +class UniffiSlab: + def __init__(self): + self.slab_handle = _UniffiLib.{{ ci.ffi_slab_new().name() }}() + # We don't need a lock around the items list, since `list.append()` is atomic: + # * Python FAQ: https://docs.python.org/2/faq/library.html#what-kinds-of-global-value-mutation-are-thread-safe + # * PEP 703 – Making the Global Interpreter Lock Optional in CPython includes a section + # specifying that `list.append()` should stay atomic: https://peps.python.org/pep-0703/#container-thread-safety + self.items = [] + + def __del__(self): + _UniffiLib.{{ ci.ffi_slab_free().name() }}(self.slab_handle) + + def _index(self, handle: UniffiHandle) -> int: + return handle & 0xFFFF + + def insert(self, value: object) -> UniffiHandle: + handle = _UniffiLib.{{ ci.ffi_slab_insert().name() }}(self.slab_handle) + if handle < 0: + raise InternalError("Slab insert error") + index = self._index(handle) + while len(self.items) <= index: + self.items.append(None) + self.items[index] = value + return handle + + def get(self, handle: UniffiHandle) -> object: + result = _UniffiLib.{{ ci.ffi_slab_check_handle().name() }}(self.slab_handle, handle) + if result < 0: + raise InternalError("Slab get error") + return self.items[self._index(handle)] + + def inc_ref(self, handle: UniffiHandle): + result = _UniffiLib.{{ ci.ffi_slab_inc_ref().name() }}(self.slab_handle, handle) + if result < 0: + raise InternalError("Slab inc-ref error") + + def remove(self, handle: UniffiHandle) -> object: + result = _UniffiLib.{{ ci.ffi_slab_dec_ref().name() }}(self.slab_handle, handle) + if result < 0: + raise InternalError("Slab remove error") + index = self._index(handle) + value = self.items[index] + if result == 1: + self.items[index] = None + return value diff --git a/uniffi_bindgen/src/bindings/python/templates/StringHelper.py b/uniffi_bindgen/src/bindings/python/templates/StringHelper.py index 40890b6abc..80a014c888 100644 --- a/uniffi_bindgen/src/bindings/python/templates/StringHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/StringHelper.py @@ -15,7 +15,6 @@ def read(buf): @staticmethod def write(value, buf): - value = _UniffiConverterString.check(value) utf8_bytes = value.encode("utf-8") buf.write_i32(len(utf8_bytes)) buf.write(utf8_bytes) @@ -27,7 +26,6 @@ def lift(buf): @staticmethod def lower(value): - value = _UniffiConverterString.check(value) with _UniffiRustBuffer.alloc_with_builder() as builder: builder.write(value.encode("utf-8")) return builder.finalize() diff --git a/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py b/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py index 8402f6095d..9acbb2fee9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py @@ -17,6 +17,10 @@ def read(buf): else: return datetime.datetime.fromtimestamp(0, tz=datetime.timezone.utc) - datetime.timedelta(seconds=-seconds, microseconds=microseconds) + @staticmethod + def check(value): + pass + @staticmethod def write(value, buf): if value >= datetime.datetime.fromtimestamp(0, datetime.timezone.utc): diff --git a/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py b/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py index f258b60a1c..8faa11bedf 100644 --- a/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py @@ -1,6 +1,7 @@ {%- if func.is_async() %} def {{ func.name()|fn_name }}({%- call py::arg_list_decl(func) -%}): + {%- call py::setup_args(func) %} return _uniffi_rust_call_async( _UniffiLib.{{ func.ffi_func().name() }}({% call py::arg_list_lowered(func) %}), _UniffiLib.{{func.ffi_rust_future_poll(ci) }}, diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py index 081c6731ce..039bf76162 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u16() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u16(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py index b80e75177d..1650bf9b60 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u32() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u32(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py index 4b87e58547..f354545e26 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u64() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u64(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py index 33026706f2..cee130b4d9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u8() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u8(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/macros.py b/uniffi_bindgen/src/bindings/python/templates/macros.py index ef3b1bb94d..3359e759e6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/macros.py +++ b/uniffi_bindgen/src/bindings/python/templates/macros.py @@ -70,6 +70,7 @@ #} {%- macro setup_args(func) %} {%- for arg in func.arguments() %} + {{ arg|check_fn }}({{ arg.name()|var_name }}) {%- match arg.default_value() %} {%- when None %} {%- when Some with(literal) %} @@ -85,6 +86,7 @@ #} {%- macro setup_args_extra_indent(func) %} {%- for arg in func.arguments() %} + {{ arg|check_fn }}({{ arg.name()|var_name }}) {%- match arg.default_value() %} {%- when None %} {%- when Some with(literal) %} @@ -104,7 +106,7 @@ def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}): {%- call setup_args_extra_indent(meth) %} return _uniffi_rust_call_async( _UniffiLib.{{ meth.ffi_func().name() }}( - self._pointer, {% call arg_list_lowered(meth) %} + self._uniffi_clone_handle(), {% call arg_list_lowered(meth) %} ), _UniffiLib.{{ meth.ffi_rust_future_poll(ci) }}, _UniffiLib.{{ meth.ffi_rust_future_complete(ci) }}, @@ -133,14 +135,14 @@ def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}): def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}) -> "{{ return_type|type_name }}": {%- call setup_args_extra_indent(meth) %} return {{ return_type|lift_fn }}( - {% call to_ffi_call_with_prefix("self._pointer", meth) %} + {% call to_ffi_call_with_prefix("self._uniffi_clone_handle()", meth) %} ) {%- when None %} def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}): {%- call setup_args_extra_indent(meth) %} - {% call to_ffi_call_with_prefix("self._pointer", meth) %} + {% call to_ffi_call_with_prefix("self._uniffi_clone_handle()", meth) %} {% endmatch %} {% endif %} diff --git a/uniffi_bindgen/src/bindings/python/templates/wrapper.py b/uniffi_bindgen/src/bindings/python/templates/wrapper.py index 24c3290ff7..f2799a57b5 100644 --- a/uniffi_bindgen/src/bindings/python/templates/wrapper.py +++ b/uniffi_bindgen/src/bindings/python/templates/wrapper.py @@ -33,8 +33,8 @@ _DEFAULT = object() {% include "RustBufferTemplate.py" %} +{% include "Slab.py" %} {% include "Helpers.py" %} -{% include "PointerManager.py" %} {% include "RustBufferHelper.py" %} # Contains loading, initialization code, and the FFI Function declarations. diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs index e6defe65cc..d8bb65df47 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -150,7 +150,7 @@ mod filters { FfiType::UInt64 => ":uint64".to_string(), FfiType::Float32 => ":float".to_string(), FfiType::Float64 => ":double".to_string(), - FfiType::RustArcPtr(_) => ":pointer".to_string(), + FfiType::Handle => ":int64".to_string(), FfiType::RustBuffer(_) => "RustBuffer.by_value".to_string(), FfiType::ForeignBytes => "ForeignBytes".to_string(), // Callback interfaces are not yet implemented, but this needs to return something in @@ -159,12 +159,7 @@ mod filters { FfiType::ForeignExecutorCallback => { unimplemented!("Foreign executors are not implemented") } - FfiType::ForeignExecutorHandle => { - unimplemented!("Foreign executors are not implemented") - } - FfiType::RustFutureHandle - | FfiType::RustFutureContinuationCallback - | FfiType::RustFutureContinuationData => { + FfiType::RustFutureContinuationCallback => { unimplemented!("Async functions are not implemented") } }) @@ -270,6 +265,22 @@ mod filters { }) } + pub fn check_rb(nm: &str, type_: &Type) -> Result { + Ok(match type_ { + Type::Object { name, .. } => format!("({}._uniffi_check {nm})", class_name_rb(name)?), + Type::Enum { .. } + | Type::Record { .. } + | Type::Optional { .. } + | Type::Sequence { .. } + | Type::Map { .. } => format!( + "RustBuffer.check_{}({})", + class_name_rb(&canonical_name(type_))?, + nm + ), + _ => "".to_owned(), + }) + } + pub fn lower_rb(nm: &str, type_: &Type) -> Result { Ok(match type_ { Type::Int8 diff --git a/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb index 677c5c729b..e221a7c427 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb @@ -1,54 +1,64 @@ class {{ obj.name()|class_name_rb }} - # A private helper for initializing instances of the class from a raw pointer, + # A private helper for initializing instances of the class from a handle, # bypassing any initialization logic and ensuring they are GC'd properly. - def self._uniffi_allocate(pointer) - pointer.autorelease = false + def self._uniffi_allocate(handle) inst = allocate - inst.instance_variable_set :@pointer, pointer - ObjectSpace.define_finalizer(inst, _uniffi_define_finalizer_by_pointer(pointer, inst.object_id)) + inst.instance_variable_set :@handle, handle + ObjectSpace.define_finalizer(inst, _uniffi_define_finalizer_by_handle(handle, inst.object_id)) return inst end # 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) + # to the actual instance, only its underlying handle. + def self._uniffi_define_finalizer_by_handle(handle, object_id) Proc.new do |_id| {{ ci.namespace()|class_name_rb }}.rust_call( :{{ obj.ffi_object_free().name() }}, - pointer + handle ) end end - # A private helper for lowering instances into a raw pointer. + def _uniffi_clone_handle() + {{ ci.namespace()|class_name_rb }}.rust_call( + :{{ obj.ffi_object_inc_ref().name() }}, + @handle + ) + return @handle + end + + # Private helpers for lowering instances into a handle. # This does an explicit typecheck, because accidentally lowering a different type of # object in a place where this type is expected, could lead to memory unsafety. - def self._uniffi_lower(inst) + def self._uniffi_check(inst) if not inst.is_a? self raise TypeError.new "Expected a {{ obj.name()|class_name_rb }} instance, got #{inst}" end - return inst.instance_variable_get :@pointer + end + + def self._uniffi_lower(inst) + return inst._uniffi_clone_handle() 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)) + {%- call rb::setup_args_extra_indent(cons) %} + handle = {% call rb::to_ffi_call(cons) %} + @handle = handle + ObjectSpace.define_finalizer(self, self.class._uniffi_define_finalizer_by_handle(handle, self.object_id)) end {%- when None %} {%- endmatch %} {% for cons in obj.alternate_constructors() -%} def self.{{ cons.name()|fn_name_rb }}({% call rb::arg_list_decl(cons) %}) - {%- call rb::coerce_args_extra_indent(cons) %} + {%- call rb::setup_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 pointer. + # and just create a new instance with the required handle. return _uniffi_allocate({% call rb::to_ffi_call(cons) %}) end {% endfor %} @@ -58,15 +68,15 @@ 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("@pointer", meth) %} + {%- call rb::setup_args_extra_indent(meth) %} + result = {% call rb::to_ffi_call_with_prefix("_uniffi_clone_handle()", 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("@pointer", meth) %} + {%- call rb::setup_args_extra_indent(meth) %} + {% call rb::to_ffi_call_with_prefix("_uniffi_clone_handle()", 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 8749139116..24f5d83ff6 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferBuilder.rb @@ -163,8 +163,8 @@ def write_{{ canonical_type_name }}(v) # The Object type {{ object_name }}. def write_{{ canonical_type_name }}(obj) - pointer = {{ object_name|class_name_rb}}._uniffi_lower obj - pack_into(8, 'Q>', pointer.address) + handle = {{ object_name|class_name_rb}}._uniffi_lower obj + pack_into(8, 'Q>', handle) end {% when Type::Enum { name: enum_name, module_path } -%} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb index b085dddf15..ed593672d7 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferStream.rb @@ -154,8 +154,8 @@ def read{{ canonical_type_name }} # The Object type {{ object_name }}. def read{{ canonical_type_name }} - pointer = FFI::Pointer.new unpack_from 8, 'Q>' - return {{ object_name|class_name_rb }}._uniffi_allocate(pointer) + handle = unpack_from 8, 'Q>' + return {{ object_name|class_name_rb }}._uniffi_allocate(handle) end {% when Type::Enum { name, module_path } -%} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb index 0194c9666d..154867e7ee 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb @@ -128,6 +128,12 @@ def consumeInto{{ canonical_type_name }} {%- let rec = ci|get_record_definition(record_name) -%} # The Record type {{ record_name }}. + def self.check_{{ canonical_type_name }}(v) + {%- for field in rec.fields() %} + {{ "v.{}"|format(field.name()|var_name_rb)|check_rb(field.as_type().borrow()) }} + {%- endfor %} + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) @@ -146,6 +152,19 @@ def consumeInto{{ canonical_type_name }} {%- let e = ci|get_enum_definition(enum_name) -%} # The Enum type {{ enum_name }}. + def self.check_{{ canonical_type_name }}(v) + {%- if !e.is_flat() %} + {%- for variant in e.variants() %} + if v.{{ variant.name()|var_name_rb }}? + {%- for field in variant.fields() %} + {{ "v.{}"|format(field.name())|check_rb(field.as_type().borrow()) }} + {%- endfor %} + return + end + {%- endfor %} + {%- endif %} + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) @@ -162,6 +181,12 @@ def consumeInto{{ canonical_type_name }} {% when Type::Optional { inner_type } -%} # The Optional type for {{ canonical_name(inner_type) }}. + + def self.check_{{ canonical_type_name }}(v) + if not v.nil? + {{ "v"|check_rb(inner_type.borrow()) }} + end + end def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| @@ -179,6 +204,12 @@ def consumeInto{{ canonical_type_name }} {% when Type::Sequence { inner_type } -%} # The Sequence type for {{ canonical_name(inner_type) }}. + def self.check_{{ canonical_type_name }}(v) + v.each do |item| + {{ "item"|check_rb(inner_type.borrow()) }} + end + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) @@ -195,6 +226,13 @@ def consumeInto{{ canonical_type_name }} {% when Type::Map { key_type: k, value_type: inner_type } -%} # The Map type for {{ canonical_name(inner_type) }}. + def self.check_{{ canonical_type_name }}(v) + v.each do |k, v| + {{ "k"|check_rb(k.borrow()) }} + {{ "v"|check_rb(inner_type.borrow()) }} + end + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) diff --git a/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb index 13214cf31b..b6dce0effa 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb @@ -2,7 +2,7 @@ {%- when Some with (return_type) %} def self.{{ func.name()|fn_name_rb }}({%- call rb::arg_list_decl(func) -%}) - {%- call rb::coerce_args(func) %} + {%- call rb::setup_args(func) %} result = {% call rb::to_ffi_call(func) %} return {{ "result"|lift_rb(return_type) }} end @@ -10,7 +10,7 @@ def self.{{ func.name()|fn_name_rb }}({%- call rb::arg_list_decl(func) -%}) {% when None %} def self.{{ func.name()|fn_name_rb }}({%- call rb::arg_list_decl(func) -%}) - {%- call rb::coerce_args(func) %} + {%- call rb::setup_args(func) %} {% call rb::to_ffi_call(func) %} end {% endmatch %} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/macros.rb b/uniffi_bindgen/src/bindings/ruby/templates/macros.rb index 8dc3e5e613..e9962f5831 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/macros.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/macros.rb @@ -60,14 +60,16 @@ [{%- for arg in func.arguments() -%}{{ arg.type_().borrow()|type_ffi }}, {% endfor -%} RustCallStatus.by_ref] {%- endmacro -%} -{%- macro coerce_args(func) %} +{%- macro setup_args(func) %} {%- for arg in func.arguments() %} - {{ arg.name() }} = {{ arg.name()|coerce_rb(ci.namespace()|class_name_rb, arg.as_type().borrow()) -}} + {{ arg.name() }} = {{ arg.name()|coerce_rb(ci.namespace()|class_name_rb, arg.as_type().borrow()) }} + {{ arg.name()|check_rb(arg.as_type().borrow()) }} {% endfor -%} {%- endmacro -%} -{%- macro coerce_args_extra_indent(func) %} - {%- for arg in func.arguments() %} +{%- macro setup_args_extra_indent(meth) %} + {%- for arg in meth.arguments() %} {{ arg.name() }} = {{ arg.name()|coerce_rb(ci.namespace()|class_name_rb, arg.as_type().borrow()) }} + {{ arg.name()|check_rb(arg.as_type().borrow()) }} {%- endfor %} {%- endmacro -%} diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs index 1ba385d361..6a3d35574e 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -455,16 +455,17 @@ impl SwiftCodeOracle { FfiType::UInt64 => "UInt64".into(), FfiType::Float32 => "Float".into(), FfiType::Float64 => "Double".into(), - FfiType::RustArcPtr(_) => "UnsafeMutableRawPointer".into(), + // Other bindings use an type alias for `Handle`, but this isn't so easy with Swift: + // * When multiple crates are built together, all swift files get merged into a + // single module which can lead to namespace conflicts. + // * `fileprivate` type aliases can't be used in a public API, even if the actual + // type is public. + FfiType::Handle => "Int64".into(), FfiType::RustBuffer(_) => "RustBuffer".into(), FfiType::ForeignBytes => "ForeignBytes".into(), FfiType::ForeignCallback => "ForeignCallback".into(), - FfiType::ForeignExecutorHandle => "Int".into(), FfiType::ForeignExecutorCallback => "ForeignExecutorCallback".into(), FfiType::RustFutureContinuationCallback => "UniFfiRustFutureContinuation".into(), - FfiType::RustFutureHandle | FfiType::RustFutureContinuationData => { - "UnsafeMutableRawPointer".into() - } } } @@ -472,9 +473,7 @@ impl SwiftCodeOracle { match ffi_type { FfiType::ForeignCallback | FfiType::ForeignExecutorCallback - | FfiType::RustFutureHandle - | FfiType::RustFutureContinuationCallback - | FfiType::RustFutureContinuationData => { + | FfiType::RustFutureContinuationCallback => { format!("{} _Nonnull", self.ffi_type_label_raw(ffi_type)) } _ => self.ffi_type_label_raw(ffi_type), @@ -571,18 +570,14 @@ pub mod filters { FfiType::UInt64 => "uint64_t".into(), FfiType::Float32 => "float".into(), FfiType::Float64 => "double".into(), - FfiType::RustArcPtr(_) => "void*_Nonnull".into(), + FfiType::Handle => "int64_t".into(), FfiType::RustBuffer(_) => "RustBuffer".into(), FfiType::ForeignBytes => "ForeignBytes".into(), FfiType::ForeignCallback => "ForeignCallback _Nonnull".into(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback _Nonnull".into(), - FfiType::ForeignExecutorHandle => "size_t".into(), FfiType::RustFutureContinuationCallback => { "UniFfiRustFutureContinuation _Nonnull".into() } - FfiType::RustFutureHandle | FfiType::RustFutureContinuationData => { - "void* _Nonnull".into() - } }) } diff --git a/uniffi_bindgen/src/bindings/swift/templates/Async.swift b/uniffi_bindgen/src/bindings/swift/templates/Async.swift index f947408182..2412ef8aa5 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Async.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Async.swift @@ -1,11 +1,13 @@ private let UNIFFI_RUST_FUTURE_POLL_READY: Int8 = 0 private let UNIFFI_RUST_FUTURE_POLL_MAYBE_READY: Int8 = 1 +fileprivate var UNIFFI_CONTINUATION_SLAB = UniffiSlab>() + fileprivate func uniffiRustCallAsync( - rustFutureFunc: () -> UnsafeMutableRawPointer, - pollFunc: (UnsafeMutableRawPointer, @escaping UniFfiRustFutureContinuation, UnsafeMutableRawPointer) -> (), - completeFunc: (UnsafeMutableRawPointer, UnsafeMutablePointer) -> F, - freeFunc: (UnsafeMutableRawPointer) -> (), + rustFutureFunc: () -> Int64, + pollFunc: (Int64, @escaping UniFfiRustFutureContinuation, Int64) -> (), + completeFunc: (Int64, UnsafeMutablePointer) -> F, + freeFunc: (Int64) -> (), liftFunc: (F) throws -> T, errorHandler: ((RustBuffer) throws -> Error)? ) async throws -> T { @@ -19,7 +21,7 @@ fileprivate func uniffiRustCallAsync( var pollResult: Int8; repeat { pollResult = await withUnsafeContinuation { - pollFunc(rustFuture, uniffiFutureContinuationCallback, ContinuationHolder($0).toOpaque()) + pollFunc(rustFuture, uniffiFutureContinuationCallback, try! UNIFFI_CONTINUATION_SLAB.insert(value: $0)) } } while pollResult != UNIFFI_RUST_FUTURE_POLL_READY @@ -31,28 +33,6 @@ fileprivate func uniffiRustCallAsync( // Callback handlers for an async calls. These are invoked by Rust when the future is ready. They // lift the return value or error and resume the suspended function. -fileprivate func uniffiFutureContinuationCallback(ptr: UnsafeMutableRawPointer, pollResult: Int8) { - ContinuationHolder.fromOpaque(ptr).resume(pollResult) -} - -// Wraps UnsafeContinuation in a class so that we can use reference counting when passing it across -// the FFI -fileprivate class ContinuationHolder { - let continuation: UnsafeContinuation - - init(_ continuation: UnsafeContinuation) { - self.continuation = continuation - } - - func resume(_ pollResult: Int8) { - self.continuation.resume(returning: pollResult) - } - - func toOpaque() -> UnsafeMutableRawPointer { - return Unmanaged.passRetained(self).toOpaque() - } - - static func fromOpaque(_ ptr: UnsafeRawPointer) -> ContinuationHolder { - return Unmanaged.fromOpaque(ptr).takeRetainedValue() - } +fileprivate func uniffiFutureContinuationCallback(handle: Int64, pollResult: Int8) { + try! UNIFFI_CONTINUATION_SLAB.remove(handle: handle).resume(returning: pollResult) } diff --git a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h index 87698e359f..337f687d01 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h +++ b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h @@ -29,7 +29,7 @@ typedef struct RustBuffer uint8_t *_Nullable data; } RustBuffer; -typedef int32_t (*ForeignCallback)(uint64_t, int32_t, const uint8_t *_Nonnull, int32_t, RustBuffer *_Nonnull); +typedef int32_t (*ForeignCallback)(int64_t, int32_t, const uint8_t *_Nonnull, int32_t, RustBuffer *_Nonnull); // Task defined in Rust that Swift executes typedef void (*UniFfiRustTaskCallback)(const void * _Nullable, int8_t); @@ -60,7 +60,7 @@ typedef struct RustCallStatus { #endif // def UNIFFI_SHARED_H // Continuation callback for UniFFI Futures -typedef void (*UniFfiRustFutureContinuation)(void * _Nonnull, int8_t); +typedef void (*UniFfiRustFutureContinuation)(int64_t, int8_t); // Scaffolding functions {%- for func in ci.iter_ffi_function_definitions() %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift index 157da46128..c05892ed29 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift @@ -3,7 +3,7 @@ // Declaration and FfiConverters for {{ type_name }} Callback Interface fileprivate let {{ callback_handler }} : ForeignCallback = - { (handle: UniFFICallbackHandle, method: Int32, argsData: UnsafePointer, argsLen: Int32, out_buf: UnsafeMutablePointer) -> Int32 in + { (handle: Int64, method: Int32, argsData: UnsafePointer, argsLen: Int32, out_buf: UnsafeMutablePointer) -> Int32 in {% for meth in methods.iter() -%} {%- let method_name = format!("invoke_{}", meth.name())|fn_name %} @@ -55,18 +55,22 @@ fileprivate let {{ callback_handler }} : ForeignCallback = switch method { case IDX_CALLBACK_FREE: - {{ ffi_converter_name }}.handleMap.remove(handle: handle) + // Call remove and swallow any errors. There's not any way to recover. + let _ = try? {{ ffi_converter_name }}.slab.remove(handle: handle) + // Sucessful return + // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` + return UNIFFI_CALLBACK_SUCCESS + case IDX_CALLBACK_INC_REF: + // Call inc_ref and swallow any errors. There's not any way to recover. + let _ = try? {{ ffi_converter_name }}.slab.incRef(handle: handle) // Sucessful return // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` return UNIFFI_CALLBACK_SUCCESS {% for meth in methods.iter() -%} {% let method_name = format!("invoke_{}", meth.name())|fn_name -%} case {{ loop.index }}: - guard let cb = {{ ffi_converter_name }}.handleMap.get(handle: handle) else { - out_buf.pointee = {{ Type::String.borrow()|lower_fn }}("No callback in handlemap; this is a Uniffi bug") - return UNIFFI_CALLBACK_UNEXPECTED_ERROR - } do { + let cb = try {{ ffi_converter_name }}.slab.get(handle: handle) return try {{ method_name }}(cb, argsData, argsLen, out_buf) } catch let error { out_buf.pointee = {{ Type::String.borrow()|lower_fn }}(String(describing: error)) diff --git a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift index d03b7ccb3f..59595c0ac9 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift @@ -1,63 +1,10 @@ -fileprivate extension NSLock { - func withLock(f: () throws -> T) rethrows -> T { - self.lock() - defer { self.unlock() } - return try f() - } -} +// Magic numbers for the Rust proxy to call using the same mechanism as every other method. -fileprivate typealias UniFFICallbackHandle = UInt64 -fileprivate class UniFFICallbackHandleMap { - private var leftMap: [UniFFICallbackHandle: T] = [:] - private var counter: [UniFFICallbackHandle: UInt64] = [:] - private var rightMap: [ObjectIdentifier: UniFFICallbackHandle] = [:] - - private let lock = NSLock() - private var currentHandle: UniFFICallbackHandle = 1 - private let stride: UniFFICallbackHandle = 1 - - func insert(obj: T) -> UniFFICallbackHandle { - lock.withLock { - let id = ObjectIdentifier(obj as AnyObject) - let handle = rightMap[id] ?? { - currentHandle += stride - let handle = currentHandle - leftMap[handle] = obj - rightMap[id] = handle - return handle - }() - counter[handle] = (counter[handle] ?? 0) + 1 - return handle - } - } - - func get(handle: UniFFICallbackHandle) -> T? { - lock.withLock { - leftMap[handle] - } - } - - func delete(handle: UniFFICallbackHandle) { - remove(handle: handle) - } - - @discardableResult - func remove(handle: UniFFICallbackHandle) -> T? { - lock.withLock { - defer { counter[handle] = (counter[handle] ?? 1) - 1 } - guard counter[handle] == 1 else { return leftMap[handle] } - let obj = leftMap.removeValue(forKey: handle) - if let obj = obj { - rightMap.removeValue(forKey: ObjectIdentifier(obj as AnyObject)) - } - return obj - } - } -} - -// Magic number for the Rust proxy to call using the same mechanism as every other method, -// to free the callback once it's dropped by Rust. +// Dec-ref the callback object private let IDX_CALLBACK_FREE: Int32 = 0 +// Inc-ref the callback object +private let IDX_CALLBACK_INC_REF: Int32 = 0x7FFF_FFFF; + // Callback return codes private let UNIFFI_CALLBACK_SUCCESS: Int32 = 0 private let UNIFFI_CALLBACK_ERROR: Int32 = 1 diff --git a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift index f9e75b2a3f..51ed79f490 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift @@ -10,28 +10,24 @@ // FfiConverter protocol for callback interfaces fileprivate struct {{ ffi_converter_name }} { - fileprivate static var handleMap = UniFFICallbackHandleMap<{{ type_name }}>() + fileprivate static var slab = UniffiSlab<{{ type_name }}>() } extension {{ ffi_converter_name }} : FfiConverter { typealias SwiftType = {{ type_name }} - // We can use Handle as the FfiType because it's a typealias to UInt64 - typealias FfiType = UniFFICallbackHandle + typealias FfiType = Int64 - public static func lift(_ handle: UniFFICallbackHandle) throws -> SwiftType { - guard let callback = handleMap.get(handle: handle) else { - throw UniffiInternalError.unexpectedStaleHandle - } - return callback + public static func lift(_ handle: Int64) throws -> SwiftType { + return try! slab.get(handle: handle) } public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> SwiftType { - let handle: UniFFICallbackHandle = try readInt(&buf) + let handle: Int64 = try readInt(&buf) return try lift(handle) } - public static func lower(_ v: SwiftType) -> UniFFICallbackHandle { - return handleMap.insert(obj: v) + public static func lower(_ v: SwiftType) -> Int64 { + return try! slab.insert(value: v) } public static func write(_ v: SwiftType, into buf: inout [UInt8]) { diff --git a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift index a34b128e23..770a0c9a52 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift @@ -9,6 +9,7 @@ fileprivate enum UniffiInternalError: LocalizedError { case unexpectedRustCallStatusCode case unexpectedRustCallError case unexpectedStaleHandle + case slabError case rustPanic(_ message: String) public var errorDescription: String? { @@ -21,6 +22,7 @@ fileprivate enum UniffiInternalError: LocalizedError { case .unexpectedRustCallStatusCode: return "Unexpected RustCallStatus code" case .unexpectedRustCallError: return "CALL_ERROR but no errorClass specified" case .unexpectedStaleHandle: return "The object in the handle map has been dropped already" + case .slabError: return "Slab error" case let .rustPanic(message): return message } } @@ -99,3 +101,12 @@ private func uniffiCheckCallStatus( throw UniffiInternalError.unexpectedRustCallStatusCode } } + +fileprivate extension NSLock { + func withLock(f: () throws -> T) rethrows -> T { + self.lock() + defer { self.unlock() } + return try f() + } +} + diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index b63631f40e..54fa96faea 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -5,31 +5,36 @@ {% include "Protocol.swift" %} public class {{ impl_class_name }}: {{ protocol_name }} { - fileprivate let pointer: UnsafeMutableRawPointer + fileprivate let handle: Int64 // TODO: We'd like this to be `private` but for Swifty reasons, // we can't implement `FfiConverter` without making this `required` and we can't // make it `required` without making it `public`. - required init(unsafeFromRawPointer pointer: UnsafeMutableRawPointer) { - self.pointer = pointer + required init(handle: Int64) { + self.handle = handle } {%- match obj.primary_constructor() %} {%- when Some with (cons) %} public convenience init({% call swift::arg_list_decl(cons) -%}) {% call swift::throws(cons) %} { - self.init(unsafeFromRawPointer: {% call swift::to_ffi_call(cons) %}) + self.init(handle: {% call swift::to_ffi_call(cons) %}) } {%- when None %} {%- endmatch %} deinit { - try! rustCall { {{ obj.ffi_object_free().name() }}(pointer, $0) } + try! rustCall { {{ obj.ffi_object_free().name() }}(handle, $0) } + } + + internal func uniffiCloneHandle() -> Int64 { + try! rustCall { {{ obj.ffi_object_inc_ref().name() }}(handle, $0) } + return handle } {% for cons in obj.alternate_constructors() %} public static func {{ cons.name()|fn_name }}({% call swift::arg_list_decl(cons) %}) {% call swift::throws(cons) %} -> {{ impl_class_name }} { - return {{ impl_class_name }}(unsafeFromRawPointer: {% call swift::to_ffi_call(cons) %}) + return {{ impl_class_name }}(handle: {% call swift::to_ffi_call(cons) %}) } {% endfor %} @@ -42,7 +47,7 @@ public class {{ impl_class_name }}: {{ protocol_name }} { return {% call swift::try(meth) %} await uniffiRustCallAsync( rustFutureFunc: { {{ meth.ffi_func().name() }}( - self.pointer + self.uniffiCloneHandle() {%- for arg in meth.arguments() -%} , {{ arg|lower_fn }}({{ arg.name()|var_name }}) @@ -75,14 +80,14 @@ public class {{ impl_class_name }}: {{ protocol_name }} { public func {{ meth.name()|fn_name }}({% call swift::arg_list_decl(meth) %}) {% call swift::throws(meth) %} -> {{ return_type|type_name }} { return {% call swift::try(meth) %} {{ return_type|lift_fn }}( - {% call swift::to_ffi_call_with_prefix("self.pointer", meth) %} + {% call swift::to_ffi_call_with_prefix("self.uniffiCloneHandle()", meth) %} ) } {%- when None %} public func {{ meth.name()|fn_name }}({% call swift::arg_list_decl(meth) %}) {% call swift::throws(meth) %} { - {% call swift::to_ffi_call_with_prefix("self.pointer", meth) %} + {% call swift::to_ffi_call_with_prefix("self.uniffiCloneHandle()", meth) %} } {%- endmatch -%} @@ -99,43 +104,45 @@ public class {{ impl_class_name }}: {{ protocol_name }} { public struct {{ ffi_converter_name }}: FfiConverter { {%- if obj.is_trait_interface() %} - fileprivate static var handleMap = UniFFICallbackHandleMap<{{ type_name }}>() + fileprivate static var slab = UniffiSlab<{{ type_name }}>() {%- endif %} - typealias FfiType = UnsafeMutableRawPointer + typealias FfiType = Int64 typealias SwiftType = {{ type_name }} - public static func lift(_ pointer: UnsafeMutableRawPointer) throws -> {{ type_name }} { - return {{ impl_class_name }}(unsafeFromRawPointer: pointer) + public static func lift(_ handle: Int64) throws -> {{ type_name }} { + {%- if obj.is_trait_interface() %} + if uniffiHandleIsFromRust(handle) { + return {{ impl_class_name }}(handle: handle) + } else { + return try! slab.remove(handle: handle) + } + {%- else %} + return {{ impl_class_name }}(handle: handle) + {%- endif %} } - public static func lower(_ value: {{ type_name }}) -> UnsafeMutableRawPointer { - {%- match obj.imp() %} - {%- when ObjectImpl::Struct %} - return value.pointer - {%- when ObjectImpl::Trait %} - guard let ptr = UnsafeMutableRawPointer(bitPattern: UInt(truncatingIfNeeded: handleMap.insert(obj: value))) else { - fatalError("Cast to UnsafeMutableRawPointer failed") + public static func lower(_ value: {{ type_name }}) -> Int64 { + {%- if obj.is_trait_interface() %} + if let rustImpl = value as? {{ impl_class_name }} { + // If we're wrapping a trait implemented in Rust, return that handle directly rather + // than wrapping it again in Swift. + return rustImpl.uniffiCloneHandle() + } else { + return try! slab.insert(value: value) } - return ptr - {%- endmatch %} + {%- else %} + // inc-ref the current handle, then return the new reference. + return value.uniffiCloneHandle() + {%- endif %} } public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> {{ type_name }} { - let v: UInt64 = try readInt(&buf) - // The Rust code won't compile if a pointer won't fit in a UInt64. - // We have to go via `UInt` because that's the thing that's the size of a pointer. - let ptr = UnsafeMutableRawPointer(bitPattern: UInt(truncatingIfNeeded: v)) - if (ptr == nil) { - throw UniffiInternalError.unexpectedNullPointer - } - return try lift(ptr!) + return try lift(try readInt(&buf)) } public static func write(_ value: {{ type_name }}, into buf: inout [UInt8]) { - // This fiddling is because `Int` is the thing that's the same size as a pointer. - // The Rust code won't compile if a pointer won't fit in a `UInt64`. - writeInt(&buf, UInt64(bitPattern: Int64(Int(bitPattern: lower(value))))) + writeInt(&buf, lower(value)) } } @@ -143,10 +150,10 @@ public struct {{ ffi_converter_name }}: FfiConverter { We always write these public functions just in case the enum is used as an external type by another crate. #} -public func {{ ffi_converter_name }}_lift(_ pointer: UnsafeMutableRawPointer) throws -> {{ type_name }} { - return try {{ ffi_converter_name }}.lift(pointer) +public func {{ ffi_converter_name }}_lift(_ handle: Int64) throws -> {{ type_name }} { + return try {{ ffi_converter_name }}.lift(handle) } -public func {{ ffi_converter_name }}_lower(_ value: {{ type_name }}) -> UnsafeMutableRawPointer { +public func {{ ffi_converter_name }}_lower(_ value: {{ type_name }}) -> Int64 { return {{ ffi_converter_name }}.lower(value) } diff --git a/uniffi_bindgen/src/bindings/swift/templates/Slab.swift b/uniffi_bindgen/src/bindings/swift/templates/Slab.swift new file mode 100644 index 0000000000..5d1ac30601 --- /dev/null +++ b/uniffi_bindgen/src/bindings/swift/templates/Slab.swift @@ -0,0 +1,65 @@ +fileprivate func uniffiHandleIsFromRust(_ handle: Int64) -> Bool { + return (handle & 0x0001_0000_0000) == 0 +} + +fileprivate class UniffiSlab { + private let slabHandle = {{ ci.ffi_slab_new().name() }}() + // TODO: use a read-write lock. + // + // Swift articles suggest using the `pthreads` version. Can we just switch to that? + // If not, maybe we could define some Rust FFI functions to handle it, although it's hard with + // the `std::sync::RwLock` since that uses a guard API + private let lock = NSLock() + var items = Array() + + private func index(_ handle: Int64) -> Int { + return Int(handle & 0xFFFF) + } + + internal func insert(value: T) throws -> Int64 { + let handle = {{ ci.ffi_slab_insert().name() }}(slabHandle) + if (handle < 0) { + throw UniffiInternalError.slabError + } + let index = index(handle) + return lock.withLock { + while (items.count <= index) { + items.append(nil) + } + items[index] = value + return handle + } + } + + internal func get(handle: Int64) throws -> T { + let result = {{ ci.ffi_slab_check_handle().name() }}(slabHandle, handle) + if (result < 0) { + throw UniffiInternalError.slabError + } + return lock.withLock { + return items[index(handle)]! + } + } + + internal func incRef(handle: Int64) throws { + let result = {{ ci.ffi_slab_inc_ref().name() }}(slabHandle, handle) + if (result < 0) { + throw UniffiInternalError.slabError + } + } + + internal func remove(handle: Int64) throws -> T { + let result = {{ ci.ffi_slab_dec_ref().name() }}(slabHandle, handle) + if (result < 0) { + throw UniffiInternalError.slabError + } + let index = index(handle) + return lock.withLock { + let value = items[index]! + if (result == 1) { + items[index] = nil + } + return value + } + } +} diff --git a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift index c34d348efb..61138aaae2 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift @@ -14,6 +14,7 @@ import {{ config.ffi_module_name() }} #endif {% include "RustBufferTemplate.swift" %} +{% include "Slab.swift" %} {% include "Helpers.swift" %} // Public interface members begin here. diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index 8f23ccbb90..66cb23ed80 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -33,11 +33,8 @@ pub enum FfiType { Int64, Float32, Float64, - /// 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. - /// The inner string references the name of the `T` type. - RustArcPtr(String), + /// 64-bit handle. See `slab.rs` for details. + Handle, /// 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. @@ -49,16 +46,10 @@ pub enum FfiType { ForeignBytes, /// Pointer to a callback function that handles all callbacks on the foreign language side. ForeignCallback, - /// Pointer-sized opaque handle that represents a foreign executor. Foreign bindings can - /// either use an actual pointer or a usized integer. - ForeignExecutorHandle, /// Pointer to the callback function that's invoked to schedule calls with a ForeignExecutor ForeignExecutorCallback, - /// Pointer to a Rust future - RustFutureHandle, /// Continuation function for a Rust future RustFutureContinuationCallback, - RustFutureContinuationData, // TODO: you can imagine a richer structural typesystem here, e.g. `Ref` or something. // We don't need that yet and it's possible we never will, so it isn't here for now. } @@ -90,11 +81,14 @@ impl From<&Type> for FfiType { // Byte strings are also always owned rust values. // We might add a separate type for borrowed byte strings in future as well. Type::Bytes => FfiType::RustBuffer(None), - // Objects are pointers to an Arc<> - Type::Object { name, .. } => FfiType::RustArcPtr(name.to_owned()), - // Callback interfaces are passed as opaque integer handles. - Type::CallbackInterface { .. } => FfiType::UInt64, - Type::ForeignExecutor => FfiType::ForeignExecutorHandle, + // Object types interfaces are passed as opaque integer handles. + Type::Object { .. } + | Type::CallbackInterface { .. } + | Type::ForeignExecutor + | Type::External { + kind: ExternalKind::Interface, + .. + } => FfiType::Handle, // Other types are serialized into a bytebuffer and deserialized on the other side. Type::Enum { .. } | Type::Record { .. } @@ -103,11 +97,8 @@ impl From<&Type> for FfiType { | Type::Map { .. } | Type::Timestamp | Type::Duration => FfiType::RustBuffer(None), - Type::External { - name, - kind: ExternalKind::Interface, - .. - } => FfiType::RustArcPtr(name.clone()), + // External data classes are also serialized as a buffer, but need a module name to + // make imports work. Type::External { name, kind: ExternalKind::DataClass, @@ -150,6 +141,24 @@ pub struct FfiFunction { } impl FfiFunction { + pub fn ffi_inc_ref(name: String) -> Self { + Self { + name, + arguments: vec![FfiArgument { + name: "handle".to_owned(), + type_: FfiType::Handle, + }], + ..Default::default() + } + } + + pub fn ffi_free(name: String) -> Self { + Self { + name, + ..Default::default() + } + } + pub fn callback_init(module_path: &str, trait_name: &str) -> Self { Self { name: uniffi_meta::init_callback_fn_symbol_name(module_path, trait_name), @@ -194,7 +203,7 @@ impl FfiFunction { ) { self.arguments = args.into_iter().collect(); if self.is_async() { - self.return_type = Some(FfiType::RustFutureHandle); + self.return_type = Some(FfiType::Handle); self.has_rust_call_status_arg = false; } else { self.return_type = return_type; diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index c56a3cd65c..b9773184ba 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -435,6 +435,105 @@ impl ComponentInterface { } } + pub fn ffi_slab_new(&self) -> FfiFunction { + FfiFunction { + name: format!("ffi_{}_slab_new", self.ffi_namespace()), + is_async: false, + arguments: vec![], + return_type: Some(FfiType::Handle), + has_rust_call_status_arg: false, + is_object_free_function: false, + } + } + + pub fn ffi_slab_free(&self) -> FfiFunction { + FfiFunction { + name: format!("ffi_{}_slab_free", self.ffi_namespace()), + is_async: false, + arguments: vec![FfiArgument { + name: "slab".to_owned(), + type_: FfiType::Handle, + }], + return_type: None, + has_rust_call_status_arg: false, + is_object_free_function: false, + } + } + + pub fn ffi_slab_insert(&self) -> FfiFunction { + FfiFunction { + name: format!("ffi_{}_slab_insert", self.ffi_namespace()), + is_async: false, + arguments: vec![FfiArgument { + name: "slab".to_owned(), + type_: FfiType::Handle, + }], + return_type: Some(FfiType::Int64), + has_rust_call_status_arg: false, + is_object_free_function: false, + } + } + + pub fn ffi_slab_check_handle(&self) -> FfiFunction { + FfiFunction { + name: format!("ffi_{}_slab_check_handle", self.ffi_namespace()), + is_async: false, + arguments: vec![ + FfiArgument { + name: "slab".to_owned(), + type_: FfiType::Handle, + }, + FfiArgument { + name: "handle".to_owned(), + type_: FfiType::Handle, + }, + ], + return_type: Some(FfiType::Int8), + has_rust_call_status_arg: false, + is_object_free_function: false, + } + } + + pub fn ffi_slab_inc_ref(&self) -> FfiFunction { + FfiFunction { + name: format!("ffi_{}_slab_inc_ref", self.ffi_namespace()), + is_async: false, + arguments: vec![ + FfiArgument { + name: "slab".to_owned(), + type_: FfiType::Handle, + }, + FfiArgument { + name: "handle".to_owned(), + type_: FfiType::Handle, + }, + ], + return_type: Some(FfiType::Int8), + has_rust_call_status_arg: false, + is_object_free_function: false, + } + } + + pub fn ffi_slab_dec_ref(&self) -> FfiFunction { + FfiFunction { + name: format!("ffi_{}_slab_dec_ref", self.ffi_namespace()), + is_async: false, + arguments: vec![ + FfiArgument { + name: "slab".to_owned(), + type_: FfiType::Handle, + }, + FfiArgument { + name: "handle".to_owned(), + type_: FfiType::Handle, + }, + ], + return_type: Some(FfiType::Int8), + has_rust_call_status_arg: false, + is_object_free_function: false, + } + } + /// Builtin FFI function to poll a Rust future. pub fn ffi_rust_future_poll(&self, return_ffi_type: Option) -> FfiFunction { FfiFunction { @@ -443,15 +542,15 @@ impl ComponentInterface { arguments: vec![ FfiArgument { name: "handle".to_owned(), - type_: FfiType::RustFutureHandle, + type_: FfiType::Handle, }, FfiArgument { name: "callback".to_owned(), type_: FfiType::RustFutureContinuationCallback, }, FfiArgument { - name: "callback_data".to_owned(), - type_: FfiType::RustFutureContinuationData, + name: "continuation_data".to_owned(), + type_: FfiType::Handle, }, ], return_type: None, @@ -469,7 +568,7 @@ impl ComponentInterface { is_async: false, arguments: vec![FfiArgument { name: "handle".to_owned(), - type_: FfiType::RustFutureHandle, + type_: FfiType::Handle, }], return_type: return_ffi_type, has_rust_call_status_arg: true, @@ -484,7 +583,7 @@ impl ComponentInterface { is_async: false, arguments: vec![FfiArgument { name: "handle".to_owned(), - type_: FfiType::RustFutureHandle, + type_: FfiType::Handle, }], return_type: None, has_rust_call_status_arg: false, @@ -499,7 +598,7 @@ impl ComponentInterface { is_async: false, arguments: vec![FfiArgument { name: "handle".to_owned(), - type_: FfiType::RustFutureHandle, + type_: FfiType::Handle, }], return_type: None, has_rust_call_status_arg: false, @@ -521,7 +620,7 @@ impl ComponentInterface { FfiType::Int64 => format!("ffi_{namespace}_{base_name}_i64"), FfiType::Float32 => format!("ffi_{namespace}_{base_name}_f32"), FfiType::Float64 => format!("ffi_{namespace}_{base_name}_f64"), - FfiType::RustArcPtr(_) => format!("ffi_{namespace}_{base_name}_pointer"), + FfiType::Handle => format!("ffi_{namespace}_{base_name}_handle"), FfiType::RustBuffer(_) => format!("ffi_{namespace}_{base_name}_rust_buffer"), _ => unimplemented!("FFI return type: {t:?}"), }, @@ -561,6 +660,7 @@ impl ComponentInterface { .cloned() .chain(self.iter_rust_buffer_ffi_function_definitions()) .chain(self.iter_futures_ffi_function_definitons()) + .chain(self.iter_slab_ffi_function_definitions()) .chain(self.iter_checksum_ffi_functions()) .chain(self.ffi_foreign_executor_callback_set()) .chain([self.ffi_uniffi_contract_version()]) @@ -573,6 +673,7 @@ impl ComponentInterface { self.iter_user_ffi_function_definitions() .cloned() .chain(self.iter_rust_buffer_ffi_function_definitions()) + .chain(self.iter_slab_ffi_function_definitions()) .chain(self.iter_checksum_ffi_functions()) .chain([self.ffi_uniffi_contract_version()]) } @@ -609,6 +710,19 @@ impl ComponentInterface { .into_iter() } + /// List all FFI functions definitions for Slab functionality. + pub fn iter_slab_ffi_function_definitions(&self) -> impl Iterator { + [ + self.ffi_slab_new(), + self.ffi_slab_free(), + self.ffi_slab_insert(), + self.ffi_slab_check_handle(), + self.ffi_slab_inc_ref(), + self.ffi_slab_dec_ref(), + ] + .into_iter() + } + /// List all FFI functions definitions for async functionality. pub fn iter_futures_ffi_function_definitons(&self) -> impl Iterator + '_ { let all_possible_return_ffi_types = [ @@ -622,9 +736,9 @@ impl ComponentInterface { Some(FfiType::Int64), Some(FfiType::Float32), Some(FfiType::Float64), - // RustBuffer and RustArcPtr have an inner field which doesn't affect the rust future + Some(FfiType::Handle), + // RustBuffer has an inner field which doesn't affect the rust future // complete scaffolding function, so we just use a placeholder value here. - Some(FfiType::RustArcPtr("".to_owned())), Some(FfiType::RustBuffer(None)), None, ]; diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index d79e7fccb1..37d66a1149 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -57,8 +57,6 @@ //! # Ok::<(), anyhow::Error>(()) //! ``` -use std::iter; - use anyhow::Result; use uniffi_meta::Checksum; @@ -100,6 +98,9 @@ pub struct Object { // avoids a weird circular dependency in the calculation. #[checksum_ignore] pub(super) ffi_func_free: FfiFunction, + // Increment the reference count for this object. Call this before lowering a handle. + #[checksum_ignore] + pub(super) ffi_func_inc_ref: FfiFunction, // Ffi function to initialize the foreign callback for trait interfaces #[checksum_ignore] pub(super) ffi_init_callback: Option, @@ -162,6 +163,10 @@ impl Object { &self.ffi_func_free } + pub fn ffi_object_inc_ref(&self) -> &FfiFunction { + &self.ffi_func_inc_ref + } + pub fn ffi_init_callback(&self) -> &FfiFunction { self.ffi_init_callback .as_ref() @@ -169,7 +174,8 @@ impl Object { } pub fn iter_ffi_function_definitions(&self) -> impl Iterator { - iter::once(&self.ffi_func_free) + [&self.ffi_func_free, &self.ffi_func_inc_ref] + .into_iter() .chain(&self.ffi_init_callback) .chain(self.constructors.iter().map(|f| &f.ffi_func)) .chain(self.methods.iter().map(|f| &f.ffi_func)) @@ -189,8 +195,8 @@ impl Object { pub fn derive_ffi_funcs(&mut self) -> Result<()> { assert!(!self.ffi_func_free.name().is_empty()); self.ffi_func_free.arguments = vec![FfiArgument { - name: "ptr".to_string(), - type_: FfiType::RustArcPtr(self.name.to_string()), + name: "handle".to_string(), + type_: FfiType::Handle, }]; self.ffi_func_free.return_type = None; self.ffi_func_free.is_object_free_function = true; @@ -236,7 +242,8 @@ impl AsType for Object { impl From for Object { fn from(meta: uniffi_meta::ObjectMetadata) -> Self { - let ffi_free_name = meta.free_ffi_symbol_name(); + let ffi_func_inc_ref = FfiFunction::ffi_inc_ref(meta.inc_ref_ffi_symbol_name()); + let ffi_func_free = FfiFunction::ffi_free(meta.free_ffi_symbol_name()); Object { module_path: meta.module_path, name: meta.name, @@ -244,10 +251,8 @@ impl From for Object { constructors: Default::default(), methods: Default::default(), uniffi_traits: Default::default(), - ffi_func_free: FfiFunction { - name: ffi_free_name, - ..Default::default() - }, + ffi_func_inc_ref, + ffi_func_free, ffi_init_callback: None, } } @@ -342,7 +347,7 @@ impl Constructor { fn derive_ffi_func(&mut self) { assert!(!self.ffi_func.name().is_empty()); self.ffi_func.arguments = self.arguments.iter().map(Into::into).collect(); - self.ffi_func.return_type = Some(FfiType::RustArcPtr(self.object_name.clone())); + self.ffi_func.return_type = Some(FfiType::Handle); } pub fn iter_types(&self) -> TypeIterator<'_> { @@ -419,7 +424,7 @@ impl Method { // hence `arguments` and `full_arguments` are different. pub fn full_arguments(&self) -> Vec { vec![Argument { - name: "ptr".to_string(), + name: "handle".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 { diff --git a/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs b/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs index 64c69e4d8e..12aa55c795 100644 --- a/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs +++ b/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs @@ -29,11 +29,11 @@ pub extern "C" fn {{ cbi.ffi_init_callback().name() }}(callback: uniffi::Foreign #[doc(hidden)] #[derive(Debug)] struct {{ trait_impl }} { - handle: u64 + handle: ::uniffi::Handle } impl {{ trait_impl }} { - fn new(handle: u64) -> Self { + fn new(handle: ::uniffi::Handle) -> Self { Self { handle } } } diff --git a/uniffi_core/Cargo.toml b/uniffi_core/Cargo.toml index fa50e0c295..04756c03ce 100644 --- a/uniffi_core/Cargo.toml +++ b/uniffi_core/Cargo.toml @@ -16,14 +16,27 @@ anyhow = "1" async-compat = { version = "0.2.1", optional = true } bytes = "1.3" camino = "1.0.8" +const-random = "0.1.15" log = "0.4" once_cell = "1.10.0" # Enable "async" so that receivers implement Future, no need for "std" since we don't block on them. oneshot = { version = "0.1", features = ["async"] } # Regular dependencies +append-only-vec = "0.1" paste = "1.0" static_assertions = "1.1.0" +[dev-dependencies] +rand = "0.8" + +# We want to test the slab code with loom, but don't want to introduce it as a direct dependency +# because that would cause issue with the mozilla-central Rust vendoring. So, uncomment the `loom` +# dopendency before running the tests, then run: +# +# cargo test -p uniffi_core --release --config build.rustflags='"--cfg loom"' slab_loom_test +# +# loom = "0.7.1" + [features] default = [] # `no_mangle` RustBuffer FFI functions diff --git a/uniffi_core/src/ffi/callbackinterface.rs b/uniffi_core/src/ffi/callbackinterface.rs index 7be66880bb..f2dab27a96 100644 --- a/uniffi_core/src/ffi/callbackinterface.rs +++ b/uniffi_core/src/ffi/callbackinterface.rs @@ -113,12 +113,14 @@ //! type and then returns to client code. //! -use crate::{ForeignCallback, ForeignCallbackCell, Lift, LiftReturn, RustBuffer}; +use crate::{ForeignCallback, ForeignCallbackCell, Handle, Lift, LiftReturn, RustBuffer}; use std::fmt; -/// The method index used by the Drop trait to communicate to the foreign language side that Rust has finished with it, -/// and it can be deleted from the handle map. +/// Decrement the reference count for the object on the foreign side. pub const IDX_CALLBACK_FREE: u32 = 0; +/// Increment the reference count for the object on the foreign side. +/// This is i32::MAX because Kotlin/Swift currently uses signed integers for the method index. +pub const IDX_CALLBACK_INC_REF: u32 = i32::MAX as u32; /// Result of a foreign callback invocation #[repr(i32)] @@ -167,7 +169,7 @@ impl ForeignCallbackInternals { } /// Invoke a callback interface method on the foreign side and return the result - pub fn invoke_callback(&self, handle: u64, method: u32, args: RustBuffer) -> R + pub fn invoke_callback(&self, handle: Handle, method: u32, args: RustBuffer) -> R where R: LiftReturn, { diff --git a/uniffi_core/src/ffi/ffidefault.rs b/uniffi_core/src/ffi/ffidefault.rs index 1f86f6b13b..28495e642f 100644 --- a/uniffi_core/src/ffi/ffidefault.rs +++ b/uniffi_core/src/ffi/ffidefault.rs @@ -39,9 +39,9 @@ impl FfiDefault for () { fn ffi_default() {} } -impl FfiDefault for *const std::ffi::c_void { +impl FfiDefault for crate::Handle { fn ffi_default() -> Self { - std::ptr::null() + Self::from_raw(0) } } @@ -51,12 +51,6 @@ impl FfiDefault for crate::RustBuffer { } } -impl FfiDefault for crate::ForeignExecutorHandle { - fn ffi_default() -> Self { - Self(std::ptr::null()) - } -} - impl FfiDefault for Option { fn ffi_default() -> Self { None diff --git a/uniffi_core/src/ffi/foreigncallbacks.rs b/uniffi_core/src/ffi/foreigncallbacks.rs index ac2463cd8e..1746489e30 100644 --- a/uniffi_core/src/ffi/foreigncallbacks.rs +++ b/uniffi_core/src/ffi/foreigncallbacks.rs @@ -10,7 +10,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; -use crate::{ForeignExecutorHandle, RustBuffer, RustTaskCallback}; +use crate::{Handle, RustBuffer, RustTaskCallback}; /// ForeignCallback is the Rust representation of a foreign language function. /// It is the basis for all callbacks interfaces. It is registered exactly once per callback interface, @@ -30,7 +30,7 @@ use crate::{ForeignExecutorHandle, RustBuffer, RustTaskCallback}; /// * Callbacks return one of the `CallbackResult` values /// Note: The output buffer might still contain 0 bytes of data. pub type ForeignCallback = unsafe extern "C" fn( - handle: u64, + handle: Handle, method: u32, args_data: *const u8, args_len: i32, @@ -50,7 +50,7 @@ pub type ForeignCallback = unsafe extern "C" fn( /// /// The callback should return one of the `ForeignExecutorCallbackResult` values. pub type ForeignExecutorCallback = extern "C" fn( - executor: ForeignExecutorHandle, + executor: Handle, delay: u32, task: Option, task_data: *const (), diff --git a/uniffi_core/src/ffi/foreignexecutor.rs b/uniffi_core/src/ffi/foreignexecutor.rs index 7b1cb9bd80..a095f69c30 100644 --- a/uniffi_core/src/ffi/foreignexecutor.rs +++ b/uniffi_core/src/ffi/foreignexecutor.rs @@ -6,20 +6,7 @@ use std::panic; -use crate::{ForeignExecutorCallback, ForeignExecutorCallbackCell}; - -/// Opaque handle for a foreign task executor. -/// -/// Foreign code can either use an actual pointer, or use an integer value casted to it. -#[repr(transparent)] -#[derive(Clone, Copy, Debug)] -pub struct ForeignExecutorHandle(pub(crate) *const ()); - -// Implement Send + Sync for `ForeignExecutor`. The foreign bindings code is responsible for -// making the `ForeignExecutorCallback` thread-safe. -unsafe impl Send for ForeignExecutorHandle {} - -unsafe impl Sync for ForeignExecutorHandle {} +use crate::{ForeignExecutorCallback, ForeignExecutorCallbackCell, Handle}; /// Result code returned by `ForeignExecutorCallback` #[repr(i8)] @@ -95,11 +82,11 @@ pub fn foreign_executor_callback_set(callback: ForeignExecutorCallback) { /// Schedule Rust calls using a foreign executor #[derive(Debug)] pub struct ForeignExecutor { - pub(crate) handle: ForeignExecutorHandle, + pub(crate) handle: Handle, } impl ForeignExecutor { - pub fn new(executor: ForeignExecutorHandle) -> Self { + pub fn new(executor: Handle) -> Self { Self { handle: executor } } @@ -162,11 +149,11 @@ impl ForeignExecutor { /// Low-level schedule interface /// /// When using this function, take care to ensure that the `ForeignExecutor` that holds the -/// `ForeignExecutorHandle` has not been dropped. +/// `Handle` has not been dropped. /// /// Returns true if the callback was successfully scheduled pub(crate) fn schedule_raw( - handle: ForeignExecutorHandle, + handle: Handle, delay: u32, callback: RustTaskCallback, data: *const (), @@ -249,6 +236,8 @@ mod test { unsafe impl Send for MockEventLoopInner {} static FOREIGN_EXECUTOR_CALLBACK_INIT: Once = Once::new(); + static EVENT_LOOP_SLAB: crate::Slab> = + crate::Slab::new_with_id_and_foreign(0, true); impl MockEventLoop { pub fn new() -> Arc { @@ -264,11 +253,9 @@ mod test { }) } - /// Create a new ForeignExecutorHandle - pub fn new_handle(self: &Arc) -> ForeignExecutorHandle { - // To keep the memory management simple, we simply leak an arc reference for this. We - // only create a handful of these in the tests so there's no need for proper cleanup. - ForeignExecutorHandle(Arc::into_raw(Arc::clone(self)) as *const ()) + /// Create a new Handle + pub fn new_handle(self: &Arc) -> Handle { + EVENT_LOOP_SLAB.insert_or_panic(Arc::clone(self)) } pub fn new_executor(self: &Arc) -> ForeignExecutor { @@ -314,13 +301,13 @@ mod test { // `ForeignExecutorCallback` that we install for testing extern "C" fn mock_executor_callback( - handle: ForeignExecutorHandle, + handle: Handle, delay: u32, task: Option, task_data: *const (), ) -> i8 { - let eventloop = handle.0 as *const MockEventLoop; - let mut inner = unsafe { (*eventloop).inner.lock().unwrap() }; + let eventloop = EVENT_LOOP_SLAB.get_clone_or_panic(handle); + let mut inner = eventloop.inner.lock().unwrap(); if inner.is_shutdown { ForeignExecutorCallbackResult::Cancelled as i8 } else { diff --git a/uniffi_core/src/ffi/foreignslab.rs b/uniffi_core/src/ffi/foreignslab.rs new file mode 100644 index 0000000000..1c2e6d9c32 --- /dev/null +++ b/uniffi_core/src/ffi/foreignslab.rs @@ -0,0 +1,99 @@ +/* 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/. */ + +//! Slab functionality for the foreign bindings +//! +//! This module creates Slab instances that the foreign bindings can use. +//! This slabs don't store anything, they just manage the handles. +//! The foreign code creates the actual object array and stores objects using the handle's index. +//! For most languages, the array is protected with a read-write lock. +//! Overall, this is a slightly less efficient system, but it's simple for bindings to use. + +use std::sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, +}; + +use crate::{derive_ffi_traits, Handle, Slab, SlabAlloc}; + +type ForeignSlab = Slab<()>; + +static SLAB_ID_COUNTER: AtomicUsize = AtomicUsize::new(0); + +// How should we allocate and manage [ForeignSlab] instances? Another Slab. +derive_ffi_traits!(impl SlabAlloc for ForeignSlab); + +// ==================== Public API ==================== +// +// All of these functions are exported as a extern "C" function in the scaffolding code + +/// Create a new slab and get a handle for the slab +pub fn uniffi_slab_new() -> Handle { + let slab_id = (SLAB_ID_COUNTER.fetch_add(1, Ordering::Relaxed) & 0xFF) as u8; + >::insert(Arc::new( + ForeignSlab::new_with_id_and_foreign(slab_id, true), + )) +} + +/// Free a slab +pub fn uniffi_slab_free(slab: Handle) { + >::remove(slab); +} + +/// Insert a new entry and get a handle for it +/// +/// Returns the handle on success, -1 on error +pub fn uniffi_slab_insert(slab: Handle) -> i64 { + let slab = >::get_clone(slab); + match slab.insert(()) { + Ok(handle) => handle.as_raw(), + Err(e) => { + println!("{e}"); + -1 + } + } +} + +/// Check that a handle is still valid +/// +/// Returns 0 on success, -1 on error +pub fn uniffi_slab_check_handle(slab: Handle, handle: Handle) -> i8 { + let slab = >::get_clone(slab); + match slab.get_clone(handle) { + Ok(_) => 0, + Err(e) => { + println!("{e}"); + -1 + } + } +} + +/// Increment the handle reference count +/// +/// Returns 0 on success, -1 on error +pub fn uniffi_slab_inc_ref(slab: Handle, handle: Handle) -> i8 { + let slab = >::get_clone(slab); + match slab.inc_ref(handle) { + Ok(_) => 0, + Err(e) => { + println!("{e}"); + -1 + } + } +} + +/// Decrement the handle reference count +/// +/// Returns 1 if the handle should be freed, 0 if not, and -1 on error. +pub fn uniffi_slab_dec_ref(slab: Handle, handle: Handle) -> i8 { + let slab = >::get_clone(slab); + match slab.remove(handle) { + Ok((_, true)) => 1, + Ok((_, false)) => 0, + Err(e) => { + println!("{e}"); + -1 + } + } +} diff --git a/uniffi_core/src/ffi/mod.rs b/uniffi_core/src/ffi/mod.rs index b606323297..5d33184514 100644 --- a/uniffi_core/src/ffi/mod.rs +++ b/uniffi_core/src/ffi/mod.rs @@ -9,15 +9,19 @@ pub mod ffidefault; pub mod foreignbytes; pub mod foreigncallbacks; pub mod foreignexecutor; +pub mod foreignslab; pub mod rustbuffer; pub mod rustcalls; pub mod rustfuture; +pub mod slab; pub use callbackinterface::*; pub use ffidefault::FfiDefault; pub use foreignbytes::*; pub use foreigncallbacks::*; pub use foreignexecutor::*; +pub use foreignslab::*; pub use rustbuffer::*; pub use rustcalls::*; pub use rustfuture::*; +pub use slab::*; diff --git a/uniffi_core/src/ffi/rustfuture.rs b/uniffi_core/src/ffi/rustfuture.rs index 0c1a24174b..b97e5b8d5f 100644 --- a/uniffi_core/src/ffi/rustfuture.rs +++ b/uniffi_core/src/ffi/rustfuture.rs @@ -12,7 +12,7 @@ //! //! 0. At startup, register a [RustFutureContinuationCallback] by calling //! rust_future_continuation_callback_set. -//! 1. Call the scaffolding function to get a [RustFutureHandle] +//! 1. Call the scaffolding function to get a [Handle] //! 2a. In a loop: //! - Call [rust_future_poll] //! - Suspend the function until the [rust_future_poll] continuation function is called @@ -86,7 +86,10 @@ use std::{ task::{Context, Poll, Wake}, }; -use crate::{rust_call_with_out_status, FfiDefault, LowerReturn, RustCallStatus}; +use crate::{ + derive_ffi_traits, rust_call_with_out_status, FfiDefault, Handle, LowerReturn, RustCallStatus, + SlabAlloc, +}; /// Result code for [rust_future_poll]. This is passed to the continuation function. #[repr(i8)] @@ -102,39 +105,28 @@ pub enum RustFuturePoll { /// /// The Rust side of things calls this when the foreign side should call [rust_future_poll] again /// to continue progress on the future. -pub type RustFutureContinuationCallback = extern "C" fn(callback_data: *const (), RustFuturePoll); - -/// Opaque handle for a Rust future that's stored by the foreign language code -#[repr(transparent)] -pub struct RustFutureHandle(*const ()); +pub type RustFutureContinuationCallback = extern "C" fn(callback_data: Handle, RustFuturePoll); // === Public FFI API === -/// Create a new [RustFutureHandle] +/// Create a new [Handle] for a RustFuture /// /// For each exported async function, UniFFI will create a scaffolding function that uses this to -/// create the [RustFutureHandle] to pass to the foreign code. -pub fn rust_future_new(future: F, tag: UT) -> RustFutureHandle +/// create the [Handle] to pass to the foreign code. +pub fn rust_future_new(future: F, tag: UT) -> Handle where - // F is the future type returned by the exported async function. It needs to be Send + `static - // since it will move between threads for an indeterminate amount of time as the foreign - // executor calls polls it and the Rust executor wakes it. It does not need to by `Sync`, - // since we synchronize all access to the values. + // See the [RustFuture] struct for an explanation of these trait bounds F: Future + Send + 'static, - // T is the output of the Future. It needs to implement [LowerReturn]. Also it must be Send + - // 'static for the same reason as F. T: LowerReturn + Send + 'static, - // The UniFfiTag ZST. The Send + 'static bound is to keep rustc happy. UT: Send + 'static, + // Needed to create a Handle + dyn RustFutureFfi: SlabAlloc, { // Create a RustFuture and coerce to `Arc`, which is what we use to // implement the FFI - let future_ffi = RustFuture::new(future, tag) as Arc>; - // Box the Arc, to convert the wide pointer into a normal sized pointer so that we can pass it - // to the foreign code. - let boxed_ffi = Box::new(future_ffi); - // We can now create a RustFutureHandle - RustFutureHandle(Box::into_raw(boxed_ffi) as *mut ()) + as SlabAlloc>::insert( + RustFuture::new(future, tag) as Arc> + ) } /// Poll a Rust future @@ -145,14 +137,15 @@ where /// /// # Safety /// -/// The [RustFutureHandle] must not previously have been passed to [rust_future_free] -pub unsafe fn rust_future_poll( - handle: RustFutureHandle, +/// The [Handle] must not previously have been passed to [rust_future_free] +pub unsafe fn rust_future_poll( + handle: Handle, callback: RustFutureContinuationCallback, - data: *const (), -) { - let future = &*(handle.0 as *mut Arc>); - future.clone().ffi_poll(callback, data) + data: Handle, +) where + dyn RustFutureFfi: SlabAlloc, +{ + as SlabAlloc>::get_clone(handle).ffi_poll(callback, data) } /// Cancel a Rust future @@ -164,10 +157,12 @@ pub unsafe fn rust_future_poll( /// /// # Safety /// -/// The [RustFutureHandle] must not previously have been passed to [rust_future_free] -pub unsafe fn rust_future_cancel(handle: RustFutureHandle) { - let future = &*(handle.0 as *mut Arc>); - future.clone().ffi_cancel() +/// The [Handle] must not previously have been passed to [rust_future_free] +pub unsafe fn rust_future_cancel(handle: Handle) +where + dyn RustFutureFfi: SlabAlloc, +{ + as SlabAlloc>::get_clone(handle).ffi_cancel() } /// Complete a Rust future @@ -177,15 +172,17 @@ pub unsafe fn rust_future_cancel(handle: RustFutureHandle) { /// /// # Safety /// -/// - The [RustFutureHandle] must not previously have been passed to [rust_future_free] +/// - The [Handle] must not previously have been passed to [rust_future_free] /// - The `T` param must correctly correspond to the [rust_future_new] call. It must /// be `>::ReturnType` -pub unsafe fn rust_future_complete( - handle: RustFutureHandle, +pub unsafe fn rust_future_complete( + handle: Handle, out_status: &mut RustCallStatus, -) -> ReturnType { - let future = &*(handle.0 as *mut Arc>); - future.ffi_complete(out_status) +) -> ReturnType +where + dyn RustFutureFfi: SlabAlloc, +{ + as SlabAlloc>::get_clone(handle).ffi_complete(out_status) } /// Free a Rust future, dropping the strong reference and releasing all references held by the @@ -193,12 +190,29 @@ pub unsafe fn rust_future_complete( /// /// # Safety /// -/// The [RustFutureHandle] must not previously have been passed to [rust_future_free] -pub unsafe fn rust_future_free(handle: RustFutureHandle) { - let future = Box::from_raw(handle.0 as *mut Arc>); - future.ffi_free() +/// The [Handle] must not previously have been passed to [rust_future_free] +pub unsafe fn rust_future_free(handle: Handle) +where + dyn RustFutureFfi: SlabAlloc, +{ + as SlabAlloc>::remove(handle).ffi_free() } +// Derive SlabAlloc for dyn RustFutureFfi for all FFI return types +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi); +derive_ffi_traits!(impl SlabAlloc for dyn RustFutureFfi<()>); + /// Thread-safe storage for [RustFutureContinuationCallback] data /// /// The basic guarantee is that all data pointers passed in are passed out exactly once to the @@ -215,8 +229,8 @@ enum ContinuationDataCell { /// The future has been cancelled, any future `store` calls should immediately result in the /// continuation being called with `RustFuturePoll::Ready`. Cancelled, - /// Continuation set, the next time `wake()` is called is called, we should invoke it. - Set(RustFutureContinuationCallback, *const ()), + /// Continuation set, the next time `wake()` is called, we should invoke it. + Set(RustFutureContinuationCallback, Handle), } impl ContinuationDataCell { @@ -226,7 +240,7 @@ impl ContinuationDataCell { /// Store new continuation data if we are in the `Empty` state. If we are in the `Waked` or /// `Cancelled` state, call the continuation immediately with the data. - fn store(&mut self, callback: RustFutureContinuationCallback, data: *const ()) { + fn store(&mut self, callback: RustFutureContinuationCallback, data: Handle) { match self { Self::Empty => *self = Self::Set(callback, data), Self::Set(old_callback, old_data) => { @@ -274,15 +288,10 @@ impl ContinuationDataCell { } } -// ContinuationDataCell is Send + Sync as long we handle the *const () pointer correctly - -unsafe impl Send for ContinuationDataCell {} -unsafe impl Sync for ContinuationDataCell {} - /// Wraps the actual future we're polling struct WrappedFuture where - // See rust_future_new for an explanation of these trait bounds + // See the [RustFuture] struct for an explanation of these trait bounds F: Future + Send + 'static, T: LowerReturn + Send + 'static, UT: Send + 'static, @@ -296,7 +305,7 @@ where impl WrappedFuture where - // See rust_future_new for an explanation of these trait bounds + // See the [RustFuture] struct for an explanation of these trait bounds F: Future + Send + 'static, T: LowerReturn + Send + 'static, UT: Send + 'static, @@ -375,7 +384,7 @@ where // that we will treat the raw pointer properly, for example by not returning it twice. unsafe impl Send for WrappedFuture where - // See rust_future_new for an explanation of these trait bounds + // See the [RustFuture] struct for an explanation of these trait bounds F: Future + Send + 'static, T: LowerReturn + Send + 'static, UT: Send + 'static, @@ -385,7 +394,16 @@ where /// Future that the foreign code is awaiting struct RustFuture where - // See rust_future_new for an explanation of these trait bounds + // F is the future type returned by the exported async function. It needs to be Send + `static + // since it will move between threads for an indeterminate amount of time as the foreign + // executor calls polls it and the Rust executor wakes it. It does not need to by `Sync`, + // since we synchronize all access to the values. + F: Future + Send + 'static, + // T is the output of the Future. It needs to implement [LowerReturn]. Also it must be Send + + // 'static for the same reason as F. + T: LowerReturn + Send + 'static, + // The UniFfiTag ZST. The Send + 'static bound is to keep rustc happy. + UT: Send + 'static, F: Future + Send + 'static, T: LowerReturn + Send + 'static, UT: Send + 'static, @@ -401,7 +419,7 @@ where impl RustFuture where - // See rust_future_new for an explanation of these trait bounds + // See the [RustFuture] struct for an explanation of these trait bounds F: Future + Send + 'static, T: LowerReturn + Send + 'static, UT: Send + 'static, @@ -414,7 +432,7 @@ where }) } - fn poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()) { + fn poll(self: Arc, callback: RustFutureContinuationCallback, data: Handle) { let ready = self.is_cancelled() || { let mut locked = self.future.lock().unwrap(); let waker: std::task::Waker = Arc::clone(&self).into(); @@ -453,7 +471,7 @@ where impl Wake for RustFuture where - // See rust_future_new for an explanation of these trait bounds + // See the [RustFuture] struct for an explanation of these trait bounds F: Future + Send + 'static, T: LowerReturn + Send + 'static, UT: Send + 'static, @@ -479,8 +497,8 @@ where /// x86-64 machine . By parametrizing on `T::ReturnType` we can instead monomorphize by hand and /// only create those functions for each of the 13 possible FFI return types. #[doc(hidden)] -trait RustFutureFfi { - fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()); +pub trait RustFutureFfi { + fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: Handle); fn ffi_cancel(&self); fn ffi_complete(&self, call_status: &mut RustCallStatus) -> ReturnType; fn ffi_free(self: Arc); @@ -488,12 +506,12 @@ trait RustFutureFfi { impl RustFutureFfi for RustFuture where - // See rust_future_new for an explanation of these trait bounds + // See the [RustFuture] struct for an explanation of these trait bounds F: Future + Send + 'static, T: LowerReturn + Send + 'static, UT: Send + 'static, { - fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()) { + fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: Handle) { self.poll(callback, data) } @@ -574,16 +592,21 @@ mod tests { (Sender(channel), rust_future) } + // Use an Arc for the continuation data. The callback will set the cell when the + // continuation is called + static CONTINUATION_DATA_SLAB: crate::Slab>> = + crate::Slab::new_with_id_and_foreign(0, true); + /// Poll a Rust future and get an OnceCell that's set when the continuation is called fn poll(rust_future: &Arc>) -> Arc> { let cell = Arc::new(OnceCell::new()); - let cell_ptr = Arc::into_raw(cell.clone()) as *const (); - rust_future.clone().ffi_poll(poll_continuation, cell_ptr); + let handle = CONTINUATION_DATA_SLAB.insert_or_panic(Arc::clone(&cell)); + rust_future.clone().ffi_poll(poll_continuation, handle); cell } - extern "C" fn poll_continuation(data: *const (), code: RustFuturePoll) { - let cell = unsafe { Arc::from_raw(data as *const OnceCell) }; + extern "C" fn poll_continuation(data: Handle, code: RustFuturePoll) { + let cell = CONTINUATION_DATA_SLAB.get_clone_or_panic(data); cell.set(code).expect("Error setting OnceCell"); } diff --git a/uniffi_core/src/ffi/slab.rs b/uniffi_core/src/ffi/slab.rs new file mode 100644 index 0000000000..d7e4dc75ce --- /dev/null +++ b/uniffi_core/src/ffi/slab.rs @@ -0,0 +1,1042 @@ +/* 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/. */ + +//! Store Arc references owned by the foreign side and use handles to manage them +//! +//! This module defines the [Slab] class allows us to insert `Arc<>` values and use [Handle] values to manage the allocation. +//! It's named "Slab" because the data structure resembles a slab-allocator, for example the `tokio` `slab` crate (https://github.com/tokio-rs/slab). +//! +//! Usage: +//! * Create a `Slab` that will store Arc values. +//! * Call `insert()` to store a value and allocated a handle that represents a single strong ref. +//! * Pass the handle across the FFI to the foreign side. +//! * When the foreign side wants to use that value, it passes back the handle back to Rust. +//! * If the FFI call treats the handle arg as a borrow, then Rust calls `get_clone` to get the stored value +//! * If the FFI call treats the handle arg as an owned value, then Rust calls `remove` to get the stored value and decrement the ref count. +//! * The foreign side can call `inc_ref` if they want to pass an owned reference back and continue to use the handle (See #1797) +//! +//! Using handles to manage arc references provides several benefits: +//! * Handles are simple integer values, which are simpler to work with than pointers. +//! * Handles store a generation counter, which can usually detect use-after-free bugs. +//! * Handles store an slab id, which can usually detect using handles with the wrong Slab. +//! * Handles only use 48 bits, which makes them easier to work with on languages like JS that don't support full 64-bit integers. +//! * Handles are signed, but always positive. This allows using negative numbers for special values. +//! Also, signed ints integrate with JNA easier. +//! * Handles have a bit to differentiate between foreign-allocated handles and rust-allocated ones. +//! The trait interface code uses this to differentiate between Rust-implemented and foreign-implemented traits. + +use std::fmt; + +use append_only_vec::AppendOnlyVec; +use sync::*; + +#[cfg(not(loom))] +mod sync { + pub(super) use std::{ + sync::{ + atomic::{AtomicU8, Ordering}, + Mutex, + }, + thread, + }; + + // Wrap UnsafeCell so that it has the same API as loom + #[derive(Debug)] + pub(crate) struct UnsafeCell(std::cell::UnsafeCell); + + impl UnsafeCell { + pub(crate) const fn new(data: T) -> UnsafeCell { + UnsafeCell(std::cell::UnsafeCell::new(data)) + } + + pub(crate) unsafe fn with(&self, f: impl FnOnce(*const T) -> R) -> R { + f(self.0.get()) + } + + pub(crate) unsafe fn with_mut(&self, f: impl FnOnce(*mut T) -> R) -> R { + f(self.0.get()) + } + } +} + +// Note: use the `cargo slab-loom-test` command to test with loom +#[cfg(loom)] +mod sync { + pub(super) use loom::{ + cell::UnsafeCell, + sync::{ + atomic::{AtomicU8, Ordering}, + Mutex, + }, + thread, + }; +} + +// This code assumes that usize is at least 32 bits +static_assertions::const_assert!(std::mem::size_of::() >= std::mem::size_of::()); +// Entry should add 64 bits of storage for unit values, concrete `Arc`, and `Arc`. +#[cfg(not(loom))] +static_assertions::const_assert!(std::mem::size_of::>() == std::mem::size_of::<()>() + 8); +#[cfg(not(loom))] +static_assertions::const_assert!( + std::mem::size_of::>>() + == std::mem::size_of::>() + 8 +); +#[cfg(not(loom))] +static_assertions::const_assert!( + std::mem::size_of::>>() + == std::mem::size_of::>() + 8 +); + +/// Slab error type +#[derive(Debug, PartialEq, Eq)] +pub enum SlabError { + SlabIdMismatch, + RustHandle, + ForeignHandle, + UseAfterFree(&'static str), + OverCapacity, + RefCountLimit, + ReaderCountLimit, + Vacant, + OutOfBounds, + LockTimeout, +} + +impl fmt::Display for SlabError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::UseAfterFree(msg) => write!( + f, + "Slab error: {msg} (was the handle re-used after being passed to remove()?)" + ), + Self::SlabIdMismatch => write!(f, "Slab id mismatch"), + Self::RustHandle => write!(f, "Handle belongs to a rust slab"), + Self::ForeignHandle => write!(f, "Handle belongs to a foreign slab"), + Self::OverCapacity => write!(f, "Slab capacity exceeded"), + Self::RefCountLimit => write!(f, "Reference count limit exceeded"), + Self::ReaderCountLimit => write!(f, "Reader count limit exceeded"), + Self::Vacant => write!(f, "Entry unexpectedly vacant"), + Self::OutOfBounds => write!(f, "Index out of bounds"), + Self::LockTimeout => write!(f, "Lock timeout"), + } + } +} + +impl std::error::Error for SlabError {} + +pub type Result = std::result::Result; + +/// Index segment of a handle +const INDEX_MASK: i64 = 0x0000_FFFF_FFFF; +/// Foreign bit of a handle +const FOREIGN_BIT: i64 = 0x0001_0000_0000; +/// Special-cased value for the `next` field that means no next entry. +const END_OF_LIST: u32 = u32::MAX; + +/// Handle for a value stored in the slab +/// +/// * The first 32 bits identify the value. +/// * The next 8 bits are for an slab id: +/// - The first bit is 0 if the handle came from Rust and 1 if it came from the foreign side. +/// - The next 7 bits are used to identify the slab. Use random values or a counter. +/// - This means that using a handle with the wrong Slab will be detected > 99% of the time. +/// * The next 8 bits are a generation counter value, this means that use-after-free bugs will be +/// detected until at least 256 inserts are performed after the free. +/// * The last 16 bits are intentionally unset, so that these can be easily used on languages like +/// JS that don't support full 64-bit integers. +#[derive(Clone, Copy, Default, PartialEq, Eq, Hash)] +#[repr(transparent)] +pub struct Handle(i64); + +impl Handle { + const fn new(slab_id: u8, generation: u8, index: u32) -> Self { + Self((generation as i64) << 40 | (slab_id as i64) << 32 | index as i64) + } + + pub const fn from_raw(val: i64) -> Self { + Self(val) + } + + pub const fn as_raw(&self) -> i64 { + self.0 + } + + fn index(&self) -> usize { + (self.0 & INDEX_MASK) as usize + } + + fn generation(&self) -> u8 { + (self.0 >> 40) as u8 + } + + fn slab_id(&self) -> u8 { + (self.0 >> 32) as u8 + } + + pub fn is_from_rust(&self) -> bool { + self.0 & FOREIGN_BIT == 0 + } + + pub fn is_foreign(&self) -> bool { + self.0 & FOREIGN_BIT != 0 + } +} + +impl fmt::Debug for Handle { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "handle-{}#{}", self.index(), self.generation()) + } +} + +/// Entry a Slab +/// +/// Entries can be vacant or occupied. +/// Vacant entries are part of the Slab's free list and don't have handles allocated for them. +/// Occupied entries are not part of the free list and are managed by handles. +/// +/// Entries store a generation counter that's incremented each time it transitions from occupied to vacant. +/// Handles store the generation counter of the entry they were allocated for. +/// When handles are used, we check that the generation counters match. +/// This mostly ensures that use-after-free bugs are detected, although it's possible for the 8-bit counter to roll over. +#[derive(Debug)] +struct Entry { + /// For vacant entries, next entry in the free list. + /// + /// # Safety + /// + /// Only access this while [Slab::next_lock] is held. + next: UnsafeCell, + /// Generation counter + generation: AtomicU8, + /// Protects `ref_count` and `value`. + /// + /// Bit 0 is a write lock. + /// Bits 1..8 form a reader count. + /// The lock will only be contended if the foreign code uses a handle after it's been freed. + state: AtomicU8, + /// Reference count, this can be atomically updated by the readers after they've read-locked + /// `state` and checked the generation value. This is pretty small, but that's okay because + /// it's only used to temporarily retain a reference that's being returned across the FFI (see + /// #1797). + ref_count: UnsafeCell, + value: UnsafeCell>, +} + +impl Entry { + const WRITE_LOCK_BIT: u8 = 0x01; + const READER_COUNT_UNIT: u8 = 0x02; + // If ref_count or reader count get close to overflowing, then we should error out. + // + // Both of these numbers should never bit hit in practice. + // Overflowing the ref count requires 200 threads to be suspended right after they returned the same handle, but before the Rust removed it. + // Overflowing the reader count requires require 64 threads to be suspended in the middle of a `read()` operation, which are typically just a handful of CPU cycles. + const REF_COUNT_LIMIT: u8 = 200; + const READER_COUNT_LIMIT: u8 = Self::READER_COUNT_UNIT * 64; + + fn new_occupied(value: T) -> Self { + Self { + next: UnsafeCell::new(END_OF_LIST), + state: AtomicU8::new(0), + generation: AtomicU8::new(0), + ref_count: UnsafeCell::new(AtomicU8::new(1)), + value: UnsafeCell::new(Some(value)), + } + } + + fn acquire_read_lock(&self, handle: Handle) -> Result<()> { + // Increment the reader count. Use a spin lock to wait for writers. As long as the foreign + // code isn't using handles after they're freed, there will never be contention. + let mut counter = 0; + loop { + let prev_state = self + .state + .fetch_add(Self::READER_COUNT_UNIT, Ordering::Acquire); + if !self.generation_matches(handle) { + self.release_read_lock(); + return Err(SlabError::UseAfterFree("generation mismatch")); + } else if prev_state >= Self::READER_COUNT_LIMIT { + self.release_read_lock(); + return Err(SlabError::ReaderCountLimit); + } else if prev_state & Self::WRITE_LOCK_BIT == 0 { + return Ok(()); + } + self.release_read_lock(); + // As mentioned above, the lock should never be contended and locks are only held for a + // handful of instructions, so let's use an extremely simple solution to manage + // contention. + if counter < 100 { + thread::yield_now(); + counter += 1; + } else { + return Err(SlabError::LockTimeout); + } + } + } + + fn release_read_lock(&self) { + self.state + .fetch_sub(Self::READER_COUNT_UNIT, Ordering::Release); + } + + fn acquire_write_lock(&self) -> Result<()> { + // Set the write lock bit. Use a spin lock to wait for writers and readers. As long as the + // foreign code isn't using handles after they're freed, there will never be contention. + let mut counter = 0; + while self + .state + .compare_exchange_weak( + 0, + Self::WRITE_LOCK_BIT, + Ordering::Acquire, + Ordering::Relaxed, + ) + .is_err() + { + // See `acquire_read_lock` for notes on this. + if counter < 100 { + thread::yield_now(); + counter += 1; + } else { + return Err(SlabError::LockTimeout); + } + } + Ok(()) + } + + fn release_write_lock(&self) { + self.state + .fetch_and(!Self::WRITE_LOCK_BIT, Ordering::Release); + } + + fn generation_matches(&self, handle: Handle) -> bool { + self.generation.load(Ordering::Relaxed) == handle.generation() + } + + /// Perform a operation with the read lock + fn read(&self, handle: Handle, f: F) -> Result + where + F: FnOnce(&AtomicU8, &Option) -> Result, + { + self.acquire_read_lock(handle)?; + let result = unsafe { + // Safety: We hold a read lock + self.ref_count + .with(|ref_count| self.value.with(|v| f(&*ref_count, &*v))) + }; + self.release_read_lock(); + result + } + + /// Perform an operation with the write lock + /// + /// This is marked unsafe because it does not check the generation. Only call this if you + /// know that you should have access to the entry. + unsafe fn write(&self, f: F) -> Result<()> + where + F: FnOnce(&mut AtomicU8, &mut Option), + { + self.acquire_write_lock()?; + unsafe { + // Safety: We hold the write lock + self.ref_count + .with_mut(|ref_count| self.value.with_mut(|v| f(&mut *ref_count, &mut *v))) + }; + self.release_write_lock(); + Ok(()) + } + + /// Increment the ref count + fn inc_ref(&self, handle: Handle) -> Result<()> { + // Increment the ref count inside `read` to ensure the generation counter matches + self.read(handle, |ref_count, _| { + let prev_ref_count = ref_count.fetch_add(1, Ordering::Relaxed); + if prev_ref_count >= Self::REF_COUNT_LIMIT { + ref_count.fetch_sub(1, Ordering::Relaxed); + Err(SlabError::RefCountLimit) + } else { + Ok(()) + } + }) + } + + /// Get a cloned value + fn get_clone(&self, handle: Handle) -> Result { + // Decrement the ref count inside `read` to ensure the generation counter matches. + self.read(handle, |_, value| match value { + Some(v) => Ok(v.clone()), + None => Err(SlabError::Vacant), + }) + } + + /// Remove a reference + /// + /// Returns the inner value plus an extra `needs_free` flag which indicates that + /// the entry should be return to the free list. + fn remove(&self, handle: Handle) -> Result<(T, bool)> { + // Decrement the ref count inside `read` to ensure the generation counter matches. + self.read(handle, |ref_count, value| { + let value = match value { + Some(v) => v.clone(), + None => return Err(SlabError::Vacant), + }; + let needs_free = ref_count.fetch_sub(1, Ordering::Relaxed) == 1; + Ok((value, needs_free)) + }) + .and_then(|(v, needs_free)| { + if needs_free { + // make_vacant() should never fail here as long as our internal logic is correct. + self.make_vacant(handle)?; + } + Ok((v, needs_free)) + }) + } + + /// Transition an entry to vacant + fn make_vacant(&self, handle: Handle) -> Result<()> { + self.generation + .compare_exchange_weak( + handle.generation(), + handle.generation().wrapping_add(1), + Ordering::Relaxed, + Ordering::Relaxed, + ) + .map_err(|_| SlabError::UseAfterFree("simultaneous frees"))?; + + // Safety: we successfully incremented the generation counter, so we know that our handle + // was valid for the entry. + unsafe { + self.write(|_, value| { + *value = None; + }) + } + } + + /// Transition an entry to occupied and return the generation value + /// + /// # Safety + /// + /// Must only be called on vacant entries that have been removed the free list before any new + /// handles are allocated. + unsafe fn make_occupied(&self, new_value: T) -> Result { + // Safety: the entry was just removed from the free list, so we have access to it + unsafe { + self.write(|ref_count, value| { + *value = Some(new_value); + ref_count.store(1, Ordering::Relaxed); + })?; + } + Ok(self.generation.load(Ordering::Relaxed)) + } +} + +/// Allocates handles that represent stored values and can be shared by the foreign code +pub struct Slab { + is_foreign: bool, + // Slab ID, including the foreign bit + slab_id: u8, + // Use an append-only vec, which has the nice property that we can push to it with a shared + // reference + entries: AppendOnlyVec>, + // Next entry in the free list. + next: UnsafeCell, + // Protects [Slab::next] and the [Entry::next] field for all entries in the slab. + next_lock: Mutex<()>, +} + +impl Slab { + #[cfg(not(loom))] + pub const fn new_with_id_and_foreign(slab_id: u8, is_foreign: bool) -> Self { + Self { + slab_id: if is_foreign { + (slab_id << 1) | 1 + } else { + slab_id << 1 + }, + is_foreign, + entries: AppendOnlyVec::new(), + next: UnsafeCell::new(END_OF_LIST), + next_lock: Mutex::new(()), + } + } + + /// This needs to be non-const because loom's AtomicU32::new() is non-const. + #[cfg(loom)] + pub fn new_with_id_and_foreign(slab_id: u8, is_foreign: bool) -> Self { + Self { + slab_id: if is_foreign { + (slab_id << 1) | 1 + } else { + slab_id << 1 + }, + is_foreign, + entries: AppendOnlyVec::new(), + next: UnsafeCell::new(END_OF_LIST), + next_lock: Mutex::new(()), + } + } + + /// Get an entry for a handle, if the handle is still valid + fn get_entry(&self, handle: Handle) -> Result<&Entry> { + let index = handle.index(); + if handle.slab_id() != self.slab_id { + if handle.is_foreign() && !self.is_foreign { + return Err(SlabError::ForeignHandle); + } else if !handle.is_foreign() && self.is_foreign { + return Err(SlabError::RustHandle); + } else { + return Err(SlabError::SlabIdMismatch); + } + } + if index < self.entries.len() { + Ok(&self.entries[index]) + } else { + Err(SlabError::OutOfBounds) + } + } + + /// Insert a new item into the Slab, either by pushing it to the end or re-allocating a previously removed entry. + pub fn insert(&self, value: T) -> Result { + let _guard = self.next_lock.lock().unwrap(); + unsafe { + // Safety: we hold `next_lock` + self.next.with_mut(|next| { + if *next == END_OF_LIST { + // No vacant entries, create a new one + if self.entries.len() + 1 >= END_OF_LIST as usize { + // ~4 billion entries allocated, a new one will overflow the bits available + // in the handle. + Err(SlabError::OverCapacity) + } else { + let index = self.entries.push(Entry::new_occupied(value)); + Ok(Handle::new(self.slab_id, 0, index as u32)) + } + } else { + // Pop a vacant entry off the free list + let entry_index = *next; + let entry = &self.entries[entry_index as usize]; + // Safety: we hold `next_lock` + entry.next.with(|entry_next| *next = *entry_next); + // Safety: + // + // We have removed entry from the free list and not allocated any + // handles yet. + // + // make_occupied() should never fail here as long as our internal logic is + // correct. + let generation = entry.make_occupied(value)?; + Ok(Handle::new(self.slab_id, generation, entry_index)) + } + }) + } + } + + /// Get a cloned value from a handle + pub fn get_clone(&self, handle: Handle) -> Result { + self.get_entry(handle)?.get_clone(handle) + } + + /// Increment the reference count + pub fn inc_ref(&self, handle: Handle) -> Result<()> { + self.get_entry(handle)?.inc_ref(handle) + } + + /// Remove a reference + /// + /// This decrements the reference count, returns the inner value and if the entry was freed + pub fn remove(&self, handle: Handle) -> Result<(T, bool)> { + let entry = self.get_entry(handle)?; + entry.remove(handle).and_then(|(v, needs_free)| { + if needs_free { + self.free_entry(handle, entry)?; + } + Ok((v, needs_free)) + }) + } + + /// Add an entry back to the free list + fn free_entry(&self, handle: Handle, entry: &Entry) -> Result<()> { + let _guard = self.next_lock.lock().unwrap(); + unsafe { + // Safety: we hold `next_lock' + self.next.with_mut(|next| { + // Safety: we hold `next_lock' + entry.next.with_mut(|entry_next| { + *entry_next = *next; + *next = handle.index() as u32; + }) + }); + } + Ok(()) + } + + pub fn insert_or_panic(&self, value: T) -> Handle { + self.insert(value).unwrap_or_else(|e| panic!("{e}")) + } + + pub fn inc_ref_or_panic(&self, handle: Handle) { + self.inc_ref(handle).unwrap_or_else(|e| panic!("{e}")) + } + + pub fn get_clone_or_panic(&self, handle: Handle) -> T { + self.get_clone(handle).unwrap_or_else(|e| panic!("{e}")) + } + + pub fn remove_or_panic(&self, handle: Handle) -> (T, bool) { + self.remove(handle).unwrap_or_else(|e| panic!("{e}")) + } +} + +// If the code above is correct, then Slab is Send + Sync +unsafe impl Send for Slab {} +unsafe impl Sync for Slab {} + +#[cfg(test)] +impl Entry { + fn reader_count(&self) -> u8 { + self.state.load(Ordering::Relaxed) / Self::READER_COUNT_UNIT + } + + fn ref_count(&self) -> u8 { + unsafe { + self.ref_count + .with(|ref_count| (*ref_count).load(Ordering::Relaxed)) + } + } +} + +#[cfg(test)] +mod entry_tests { + use super::*; + use std::sync::{Arc, Weak}; + + fn test_setup() -> (Entry>, Handle, Weak<()>) { + let obj = Arc::new(()); + let weak = Arc::downgrade(&obj); + let entry = Entry::new_occupied(obj); + let handle = Handle::new(0, 0, 0); + (entry, handle, weak) + } + + #[test] + fn test_ref_count() { + let (entry, handle, weak) = test_setup(); + assert_eq!(entry.ref_count(), 1); + entry.inc_ref(handle).unwrap(); + assert_eq!(entry.ref_count(), 2); + let needs_free = entry.remove(handle).unwrap().1; + assert_eq!(entry.ref_count(), 1); + assert!(!needs_free); + let needs_free = entry.remove(handle).unwrap().1; + assert!(needs_free); + assert_eq!(weak.strong_count(), 0); + } + + #[test] + fn test_extra_release() { + let (entry, handle, _) = test_setup(); + entry.remove(handle).unwrap(); + assert!(entry.remove(handle).is_err()); + } + + // Test that incrementing the reader count fails before getting close to the limit + #[test] + fn test_ref_count_overflow() { + // Create an entry with ref_count = 1 + let (entry, handle, weak) = test_setup(); + // Incrementing this many times is okay + for _ in 0..199 { + entry.inc_ref(handle).unwrap(); + } + // 1 more should fail because it gets us too close the limit where we run out of bits + assert_eq!(entry.inc_ref(handle), Err(SlabError::RefCountLimit)); + // If we remove the references then the value should be freed. + for _ in 0..200 { + entry.remove(handle).unwrap(); + } + assert_eq!(weak.strong_count(), 0); + } + + // Test that incrementing the reader count fails before getting close to the limits + #[test] + fn test_reader_overflow() { + // 800 readers are okay + let (entry, handle, _) = test_setup(); + for _ in 0..64 { + entry.acquire_read_lock(handle).unwrap(); + } + // 1 more should fail because it gets us too close to the limit where we run out of bits + assert_eq!(entry.inc_ref(handle), Err(SlabError::ReaderCountLimit)); + // Test decrementing the reader count + for _ in 0..64 { + entry.release_read_lock(); + } + assert_eq!(entry.reader_count(), 0); + } +} + +/// Create a static Slab value with a random id +#[macro_export] +macro_rules! static_slab { + ($ident:ident, $($tt:tt)*) => { + #[cfg(not(loom))] + static $ident: $crate::Slab::<$($tt)*> = $crate::Slab::<$($tt)*>::new_with_id_and_foreign($crate::deps::const_random::const_random!(u8), false); + + // If loom is configured, then new_with_id_and_foreign won't be const so we need to wrap + // everything in a once_cell `Lazy` instance. + #[cfg(loom)] + static $ident: ::once_cell::sync::Lazy<$crate::Slab::<$($tt)*>> = ::once_cell::sync::Lazy::new(|| { + $crate::Slab::<$($tt)*>::new_with_id_and_foreign($crate::deps::const_random::const_random!(u8), false) + }); + }; +} + +#[cfg(test)] +mod slab_tests { + use super::*; + use rand::{rngs::StdRng, RngCore, SeedableRng}; + use std::sync::Arc; + + #[test] + fn test_simple_usage() { + let slab = Slab::new_with_id_and_foreign(0, false); + let handle1 = slab.insert(Arc::new("Hello")).unwrap(); + let handle2 = slab.insert(Arc::new("Goodbye")).unwrap(); + assert_eq!(slab.entries.len(), 2); + assert_eq!(*slab.get_clone(handle1).unwrap(), "Hello"); + slab.remove(handle1).unwrap(); + assert_eq!(*slab.get_clone(handle2).unwrap(), "Goodbye"); + slab.remove(handle2).unwrap(); + } + + #[test] + fn test_slab_id_check() { + let slab = Slab::>::new_with_id_and_foreign(1, false); + let slab2 = Slab::>::new_with_id_and_foreign(2, false); + let handle = slab.insert(Arc::new("Hello")).unwrap(); + assert_eq!(Err(SlabError::SlabIdMismatch), slab2.get_clone(handle)); + assert_eq!(Err(SlabError::SlabIdMismatch), slab2.remove(handle)); + } + + #[test] + fn test_foreign_handle_with_rust_slab() { + let slab = Slab::>::new_with_id_and_foreign(1, false); + let handle = slab.insert(Arc::new("Hello")).unwrap(); + let foreign_handle = Handle::from_raw(handle.as_raw() | FOREIGN_BIT); + assert_eq!( + Err(SlabError::ForeignHandle), + slab.get_clone(foreign_handle) + ); + } + + #[test] + fn test_rust_handle_with_foreign_slab() { + let slab = Slab::>::new_with_id_and_foreign(1, true); + let handle = slab.insert(Arc::new("Hello")).unwrap(); + let rust_handle = Handle::from_raw(handle.as_raw() & !FOREIGN_BIT); + assert_eq!(Err(SlabError::RustHandle), slab.get_clone(rust_handle)); + } + + fn rand_index(rng: &mut StdRng, vec: &Vec) -> usize { + rng.next_u32() as usize % vec.len() + } + + // Wraps a slab for easier testing + #[derive(Clone)] + pub struct TestSlab { + slab: Arc>, + counter: Arc, + } + + impl TestSlab { + pub fn new() -> Self { + Self { + slab: Arc::new(Slab::new_with_id_and_foreign(0, false)), + counter: Arc::new(AtomicU8::new(0)), + } + } + + pub fn insert(&self) -> TestHandle { + let value = self.counter.fetch_add(1, Ordering::Relaxed); + let handle = self.slab.insert(value).unwrap(); + TestHandle { + handle, + value, + ref_count: 1, + } + } + + pub fn check(&self, handle: &TestHandle) { + let value = self.slab.get_clone(handle.handle).unwrap(); + assert_eq!(value, handle.value); + } + + pub fn inc_ref(&self, handle: &mut TestHandle) { + self.slab.inc_ref(handle.handle).unwrap(); + handle.ref_count += 1; + } + + pub fn remove(&self, handle: &mut TestHandle) -> bool { + handle.ref_count -= 1; + let (value, freed) = self.slab.remove(handle.handle).unwrap(); + assert_eq!(value, handle.value); + assert_eq!(freed, handle.ref_count == 0); + freed + } + + pub fn check_use_after_free_detection(&self, handle: &TestHandle) { + let result = self.slab.get_clone(handle.handle); + assert!( + matches!(result, Err(SlabError::UseAfterFree(_))), + "{result:?}" + ); + } + } + + // Store a handle, it's entry's value, and it's ref count together + pub struct TestHandle { + pub handle: Handle, + pub value: u8, + pub ref_count: u8, + } + + impl fmt::Debug for TestHandle { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.handle.fmt(f) + } + } + + #[test] + fn stress_test() { + let mut rng = StdRng::seed_from_u64(42); + for i in 0..100 { + println!("---------------------- {i} ------------------------"); + let slab = TestSlab::new(); + let mut allocated = vec![]; + let mut freed = vec![]; + // Note; the inner loop is 255 elements, because that's the limit of insertions before + // our use-after-free detection can fail. + for _ in 0..255 { + // Insert or remove a handle + let roll = rng.next_u32() % 3; + if allocated.is_empty() || roll == 0 { + // Insert + println!("slab.insert()"); + let handle = slab.insert(); + println!("{handle:?}: handle"); + allocated.push(handle); + } else if roll == 2 { + // inc_ref + let idx = rand_index(&mut rng, &allocated); + let handle = &mut allocated[idx]; + println!("{handle:?}: inc_ref"); + slab.inc_ref(handle); + } else { + // Remove + let idx = rand_index(&mut rng, &allocated); + let handle = &mut allocated[idx]; + println!("{handle:?}: remove"); + if slab.remove(handle) { + println!("{handle:?}: freed"); + freed.push(allocated.remove(idx)); + } + } + + // Test getting all handles, allocated or freed + for handle in allocated.iter() { + println!("{handle:?}: check"); + slab.check(handle); + } + for handle in freed.iter() { + println!("{handle:?}: check_use_after_free_detection"); + slab.check_use_after_free_detection(handle); + } + } + } + } +} + +#[cfg(loom)] +mod slab_loom_test { + use super::slab_tests::{TestHandle, TestSlab}; + use super::*; + use loom::{ + model::Builder, + sync::{atomic::AtomicU64, Arc}, + thread, + }; + + // Simple tracing for the loom tests. + macro_rules! trace { + ($($tt:tt)*) => { + println!("{:?}: {}", thread::current().id(), format!($($tt)*)); + } + } + + // In these tests we're going to swap handles using AtomicU64 + impl TestHandle { + pub fn as_raw(&self) -> u64 { + self.handle.as_raw() as u64 | (self.value as u64) << 48 | (self.ref_count as u64) << 56 + } + + pub fn from_raw(raw: u64) -> Self { + Self { + handle: Handle::from_raw((raw & 0xFFFF_FFFF_FFFF) as i64), + value: ((raw >> 48) & 0xFF) as u8, + ref_count: ((raw >> 56) & 0xFF) as u8, + } + } + + pub fn swap(&mut self, shared: &AtomicU64) { + let raw = shared.swap(self.as_raw(), Ordering::AcqRel); + *self = Self::from_raw(raw) + } + } + + /// Test a set of threads that shares handles between themselves + /// + /// This runs the same basic test with different parameters. These numbers may seem low, but + /// they cause loom to run a tens of thousands of combinations. + #[test] + fn test_shared_handles() { + // Test with less threads but a higher preemption bound + test_shared_handles_case(2, 4, 3); + // Test with more threads, but a lower preemption bound + test_shared_handles_case(3, 4, 2); + } + + fn test_shared_handles_case(thread_count: usize, iterations: usize, preemption_bound: usize) { + let mut builder = Builder::default(); + builder.max_branches = 10_000; + // Limit the number of times a thread can be pre-empted. This severely limits the number + // of iterations loom needs to run. The `loom` docs say "2-3 is enough to catch most + // bugs", and this has been true in testing. Let's stay slightly on the cautious side and + // set it to 4. + builder.preemption_bound = Some(preemption_bound); + let iteration_counter = std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)); + + builder.check(move || { + trace!( + "---------------------- {} -----------------------------", + iteration_counter.fetch_add(1, Ordering::Relaxed) + ); + let slab = TestSlab::new(); + // Used to share handles between threads + let shared = Arc::new(AtomicU64::new(slab.insert().as_raw())); + for _ in 0..thread_count { + let slab = slab.clone(); + let shared = shared.clone(); + thread::spawn(move || { + trace!("startup"); + let mut current = slab.insert(); + trace!("{current:?}: initial handle"); + let mut freed_handles = vec![]; + for _ in 0..iterations { + trace!("{current:?}: swapping out"); + current.swap(&shared); + trace!("{current:?}: inc_ref"); + slab.inc_ref(&mut current); + trace!("{current:?}: check"); + slab.check(¤t); + // Swap and dec-ref + trace!("{current:?}: swapping out"); + current.swap(&shared); + trace!("{current:?}: remove"); + let freed = slab.remove(&mut current); + trace!("{current:?}: {}", if freed { "freed" } else { "live" }); + if freed { + freed_handles.push(current); + trace!("inserting new handle"); + current = slab.insert(); + trace!("{current:?}: new handle"); + } + // Check all freed handles + for freed in &freed_handles { + trace!("{freed:?}: get_clone for freed handle check"); + slab.check_use_after_free_detection(freed); + } + trace!("loop done"); + } + }); + } + }) + } + + /// Test two threads calling `remove` when there's only 1 reference + #[test] + fn test_extra_remove() { + loom::model(|| { + let slab = Arc::new(Slab::new_with_id_and_foreign(0, false)); + let slab2 = Arc::clone(&slab); + let handle = slab.insert(42).unwrap(); + + let result1 = thread::spawn(move || slab.remove(handle)).join().unwrap(); + let result2 = thread::spawn(move || slab2.remove(handle)).join().unwrap(); + // One remove should succeed and one should fail with `SlabError::UseAfterFree` + match (&result1, &result2) { + (Ok((42, true)), Err(SlabError::UseAfterFree(_))) + | (Err(SlabError::UseAfterFree(_)), Ok((42, true))) => (), + _ => panic!("Unexpected results: ({result1:?}, {result2:?})"), + } + }) + } + + /// Test one threads calling `remove`` and one calling `get_clone` when there's only 1 reference + #[test] + fn test_get_with_extra_remove() { + loom::model(|| { + let slab = Arc::new(Slab::new_with_id_and_foreign(0, false)); + let slab2 = Arc::clone(&slab); + let handle = slab.insert(42).unwrap(); + + let result1 = thread::spawn(move || slab.get_clone(handle)) + .join() + .unwrap(); + let result2 = thread::spawn(move || slab2.remove(handle)).join().unwrap(); + // `get_clone` may or may not succeed, remove should always succeed + match (&result1, &result2) { + (Ok(42), Ok((42, true))) | (Err(SlabError::UseAfterFree(_)), Ok((42, true))) => (), + _ => panic!("Unexpected results: ({result1:?}, {result2:?})"), + } + }) + } + + /// Test various combinations of: + /// * an extra `remove`, + /// * a `get_clone` + /// * An `insert` that may re-allocate the entry + #[test] + fn test_invalid_access_combos() { + loom::model(|| { + let slab = Arc::new(Slab::new_with_id_and_foreign(0, false)); + let slab2 = Arc::clone(&slab); + let slab3 = Arc::clone(&slab); + let slab4 = Arc::clone(&slab); + let handle = slab.insert(42).unwrap(); + + let result1 = thread::spawn(move || slab.get_clone(handle)) + .join() + .unwrap(); + let result2 = thread::spawn(move || slab2.remove(handle)).join().unwrap(); + let result3 = thread::spawn(move || slab3.remove(handle)).join().unwrap(); + let result4 = thread::spawn(move || slab4.insert(43)).join().unwrap(); + // * `get_clone` may or may not succeed + // * One of the `remove` calls should succeed + // * `insert` should always succeed + match &result1 { + Ok(42) | Err(SlabError::UseAfterFree(_)) => (), + _ => panic!("Unexpected get_clone() result: {result1:?}"), + } + match (&result2, &result3) { + (Ok((42, true)), Err(SlabError::UseAfterFree(_))) + | (Err(SlabError::UseAfterFree(_)), Ok((42, true))) => (), + _ => panic!("Unexpected remove() results: ({result2:?}, {result3:?})"), + } + match &result4 { + Ok(_) => (), + _ => panic!("Unexpected insert() result: {result4:?}"), + } + }) + } +} diff --git a/uniffi_core/src/ffi_converter_impls.rs b/uniffi_core/src/ffi_converter_impls.rs index af18f3873b..0ead84f3f4 100644 --- a/uniffi_core/src/ffi_converter_impls.rs +++ b/uniffi_core/src/ffi_converter_impls.rs @@ -23,7 +23,7 @@ /// "UT" means an abitrary `UniFfiTag` type. use crate::{ check_remaining, derive_ffi_traits, ffi_converter_rust_buffer_lift_and_lower, metadata, - ConvertError, FfiConverter, ForeignExecutor, Lift, LiftReturn, Lower, LowerReturn, + ConvertError, FfiConverter, ForeignExecutor, Handle, Lift, LiftReturn, Lower, LowerReturn, MetadataBuffer, Result, RustBuffer, UnexpectedUniFFICallbackError, }; use anyhow::bail; @@ -411,7 +411,7 @@ where /// The foreign bindings may use an actual pointer to the executor object, or a usized integer /// handle. unsafe impl FfiConverter for ForeignExecutor { - type FfiType = crate::ForeignExecutorHandle; + type FfiType = Handle; // Passing these back to the foreign bindings is currently not supported fn lower(executor: Self) -> Self::FfiType { @@ -419,13 +419,7 @@ unsafe impl FfiConverter for ForeignExecutor { } fn write(executor: Self, buf: &mut Vec) { - // Use native endian when writing these values, so they can be casted to pointer values - match std::mem::size_of::() { - // Use native endian when reading these values, so they can be casted to pointer values - 4 => buf.put_u32_ne(executor.handle.0 as u32), - 8 => buf.put_u64_ne(executor.handle.0 as u64), - n => panic!("Invalid usize width: {n}"), - }; + buf.put_i64(executor.handle.as_raw()) } fn try_lift(executor: Self::FfiType) -> Result { @@ -433,13 +427,7 @@ unsafe impl FfiConverter for ForeignExecutor { } fn try_read(buf: &mut &[u8]) -> Result { - let usize_val = match std::mem::size_of::() { - // Use native endian when reading these values, so they can be casted to pointer values - 4 => buf.get_u32_ne() as usize, - 8 => buf.get_u64_ne() as usize, - n => panic!("Invalid usize width: {n}"), - }; - >::try_lift(crate::ForeignExecutorHandle(usize_val as *const ())) + >::try_lift(Handle::from_raw(buf.get_i64())) } const TYPE_ID_META: MetadataBuffer = diff --git a/uniffi_core/src/ffi_converter_traits.rs b/uniffi_core/src/ffi_converter_traits.rs index 3b5914e32f..ed1fadbbb3 100644 --- a/uniffi_core/src/ffi_converter_traits.rs +++ b/uniffi_core/src/ffi_converter_traits.rs @@ -51,7 +51,9 @@ use std::{borrow::Borrow, sync::Arc}; use anyhow::bail; use bytes::Buf; -use crate::{FfiDefault, MetadataBuffer, Result, RustBuffer, UnexpectedUniFFICallbackError}; +use crate::{ + FfiDefault, Handle, MetadataBuffer, Result, RustBuffer, UnexpectedUniFFICallbackError, +}; /// Generalized FFI conversions /// @@ -351,6 +353,24 @@ pub trait ConvertError: Sized { fn try_convert_unexpected_callback_error(e: UnexpectedUniFFICallbackError) -> Result; } +/// Store instances `Arc` in a [crate::Slab] and generate handles to pass to the foreign code +/// +/// ## Safety +/// +/// All traits are unsafe (implementing it requires `unsafe impl`) because we can't guarantee +/// that it's safe to pass your type out to foreign-language code and back again. Buggy +/// implementations of this trait might violate some assumptions made by the generated code, +/// or might not match with the corresponding code in the generated foreign-language bindings. +/// These traits should not be used directly, only in generated code, and the generated code should +/// have fixture tests to test that everything works correctly together. +/// `&T` using the Arc. +pub unsafe trait SlabAlloc { + fn insert(value: Arc) -> Handle; + fn inc_ref(handle: Handle); + fn get_clone(handle: Handle) -> Arc; + fn remove(handle: Handle) -> Arc; +} + /// Derive FFI traits /// /// This can be used to derive: @@ -463,4 +483,30 @@ macro_rules! derive_ffi_traits { } } }; + + (impl $(<$($generic:ident),*>)? $(::uniffi::)? SlabAlloc<$ut:path> for $ty:ty $(where $($where:tt)*)?) => { + // Using an anonymous const here creates a new namespace so that our `SLAB` static doesn't + // conflict with other names. + const _: () = { + $crate::static_slab!(SLAB, ::std::sync::Arc<$ty>); + + unsafe impl $(<$($generic),*>)* $crate::SlabAlloc<$ut> for $ty $(where $($where)*)* + { + fn insert(value: ::std::sync::Arc) -> $crate::Handle { + SLAB.insert_or_panic(value) + } + fn inc_ref(handle: $crate::Handle) { + SLAB.inc_ref_or_panic(handle) + } + + fn get_clone(handle: $crate::Handle) -> ::std::sync::Arc { + SLAB.get_clone_or_panic(handle) + } + + fn remove(handle: $crate::Handle) -> ::std::sync::Arc { + SLAB.remove_or_panic(handle).0 + } + } + }; + }; } diff --git a/uniffi_core/src/lib.rs b/uniffi_core/src/lib.rs index 9003b08f9b..1763b93cec 100644 --- a/uniffi_core/src/lib.rs +++ b/uniffi_core/src/lib.rs @@ -46,6 +46,7 @@ pub mod metadata; pub use ffi::*; pub use ffi_converter_traits::{ ConvertError, FfiConverter, FfiConverterArc, Lift, LiftRef, LiftReturn, Lower, LowerReturn, + SlabAlloc, }; pub use metadata::*; @@ -56,6 +57,7 @@ pub mod deps { #[cfg(feature = "tokio")] pub use async_compat; pub use bytes; + pub use const_random; pub use log; pub use static_assertions; } diff --git a/uniffi_macros/src/export.rs b/uniffi_macros/src/export.rs index 7d3f54da93..364e54470d 100644 --- a/uniffi_macros/src/export.rs +++ b/uniffi_macros/src/export.rs @@ -22,6 +22,7 @@ use self::{ use crate::util::{ident_to_string, mod_path}; pub use attributes::ExportAttributeArguments; pub use callback_interface::ffi_converter_callback_interface_impl; +pub use trait_interface::alter_trait; // TODO(jplatte): Ensure no generics, … // TODO(jplatte): Aggregate errors instead of short-circuiting, wherever possible @@ -76,7 +77,7 @@ pub(crate) fn expand_export( } => { let trait_name = ident_to_string(&self_ident); let trait_impl_ident = callback_interface::trait_impl_ident(&trait_name); - let trait_impl = callback_interface::trait_impl(&mod_path, &self_ident, &items) + let trait_impl = callback_interface::trait_impl(&mod_path, &self_ident, &items, false) .unwrap_or_else(|e| e.into_compile_error()); let metadata_items = callback_interface::metadata_items(&self_ident, &items, &mod_path) .unwrap_or_else(|e| vec![e.into_compile_error()]); @@ -101,6 +102,14 @@ pub(crate) fn expand_export( } } +/// Alter the tokens wrapped with the `[uniffi::export]` if needed +pub fn alter_input(item: &Item) -> TokenStream { + match item { + Item::Trait(item_trait) => alter_trait(item_trait), + _ => quote! { #item }, + } +} + /// Rewrite Self type alias usage in an impl block to the type itself. /// /// For example, diff --git a/uniffi_macros/src/export/callback_interface.rs b/uniffi_macros/src/export/callback_interface.rs index 81258f0bb7..77538a1f92 100644 --- a/uniffi_macros/src/export/callback_interface.rs +++ b/uniffi_macros/src/export/callback_interface.rs @@ -16,6 +16,7 @@ pub(super) fn trait_impl( mod_path: &str, trait_ident: &Ident, items: &[ImplItem], + trait_interface: bool, ) -> syn::Result { let trait_name = ident_to_string(trait_ident); let trait_impl_ident = trait_impl_ident(&trait_name); @@ -32,6 +33,18 @@ pub(super) fn trait_impl( _ => unreachable!("traits have no constructors"), }) .collect::>()?; + + let uniffi_foreign_handle = trait_interface.then(|| { + quote! { + fn uniffi_foreign_handle(&self) -> Option<::uniffi::Handle> { + #internals_ident.invoke_callback::<(), crate::UniFfiTag>( + self.handle, uniffi::IDX_CALLBACK_INC_REF, Default::default() + ); + Some(self.handle) + } + } + }); + Ok(quote! { #[doc(hidden)] static #internals_ident: ::uniffi::ForeignCallbackInternals = ::uniffi::ForeignCallbackInternals::new(); @@ -45,11 +58,11 @@ pub(super) fn trait_impl( #[doc(hidden)] #[derive(Debug)] struct #trait_impl_ident { - handle: u64, + handle: ::uniffi::Handle, } impl #trait_impl_ident { - fn new(handle: u64) -> Self { + fn new(handle: ::uniffi::Handle) -> Self { Self { handle } } } @@ -66,6 +79,7 @@ pub(super) fn trait_impl( impl #trait_ident for #trait_impl_ident { #trait_impl_methods + #uniffi_foreign_handle } }) } @@ -106,16 +120,16 @@ pub fn ffi_converter_callback_interface_impl( #[doc(hidden)] #[automatically_derived] unsafe #lift_impl_spec { - type FfiType = u64; + type FfiType = i64; fn try_lift(v: Self::FfiType) -> ::uniffi::deps::anyhow::Result { - Ok(::std::boxed::Box::new(<#trait_impl_ident>::new(v))) + Ok(::std::boxed::Box::new(<#trait_impl_ident>::new(::uniffi::Handle::from_raw(v)))) } fn try_read(buf: &mut &[u8]) -> ::uniffi::deps::anyhow::Result { use uniffi::deps::bytes::Buf; ::uniffi::check_remaining(buf, 8)?; - >::try_lift(buf.get_u64()) + >::try_lift(buf.get_i64()) } const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code( diff --git a/uniffi_macros/src/export/scaffolding.rs b/uniffi_macros/src/export/scaffolding.rs index d00d8403bd..cb333aac98 100644 --- a/uniffi_macros/src/export/scaffolding.rs +++ b/uniffi_macros/src/export/scaffolding.rs @@ -137,23 +137,8 @@ impl ScaffoldingBits { let params: Vec<_> = iter::once(quote! { uniffi_self_lowered: #lift_impl::FfiType }) .chain(sig.scaffolding_params()) .collect(); - let try_lift_self = if is_trait { - // For trait interfaces we need to special case this. Trait interfaces normally lift - // foreign trait impl pointers. However, for a method call, we want to lift a Rust - // pointer. - quote! { - { - let foreign_arc = ::std::boxed::Box::leak(unsafe { Box::from_raw(uniffi_self_lowered as *mut ::std::sync::Arc) }); - // Take a clone for our own use. - Ok(::std::sync::Arc::clone(foreign_arc)) - } - } - } else { - quote! { #lift_impl::try_lift(uniffi_self_lowered) } - }; - let lift_closure = sig.lift_closure(Some(quote! { - match #try_lift_self { + match #lift_impl::try_lift(uniffi_self_lowered) { Ok(v) => v, Err(e) => return Err(("self", e)) } @@ -264,7 +249,7 @@ pub(super) fn gen_ffi_function( quote! { #[doc(hidden)] #[no_mangle] - pub extern "C" fn #ffi_ident(#(#params,)*) -> ::uniffi::RustFutureHandle { + pub extern "C" fn #ffi_ident(#(#params,)*) -> ::uniffi::Handle { ::uniffi::deps::log::debug!(#name); let uniffi_lift_args = #lift_closure; match uniffi_lift_args() { diff --git a/uniffi_macros/src/export/trait_interface.rs b/uniffi_macros/src/export/trait_interface.rs index c4755015dc..1ad1ae4f40 100644 --- a/uniffi_macros/src/export/trait_interface.rs +++ b/uniffi_macros/src/export/trait_interface.rs @@ -4,6 +4,7 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{quote, quote_spanned}; +use syn::ItemTrait; use crate::{ export::{ @@ -11,9 +12,8 @@ use crate::{ item::ImplItem, }, object::interface_meta_static_var, - util::{ident_to_string, tagged_impl_header}, + util::{derive_ffi_traits, ident_to_string, tagged_impl_header}, }; -use uniffi_meta::free_fn_symbol_name; pub(super) fn gen_trait_scaffolding( mod_path: &str, @@ -26,24 +26,38 @@ pub(super) fn gen_trait_scaffolding( return Err(syn::Error::new_spanned(rt, "not supported for traits")); } let trait_name = ident_to_string(&self_ident); - let trait_impl = callback_interface::trait_impl(mod_path, &self_ident, &items) + let trait_impl = callback_interface::trait_impl(mod_path, &self_ident, &items, true) .unwrap_or_else(|e| e.into_compile_error()); - + let inc_ref_fn_ident = Ident::new( + &uniffi_meta::inc_ref_fn_symbol_name(mod_path, &trait_name), + Span::call_site(), + ); let free_fn_ident = Ident::new( - &free_fn_symbol_name(mod_path, &trait_name), + &uniffi_meta::free_fn_symbol_name(mod_path, &trait_name), Span::call_site(), ); - let free_tokens = quote! { + let helper_ffi_fn_tokens = quote! { + #[doc(hidden)] + #[no_mangle] + pub extern "C" fn #inc_ref_fn_ident( + handle: ::uniffi::Handle, + call_status: &mut ::uniffi::RustCallStatus + ) { + uniffi::rust_call(call_status, || { + >::inc_ref(handle); + Ok(()) + }); + } + #[doc(hidden)] #[no_mangle] pub extern "C" fn #free_fn_ident( - ptr: *const ::std::ffi::c_void, + handle: ::uniffi::Handle, call_status: &mut ::uniffi::RustCallStatus ) { uniffi::rust_call(call_status, || { - assert!(!ptr.is_null()); - drop(unsafe { ::std::boxed::Box::from_raw(ptr as *mut std::sync::Arc) }); + >::remove(handle); Ok(()) }); } @@ -73,7 +87,7 @@ pub(super) fn gen_trait_scaffolding( Ok(quote_spanned! { self_ident.span() => #meta_static_var - #free_tokens + #helper_ffi_fn_tokens #trait_impl #impl_tokens #ffi_converter_tokens @@ -83,6 +97,8 @@ pub(super) fn gen_trait_scaffolding( pub(crate) fn ffi_converter(mod_path: &str, trait_ident: &Ident, udl_mode: bool) -> TokenStream { let impl_spec = tagged_impl_header("FfiConverterArc", "e! { dyn #trait_ident }, udl_mode); let lift_ref_impl_spec = tagged_impl_header("LiftRef", "e! { dyn #trait_ident }, udl_mode); + let derive_ffi_traits = + derive_ffi_traits("e! { dyn #trait_ident }, udl_mode, &["SlabAlloc"]); let trait_name = ident_to_string(trait_ident); let trait_impl_ident = callback_interface::trait_impl_ident(&trait_name); @@ -93,30 +109,45 @@ pub(crate) fn ffi_converter(mod_path: &str, trait_ident: &Ident, udl_mode: bool) // and thus help the user debug why the requirement isn't being met. uniffi::deps::static_assertions::assert_impl_all!(dyn #trait_ident: Sync, Send); + #derive_ffi_traits + unsafe #impl_spec { - type FfiType = *const ::std::os::raw::c_void; + type FfiType = ::uniffi::Handle; + + fn lower(obj: ::std::sync::Arc) -> ::uniffi::Handle { + // If obj wraps a foreign implementation, then `uniffi_foreign_handle` will return + // the handle here and we can use that rather than wrapping it again with Rust. + let handle = match obj.uniffi_foreign_handle() { + Some(handle) => handle, + None => >::insert(obj), + }; + handle - fn lower(obj: ::std::sync::Arc) -> Self::FfiType { - ::std::boxed::Box::into_raw(::std::boxed::Box::new(obj)) as *const ::std::os::raw::c_void } - fn try_lift(v: Self::FfiType) -> ::uniffi::deps::anyhow::Result<::std::sync::Arc> { - Ok(::std::sync::Arc::new(<#trait_impl_ident>::new(v as u64))) + fn try_lift(handle: ::uniffi::Handle) -> ::uniffi::deps::anyhow::Result<::std::sync::Arc> { + Ok(if handle.is_foreign() { + // For foreign handles, construct a struct that implements the trait by calling + // the handle + ::std::sync::Arc::new(<#trait_impl_ident>::new(handle)) + } else { + // For Rust handles, remove the `Arc<>` from our slab. + >::remove(handle) + }) } fn write(obj: ::std::sync::Arc, buf: &mut Vec) { - ::uniffi::deps::static_assertions::const_assert!(::std::mem::size_of::<*const ::std::ffi::c_void>() <= 8); - ::uniffi::deps::bytes::BufMut::put_u64( + ::uniffi::deps::bytes::BufMut::put_i64( buf, - >::lower(obj) as u64, + >::lower(obj).as_raw(), ); } fn try_read(buf: &mut &[u8]) -> ::uniffi::Result<::std::sync::Arc> { - ::uniffi::deps::static_assertions::const_assert!(::std::mem::size_of::<*const ::std::ffi::c_void>() <= 8); ::uniffi::check_remaining(buf, 8)?; >::try_lift( - ::uniffi::deps::bytes::Buf::get_u64(buf) as Self::FfiType) + ::uniffi::Handle::from_raw(::uniffi::deps::bytes::Buf::get_i64(buf)) + ) } const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_INTERFACE) @@ -130,3 +161,31 @@ pub(crate) fn ffi_converter(mod_path: &str, trait_ident: &Ident, udl_mode: bool) } } } + +pub fn alter_trait(item: &ItemTrait) -> TokenStream { + let ItemTrait { + attrs, + vis, + unsafety, + auto_token, + trait_token, + ident, + generics, + colon_token, + supertraits, + items, + .. + } = item; + + quote! { + #(#attrs)* + #vis #unsafety #auto_token #trait_token #ident #generics #colon_token #supertraits { + #(#items)* + + #[doc(hidden)] + fn uniffi_foreign_handle(&self) -> Option<::uniffi::Handle> { + None + } + } + } +} diff --git a/uniffi_macros/src/lib.rs b/uniffi_macros/src/lib.rs index b7ba86ddc1..50520186f4 100644 --- a/uniffi_macros/src/lib.rs +++ b/uniffi_macros/src/lib.rs @@ -104,20 +104,28 @@ pub fn export(attr_args: TokenStream, input: TokenStream) -> TokenStream { } fn do_export(attr_args: TokenStream, input: TokenStream, udl_mode: bool) -> TokenStream { - let copied_input = (!udl_mode).then(|| proc_macro2::TokenStream::from(input.clone())); - let gen_output = || { let args = syn::parse(attr_args)?; let item = syn::parse(input)?; - expand_export(item, args, udl_mode) + let altered_input = if udl_mode { + quote! {} + } else { + export::alter_input(&item) + }; + let output = expand_export(item, args, udl_mode)?; + Ok(quote! { + #altered_input + #output + }) }; - let output = gen_output().unwrap_or_else(syn::Error::into_compile_error); + gen_output() + .unwrap_or_else(syn::Error::into_compile_error) + .into() +} - quote! { - #copied_input - #output - } - .into() +#[proc_macro_attribute] +pub fn trait_interface(_attr_args: TokenStream, input: TokenStream) -> TokenStream { + export::alter_trait(&parse_macro_input!(input)).into() } #[proc_macro_derive(Record, attributes(uniffi))] diff --git a/uniffi_macros/src/object.rs b/uniffi_macros/src/object.rs index 573a1eaadd..71c73f6fa3 100644 --- a/uniffi_macros/src/object.rs +++ b/uniffi_macros/src/object.rs @@ -1,15 +1,23 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::quote; use syn::DeriveInput; -use uniffi_meta::free_fn_symbol_name; -use crate::util::{create_metadata_items, ident_to_string, mod_path, tagged_impl_header}; +use crate::util::{ + create_metadata_items, derive_ffi_traits, ident_to_string, mod_path, tagged_impl_header, +}; pub fn expand_object(input: DeriveInput, udl_mode: bool) -> syn::Result { let module_path = mod_path()?; let ident = &input.ident; let name = ident_to_string(ident); - let free_fn_ident = Ident::new(&free_fn_symbol_name(&module_path, &name), Span::call_site()); + let inc_ref_fn_ident = Ident::new( + &uniffi_meta::inc_ref_fn_symbol_name(&module_path, &name), + Span::call_site(), + ); + let free_fn_ident = Ident::new( + &uniffi_meta::free_fn_symbol_name(&module_path, &name), + Span::call_site(), + ); let meta_static_var = (!udl_mode).then(|| { interface_meta_static_var(ident, false, &module_path) .unwrap_or_else(syn::Error::into_compile_error) @@ -17,18 +25,26 @@ pub fn expand_object(input: DeriveInput, udl_mode: bool) -> syn::Result>::inc_ref(handle); + Ok(()) + }); + } + #[doc(hidden)] #[no_mangle] pub extern "C" fn #free_fn_ident( - ptr: *const ::std::ffi::c_void, + handle: ::uniffi::Handle, call_status: &mut ::uniffi::RustCallStatus ) { uniffi::rust_call(call_status, || { - assert!(!ptr.is_null()); - let ptr = ptr.cast::<#ident>(); - unsafe { - ::std::sync::Arc::decrement_strong_count(ptr); - } + <#ident as ::uniffi::SlabAlloc>::remove(handle); Ok(()) }); } @@ -42,6 +58,7 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { let name = ident_to_string(ident); let impl_spec = tagged_impl_header("FfiConverterArc", ident, udl_mode); let lift_ref_impl_spec = tagged_impl_header("LiftRef", ident, udl_mode); + let derive_ffi_traits = derive_ffi_traits(ident, udl_mode, &["SlabAlloc"]); let mod_path = match mod_path() { Ok(p) => p, Err(e) => return e.into_compile_error(), @@ -54,6 +71,8 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { // and thus help the user debug why the requirement isn't being met. uniffi::deps::static_assertions::assert_impl_all!(#ident: Sync, Send); + #derive_ffi_traits + #[doc(hidden)] #[automatically_derived] /// Support for passing reference-counted shared objects via the FFI. @@ -62,8 +81,7 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { /// by reference must be encapsulated in an `Arc`, and must be safe to share /// across threads. unsafe #impl_spec { - // Don't use a pointer to as that requires a `pub ` - type FfiType = *const ::std::os::raw::c_void; + type FfiType = ::uniffi::Handle; /// 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 @@ -75,7 +93,7 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { /// call the destructor function specific to the type `T`. Calling the destructor /// function for other types may lead to undefined behaviour. fn lower(obj: ::std::sync::Arc) -> Self::FfiType { - ::std::sync::Arc::into_raw(obj) as Self::FfiType + <#ident as ::uniffi::SlabAlloc>::insert(obj) } /// When lifting, we receive a "borrow" of the `Arc` that is owned by @@ -84,11 +102,7 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { /// 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) -> ::uniffi::Result<::std::sync::Arc> { - let v = v as *const #ident; - // We musn't drop the `Arc` that is owned by the foreign-language code. - let foreign_arc = ::std::mem::ManuallyDrop::new(unsafe { ::std::sync::Arc::::from_raw(v) }); - // Take a clone for our own use. - Ok(::std::sync::Arc::clone(&*foreign_arc)) + Ok(<#ident as ::uniffi::SlabAlloc>::remove(v)) } /// When writing as a field of a complex structure, make a clone and transfer ownership @@ -100,8 +114,7 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { /// call the destructor function specific to the type `T`. Calling the destructor /// function for other types may lead to undefined behaviour. fn write(obj: ::std::sync::Arc, buf: &mut Vec) { - ::uniffi::deps::static_assertions::const_assert!(::std::mem::size_of::<*const ::std::ffi::c_void>() <= 8); - ::uniffi::deps::bytes::BufMut::put_u64(buf, >::lower(obj) as u64); + ::uniffi::deps::bytes::BufMut::put_i64(buf, >::lower(obj).as_raw()) } /// When reading as a field of a complex structure, we receive a "borrow" of the `Arc` @@ -110,9 +123,10 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { /// Safety: the buffer must contain a pointer previously obtained by calling /// the `lower()` or `write()` method of this impl. fn try_read(buf: &mut &[u8]) -> ::uniffi::Result<::std::sync::Arc> { - ::uniffi::deps::static_assertions::const_assert!(::std::mem::size_of::<*const ::std::ffi::c_void>() <= 8); ::uniffi::check_remaining(buf, 8)?; - >::try_lift(::uniffi::deps::bytes::Buf::get_u64(buf) as Self::FfiType) + >::try_lift( + ::uniffi::Handle::from_raw(::uniffi::deps::bytes::Buf::get_i64(buf)) + ) } const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_INTERFACE) diff --git a/uniffi_macros/src/setup_scaffolding.rs b/uniffi_macros/src/setup_scaffolding.rs index a21016e9ff..3c34a1127f 100644 --- a/uniffi_macros/src/setup_scaffolding.rs +++ b/uniffi_macros/src/setup_scaffolding.rs @@ -19,6 +19,12 @@ pub fn setup_scaffolding(namespace: String) -> Result { let ffi_rustbuffer_from_bytes_ident = format_ident!("ffi_{module_path}_rustbuffer_from_bytes"); let ffi_rustbuffer_free_ident = format_ident!("ffi_{module_path}_rustbuffer_free"); let ffi_rustbuffer_reserve_ident = format_ident!("ffi_{module_path}_rustbuffer_reserve"); + let ffi_slab_new = format_ident!("ffi_{module_path}_slab_new"); + let ffi_slab_free = format_ident!("ffi_{module_path}_slab_free"); + let ffi_slab_insert = format_ident!("ffi_{module_path}_slab_insert"); + let ffi_slab_check_handle = format_ident!("ffi_{module_path}_slab_check_handle"); + let ffi_slab_inc_ref = format_ident!("ffi_{module_path}_slab_inc_ref"); + let ffi_slab_dec_ref = format_ident!("ffi_{module_path}_slab_dec_ref"); let reexport_hack_ident = format_ident!("{module_path}_uniffi_reexport_hack"); let ffi_foreign_executor_callback_set_ident = format_ident!("ffi_{module_path}_foreign_executor_callback_set"); @@ -62,36 +68,78 @@ pub fn setup_scaffolding(namespace: String) -> Result { #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub extern "C" fn #ffi_rustbuffer_alloc_ident(size: i32, call_status: &mut uniffi::RustCallStatus) -> uniffi::RustBuffer { - uniffi::ffi::uniffi_rustbuffer_alloc(size, call_status) + pub extern "C" fn #ffi_rustbuffer_alloc_ident(size: i32, call_status: &mut ::uniffi::RustCallStatus) -> ::uniffi::RustBuffer { + ::uniffi::ffi::uniffi_rustbuffer_alloc(size, call_status) } #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub unsafe extern "C" fn #ffi_rustbuffer_from_bytes_ident(bytes: uniffi::ForeignBytes, call_status: &mut uniffi::RustCallStatus) -> uniffi::RustBuffer { - uniffi::ffi::uniffi_rustbuffer_from_bytes(bytes, call_status) + pub unsafe extern "C" fn #ffi_rustbuffer_from_bytes_ident(bytes: ::uniffi::ForeignBytes, call_status: &mut ::uniffi::RustCallStatus) -> ::uniffi::RustBuffer { + ::uniffi::ffi::uniffi_rustbuffer_from_bytes(bytes, call_status) } #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub unsafe extern "C" fn #ffi_rustbuffer_free_ident(buf: uniffi::RustBuffer, call_status: &mut uniffi::RustCallStatus) { - uniffi::ffi::uniffi_rustbuffer_free(buf, call_status); + pub unsafe extern "C" fn #ffi_rustbuffer_free_ident(buf: ::uniffi::RustBuffer, call_status: &mut ::uniffi::RustCallStatus) { + ::uniffi::ffi::uniffi_rustbuffer_free(buf, call_status); } #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub unsafe extern "C" fn #ffi_rustbuffer_reserve_ident(buf: uniffi::RustBuffer, additional: i32, call_status: &mut uniffi::RustCallStatus) -> uniffi::RustBuffer { - uniffi::ffi::uniffi_rustbuffer_reserve(buf, additional, call_status) + pub unsafe extern "C" fn #ffi_rustbuffer_reserve_ident(buf: ::uniffi::RustBuffer, additional: i32, call_status: &mut ::uniffi::RustCallStatus) -> ::uniffi::RustBuffer { + ::uniffi::ffi::uniffi_rustbuffer_reserve(buf, additional, call_status) } #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub extern "C" fn #ffi_foreign_executor_callback_set_ident(callback: uniffi::ffi::ForeignExecutorCallback) { - uniffi::ffi::foreign_executor_callback_set(callback) + pub unsafe extern "C" fn #ffi_slab_new() -> ::uniffi::Handle { + ::uniffi::uniffi_slab_new() + } + + #[allow(clippy::missing_safety_doc, missing_docs)] + #[doc(hidden)] + #[no_mangle] + pub unsafe extern "C" fn #ffi_slab_free(handle: ::uniffi::Handle) { + ::uniffi::uniffi_slab_free(handle) + } + + #[allow(clippy::missing_safety_doc, missing_docs)] + #[doc(hidden)] + #[no_mangle] + pub unsafe extern "C" fn #ffi_slab_insert(slab: ::uniffi::Handle) -> i64 { + ::uniffi::uniffi_slab_insert(slab) + } + + #[allow(clippy::missing_safety_doc, missing_docs)] + #[doc(hidden)] + #[no_mangle] + pub unsafe extern "C" fn #ffi_slab_check_handle(slab: ::uniffi::Handle, handle: ::uniffi::Handle) -> i8 { + ::uniffi::uniffi_slab_check_handle(slab, handle) + } + + #[allow(clippy::missing_safety_doc, missing_docs)] + #[doc(hidden)] + #[no_mangle] + pub unsafe extern "C" fn #ffi_slab_inc_ref(slab: ::uniffi::Handle, handle: ::uniffi::Handle) -> i8 { + ::uniffi::uniffi_slab_inc_ref(slab, handle) + } + + #[allow(clippy::missing_safety_doc, missing_docs)] + #[doc(hidden)] + #[no_mangle] + pub unsafe extern "C" fn #ffi_slab_dec_ref(slab: ::uniffi::Handle, handle: ::uniffi::Handle) -> i8 { + ::uniffi::uniffi_slab_dec_ref(slab, handle) + } + + #[allow(clippy::missing_safety_doc, missing_docs)] + #[doc(hidden)] + #[no_mangle] + pub extern "C" fn #ffi_foreign_executor_callback_set_ident(callback: ::uniffi::ffi::ForeignExecutorCallback) { + ::uniffi::ffi::foreign_executor_callback_set(callback) } #ffi_rust_future_scaffolding_fns @@ -130,7 +178,7 @@ pub fn setup_scaffolding(namespace: String) -> Result { #[doc(hidden)] pub trait UniffiCustomTypeConverter { type Builtin; - fn into_custom(val: Self::Builtin) -> uniffi::Result where Self: Sized; + fn into_custom(val: Self::Builtin) -> ::uniffi::Result where Self: Sized; fn from_custom(obj: Self) -> Self::Builtin; } }) @@ -138,12 +186,12 @@ pub fn setup_scaffolding(namespace: String) -> Result { /// Generates the rust_future_* functions /// -/// The foreign side uses a type-erased `RustFutureHandle` to interact with futures, which presents +/// The foreign side uses a type-erased `Handle` to interact with futures, which presents /// a problem when creating scaffolding functions. What is the `ReturnType` parameter of `RustFutureFfi`? /// /// Handle this by using some brute-force monomorphization. For each possible ffi type, we /// generate a set of scaffolding functions. The bindings code is responsible for calling the one -/// corresponds the scaffolding function that created the `RustFutureHandle`. +/// corresponds the scaffolding function that created the `Handle`. /// /// This introduces safety issues, but we do get some type checking. If the bindings code calls /// the wrong rust_future_complete function, they should get an unexpected return type, which @@ -160,7 +208,7 @@ fn rust_future_scaffolding_fns(module_path: &str) -> TokenStream { (quote! { i64 }, "i64"), (quote! { f32 }, "f32"), (quote! { f64 }, "f64"), - (quote! { *const ::std::ffi::c_void }, "pointer"), + (quote! { ::uniffi::Handle }, "handle"), (quote! { ::uniffi::RustBuffer }, "rust_buffer"), (quote! { () }, "void"), ]; @@ -175,32 +223,32 @@ fn rust_future_scaffolding_fns(module_path: &str) -> TokenStream { #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub unsafe extern "C" fn #ffi_rust_future_poll(handle: ::uniffi::RustFutureHandle, callback: ::uniffi::RustFutureContinuationCallback, data: *const ()) { - ::uniffi::ffi::rust_future_poll::<#return_type>(handle, callback, data); + pub unsafe extern "C" fn #ffi_rust_future_poll(handle: ::uniffi::Handle, callback: ::uniffi::RustFutureContinuationCallback, data: ::uniffi::Handle) { + ::uniffi::ffi::rust_future_poll::<#return_type, crate::UniFfiTag>(handle, callback, data); } #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub unsafe extern "C" fn #ffi_rust_future_cancel(handle: ::uniffi::RustFutureHandle) { - ::uniffi::ffi::rust_future_cancel::<#return_type>(handle) + pub unsafe extern "C" fn #ffi_rust_future_cancel(handle: ::uniffi::Handle) { + ::uniffi::ffi::rust_future_cancel::<#return_type, crate::UniFfiTag>(handle) } #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] pub unsafe extern "C" fn #ffi_rust_future_complete( - handle: ::uniffi::RustFutureHandle, + handle: ::uniffi::Handle, out_status: &mut ::uniffi::RustCallStatus ) -> #return_type { - ::uniffi::ffi::rust_future_complete::<#return_type>(handle, out_status) + ::uniffi::ffi::rust_future_complete::<#return_type, crate::UniFfiTag>(handle, out_status) } #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub unsafe extern "C" fn #ffi_rust_future_free(handle: ::uniffi::RustFutureHandle) { - ::uniffi::ffi::rust_future_free::<#return_type>(handle) + pub unsafe extern "C" fn #ffi_rust_future_free(handle: ::uniffi::Handle) { + ::uniffi::ffi::rust_future_free::<#return_type, crate::UniFfiTag>(handle) } } }) diff --git a/uniffi_macros/src/util.rs b/uniffi_macros/src/util.rs index 9f213ea1d7..9cfd554491 100644 --- a/uniffi_macros/src/util.rs +++ b/uniffi_macros/src/util.rs @@ -216,7 +216,7 @@ pub(crate) fn tagged_impl_header( } } -pub(crate) fn derive_all_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream { +pub(crate) fn derive_all_ffi_traits(ty: impl ToTokens, udl_mode: bool) -> TokenStream { if udl_mode { quote! { ::uniffi::derive_ffi_traits!(local #ty); } } else { @@ -224,7 +224,11 @@ pub(crate) fn derive_all_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream { } } -pub(crate) fn derive_ffi_traits(ty: &Ident, udl_mode: bool, trait_names: &[&str]) -> TokenStream { +pub(crate) fn derive_ffi_traits( + ty: impl ToTokens, + udl_mode: bool, + trait_names: &[&str], +) -> TokenStream { let trait_idents = trait_names .iter() .map(|name| Ident::new(name, Span::call_site())); diff --git a/uniffi_meta/src/ffi_names.rs b/uniffi_meta/src/ffi_names.rs index 44a5bc3e63..1e2e423d6c 100644 --- a/uniffi_meta/src/ffi_names.rs +++ b/uniffi_meta/src/ffi_names.rs @@ -39,6 +39,12 @@ pub fn free_fn_symbol_name(namespace: &str, object_name: &str) -> String { format!("uniffi_{namespace}_fn_free_{object_name}") } +/// FFI symbol name for the `inc_ref` function for an object. +pub fn inc_ref_fn_symbol_name(namespace: &str, object_name: &str) -> String { + let object_name = object_name.to_ascii_lowercase(); + format!("uniffi_{namespace}_fn_inc_ref_{object_name}") +} + /// FFI symbol name for the `init_callback` function for a callback interface pub fn init_callback_fn_symbol_name(namespace: &str, callback_interface_name: &str) -> String { let callback_interface_name = callback_interface_name.to_ascii_lowercase(); diff --git a/uniffi_meta/src/lib.rs b/uniffi_meta/src/lib.rs index 1c8a66801c..60cbf230b6 100644 --- a/uniffi_meta/src/lib.rs +++ b/uniffi_meta/src/lib.rs @@ -319,6 +319,14 @@ pub struct CallbackInterfaceMetadata { } impl ObjectMetadata { + /// FFI symbol name for the `inc_ref` function for this object. + /// + /// This function is used to increment the reference count before lowering an object to pass + /// back to Rust. + pub fn inc_ref_ffi_symbol_name(&self) -> String { + inc_ref_fn_symbol_name(&self.module_path, &self.name) + } + /// FFI symbol name for the `free` function for this object. /// /// This function is used to free the memory used by this object.