From 04c22b9a2a76fe92e19ac3d17e12bf7a9d22b1bf Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Fri, 7 Apr 2023 15:33:36 -0400 Subject: [PATCH 1/5] Adding the `foreignexecutor` module This is the first step in an overall async plan layed out in PR #1494. This allows Rust calls to be scheduled using the foreign executor (AKA task queue, event loop, etc.). It's not yet documented in the manual, I plan to do that when I implement async callbacks. I hope to use this to simplify the `RustFuture` code. Issue 1507 makes me think that we don't want the foreign bindings to handle ownership of the future, it's very tricky to manage this across an FFI. Instead, let's have Rust manage the future and have the foreign bindings only handle the scheduling. Added support for `usize' ints. This lets foreign bindings choose between using an int or pointer to represent an opaque type. Kotlin: moved the async imports to Types.rs, so that we can use `add_import()` to handle them. The ForeignExecutor.kt template wants to import some of the same items. Swift: Added support for initialization functions --- Cargo.toml | 1 + fixtures/foreign-executor/Cargo.toml | 19 + fixtures/foreign-executor/build.rs | 7 + .../foreign-executor/src/foreign_executor.udl | 7 + fixtures/foreign-executor/src/lib.rs | 70 +++ .../tests/bindings/test_foreign_executor.kts | 47 ++ .../tests/bindings/test_foreign_executor.py | 50 ++ .../bindings/test_foreign_executor.swift | 42 ++ .../tests/test_generated_bindings.rs | 5 + fixtures/metadata/src/tests.rs | 1 + .../bindings/kotlin/gen_kotlin/executor.rs | 22 + .../src/bindings/kotlin/gen_kotlin/mod.rs | 5 + .../templates/ForeignExecutorTemplate.kt | 60 +++ .../src/bindings/kotlin/templates/Helpers.kt | 80 ++++ .../src/bindings/kotlin/templates/Types.kt | 17 + .../src/bindings/kotlin/templates/wrapper.kt | 12 +- .../bindings/python/gen_python/executor.rs | 21 + .../src/bindings/python/gen_python/mod.rs | 6 +- .../templates/ForeignExecutorTemplate.py | 42 ++ .../templates/NamespaceLibraryTemplate.py | 32 +- .../python/templates/PointerManager.py | 65 +++ .../python/templates/RustBufferTemplate.py | 5 + .../src/bindings/python/templates/Types.py | 3 + .../src/bindings/python/templates/macros.py | 2 +- .../src/bindings/python/templates/wrapper.py | 2 + .../src/bindings/ruby/gen_ruby/mod.rs | 9 + .../src/bindings/swift/gen_swift/executor.rs | 22 + .../src/bindings/swift/gen_swift/mod.rs | 11 +- .../swift/templates/BridgingHeaderTemplate.h | 12 + .../templates/ForeignExecutorTemplate.swift | 52 +++ .../bindings/swift/templates/Helpers.swift | 2 +- .../src/bindings/swift/templates/Types.swift | 3 + .../bindings/swift/templates/wrapper.swift | 19 +- uniffi_bindgen/src/interface/ffi.rs | 8 +- uniffi_bindgen/src/interface/mod.rs | 25 +- uniffi_bindgen/src/interface/types/mod.rs | 8 + .../src/interface/types/resolver.rs | 1 + uniffi_bindgen/src/scaffolding/mod.rs | 9 +- uniffi_core/src/ffi/ffidefault.rs | 6 + uniffi_core/src/ffi/foreignexecutor.rs | 435 ++++++++++++++++++ uniffi_core/src/ffi/mod.rs | 2 + uniffi_core/src/ffi_converter_impls.rs | 43 ++ uniffi_core/src/metadata.rs | 1 + uniffi_meta/src/lib.rs | 1 + uniffi_meta/src/reader.rs | 3 +- 45 files changed, 1258 insertions(+), 37 deletions(-) create mode 100644 fixtures/foreign-executor/Cargo.toml create mode 100644 fixtures/foreign-executor/build.rs create mode 100644 fixtures/foreign-executor/src/foreign_executor.udl create mode 100644 fixtures/foreign-executor/src/lib.rs create mode 100644 fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts create mode 100644 fixtures/foreign-executor/tests/bindings/test_foreign_executor.py create mode 100644 fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift create mode 100644 fixtures/foreign-executor/tests/test_generated_bindings.rs create mode 100644 uniffi_bindgen/src/bindings/kotlin/gen_kotlin/executor.rs create mode 100644 uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt create mode 100644 uniffi_bindgen/src/bindings/python/gen_python/executor.rs create mode 100644 uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py create mode 100644 uniffi_bindgen/src/bindings/python/templates/PointerManager.py create mode 100644 uniffi_bindgen/src/bindings/swift/gen_swift/executor.rs create mode 100644 uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift create mode 100644 uniffi_core/src/ffi/foreignexecutor.rs diff --git a/Cargo.toml b/Cargo.toml index 1f2c85e9ac..10da101131 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ members = [ "fixtures/external-types/crate-two", "fixtures/external-types/lib", + "fixtures/foreign-executor", "fixtures/keywords/kotlin", "fixtures/keywords/rust", "fixtures/keywords/swift", diff --git a/fixtures/foreign-executor/Cargo.toml b/fixtures/foreign-executor/Cargo.toml new file mode 100644 index 0000000000..6ee6bc02ca --- /dev/null +++ b/fixtures/foreign-executor/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "uniffi-fixture-foreign-executor" +version = "0.23.0" +edition = "2021" +license = "MPL-2.0" +publish = false + +[lib] +crate-type = ["lib", "cdylib"] +name = "uniffi_fixture_foreign_executor" + +[dependencies] +uniffi = {path = "../../uniffi"} + +[build-dependencies] +uniffi = {path = "../../uniffi", features = ["build"] } + +[dev-dependencies] +uniffi = {path = "../../uniffi", features = ["bindgen-tests"] } diff --git a/fixtures/foreign-executor/build.rs b/fixtures/foreign-executor/build.rs new file mode 100644 index 0000000000..3998c767e1 --- /dev/null +++ b/fixtures/foreign-executor/build.rs @@ -0,0 +1,7 @@ +/* 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/. */ + +fn main() { + uniffi::generate_scaffolding("./src/foreign_executor.udl").unwrap(); +} diff --git a/fixtures/foreign-executor/src/foreign_executor.udl b/fixtures/foreign-executor/src/foreign_executor.udl new file mode 100644 index 0000000000..2933e56c6a --- /dev/null +++ b/fixtures/foreign-executor/src/foreign_executor.udl @@ -0,0 +1,7 @@ +namespace fixture_foreign_executor { }; + +interface ForeignExecutorTester { + constructor(ForeignExecutor executor); + [Name=new_from_sequence] + constructor(sequence executors); +}; diff --git a/fixtures/foreign-executor/src/lib.rs b/fixtures/foreign-executor/src/lib.rs new file mode 100644 index 0000000000..8e31946217 --- /dev/null +++ b/fixtures/foreign-executor/src/lib.rs @@ -0,0 +1,70 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use std::sync::{Arc, Mutex}; +use std::thread; +use std::time::Instant; +use uniffi::ForeignExecutor; + +pub struct ForeignExecutorTester { + executor: ForeignExecutor, + last_result: Arc>>, +} + +// All constructor have to be defined in UDL for now +impl ForeignExecutorTester { + fn new(executor: ForeignExecutor) -> Self { + Self { + executor, + last_result: Arc::new(Mutex::new(None)), + } + } + + // Test inputting the ForeignExecutor from a Vec. This tests that they can be written to a + // `RustBuffer` + fn new_from_sequence(executors: Vec) -> Self { + assert_eq!(executors.len(), 1); + Self::new(executors.into_iter().next().unwrap()) + } +} + +#[uniffi::export] +impl ForeignExecutorTester { + /// Schedule a fire-and-forget task to run the test + fn schedule_test(&self, delay: u32) { + let last_result = self.last_result.clone(); + *last_result.lock().unwrap() = None; + // Start a thread to schedule the call. This tests if the foreign bindings can handle + // schedule callbacks from a thread that they don't manage. + thread::scope(move |scope| { + scope.spawn(move || { + let start_time = Instant::now(); + let initial_thread_id = thread::current().id(); + // Schedule a call with the foreign executor + self.executor.schedule(delay, move || { + // Return data on when/where the call happened. We check that this matches the + // expectations in the foreign bindings tests + let call_happened_in_different_thread = + thread::current().id() != initial_thread_id; + let delay_ms = start_time.elapsed().as_millis() as u32; + *last_result.lock().unwrap() = Some(TestResult { + call_happened_in_different_thread, + delay_ms, + }); + }); + }); + }); + } + + fn get_last_result(&self) -> Option { + self.last_result.lock().unwrap().take() + } +} +#[derive(uniffi::Record)] +pub struct TestResult { + pub call_happened_in_different_thread: bool, + pub delay_ms: u32, +} + +include!(concat!(env!("OUT_DIR"), "/foreign_executor.uniffi.rs")); diff --git a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts new file mode 100644 index 0000000000..38ee5a7abd --- /dev/null +++ b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts @@ -0,0 +1,47 @@ +/* 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 uniffi.fixture_foreign_executor.* +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking + + +val coroutineScope = CoroutineScope(Dispatchers.IO) +// Test scheduling calls with no delay +runBlocking { + val tester = ForeignExecutorTester(coroutineScope) + launch { + tester.scheduleTest(0U) + } + delay(100L) + val result = tester.getLastResult() ?: throw RuntimeException("ForeignExecutorTester.getLastResult() returned null") + assert(result.callHappenedInDifferentThread) + assert(result.delayMs <= 100U) + tester.close() +} + +// Test scheduling calls with a delay and using the newFromSequence constructor +runBlocking { + val tester = ForeignExecutorTester.newFromSequence(listOf(coroutineScope)) + launch { + tester.scheduleTest(100U) + } + delay(200L) + val result = tester.getLastResult() ?: throw RuntimeException("ForeignExecutorTester.getLastResult() returned null") + assert(result.callHappenedInDifferentThread) + assert(result.delayMs >= 100U) + 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/foreign-executor/tests/bindings/test_foreign_executor.py b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.py new file mode 100644 index 0000000000..fdd06be520 --- /dev/null +++ b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.py @@ -0,0 +1,50 @@ +# 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 asyncio +import unittest +import weakref +from fixture_foreign_executor import ForeignExecutorTester + +class TestForeignExecutor(unittest.TestCase): + def test_schedule(self): + async def run_test(constructor, delay): + if constructor == "primary": + tester = ForeignExecutorTester(asyncio.get_running_loop()) + elif constructor == "new_from_sequence": + tester = ForeignExecutorTester.new_from_sequence([asyncio.get_running_loop()]) + else: + raise AssertionError(f"Unknown constructor: {constructor}") + tester.schedule_test(delay) + await asyncio.sleep((delay / 1000) + 0.1) + return tester.get_last_result() + + # Test no delay and lifting the foreign executor directly + result = asyncio.run(run_test("primary", 0)) + self.assertTrue(result.call_happened_in_different_thread) + self.assertTrue(result.delay_ms <= 1) + + # Test no delay and reading the foreign executor from a list + result = asyncio.run(run_test("new_from_sequence", 10)) + self.assertTrue(result.call_happened_in_different_thread) + self.assertTrue(9 <= result.delay_ms <= 11) + + def test_reference_counts(self): + # Create an event loop + loop = asyncio.new_event_loop() + loop_ref = weakref.ref(loop) + # Create ForeignExecutorTester that stores the loop + tester = ForeignExecutorTester(loop) + tester2 = ForeignExecutorTester.new_from_sequence([loop]), + # Test that testers hold a reference to the loop. After deleting the loop, the weakref should still be alive + loop.close() + del loop + self.assertNotEqual(loop_ref(), None, "ForeignExecutor didn't take a reference to the event loop") + # Deleting testers should cause the loop to be destroyed + del tester + del tester2 + self.assertEqual(loop_ref(), None, "ForeignExecutor didn't release a reference to the event loop") + +if __name__=='__main__': + unittest.main() diff --git a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift new file mode 100644 index 0000000000..1c8f6b0030 --- /dev/null +++ b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift @@ -0,0 +1,42 @@ +/* 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 Foundation +import fixture_foreign_executor + +func runTest(tester: ForeignExecutorTester, delay: UInt32) async -> TestResult { + let handle = Task { + tester.scheduleTest(delay: delay) + try! await Task.sleep(nanoseconds: numericCast((delay + 10) * 1000000)) + return tester.getLastResult()! + } + return await handle.value +} + +Task { + // Test scheduling with no delay + let result = await runTest( + tester: ForeignExecutorTester( + executor: UniFfiForeignExecutor(priority: TaskPriority.background) + ), + delay: 0 + ) + assert(result.callHappenedInDifferentThread) + assert(result.delayMs <= 1) + + // Test scheduling with delay and an executor created from a list + let result2 = await runTest( + tester: ForeignExecutorTester.newFromSequence( + executors: [UniFfiForeignExecutor(priority: TaskPriority.background)] + ), + delay: 1000 + ) + assert(result2.callHappenedInDifferentThread) + assert(result2.delayMs >= 90) + assert(result2.delayMs <= 110) +} + + + +// No need to test reference counting, since `UniFfiForeignExecutor` on Swift is just a value type diff --git a/fixtures/foreign-executor/tests/test_generated_bindings.rs b/fixtures/foreign-executor/tests/test_generated_bindings.rs new file mode 100644 index 0000000000..46605c0e93 --- /dev/null +++ b/fixtures/foreign-executor/tests/test_generated_bindings.rs @@ -0,0 +1,5 @@ +uniffi::build_foreign_language_testcases!( + "tests/bindings/test_foreign_executor.py", + "tests/bindings/test_foreign_executor.kts", + "tests/bindings/test_foreign_executor.swift", +); diff --git a/fixtures/metadata/src/tests.rs b/fixtures/metadata/src/tests.rs index f42947a38a..b7e5c85c2f 100644 --- a/fixtures/metadata/src/tests.rs +++ b/fixtures/metadata/src/tests.rs @@ -120,6 +120,7 @@ mod test_type_ids { check_type_id::(Type::F64); check_type_id::(Type::Bool); check_type_id::(Type::String); + check_type_id::(Type::ForeignExecutor); } #[test] diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/executor.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/executor.rs new file mode 100644 index 0000000000..b6297899c0 --- /dev/null +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/executor.rs @@ -0,0 +1,22 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use crate::backend::{CodeOracle, CodeType}; + +pub struct ForeignExecutorCodeType; + +impl CodeType for ForeignExecutorCodeType { + fn type_label(&self, _oracle: &dyn CodeOracle) -> String { + // Kotlin uses a CoroutineScope for ForeignExecutor + "CoroutineScope".into() + } + + fn canonical_name(&self, _oracle: &dyn CodeOracle) -> String { + "ForeignExecutor".into() + } + + fn initialization_fn(&self, _oracle: &dyn CodeOracle) -> Option { + Some("FfiConverterForeignExecutor.register".into()) + } +} diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 18295c7385..a5cede607d 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -20,6 +20,7 @@ mod compounds; mod custom; mod enum_; mod error; +mod executor; mod external; mod miscellany; mod object; @@ -253,6 +254,7 @@ impl KotlinCodeOracle { Type::CallbackInterface(id) => { Box::new(callback_interface::CallbackInterfaceCodeType::new(id)) } + Type::ForeignExecutor => Box::new(executor::ForeignExecutorCodeType), Type::Optional(inner) => Box::new(compounds::OptionalCodeType::new(*inner)), Type::Sequence(inner) => Box::new(compounds::SequenceCodeType::new(*inner)), Type::Map(key, value) => Box::new(compounds::MapCodeType::new(*key, *value)), @@ -319,6 +321,8 @@ impl CodeOracle for KotlinCodeOracle { }, FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(), FfiType::ForeignCallback => "ForeignCallback".to_string(), + FfiType::ForeignExecutorHandle => "USize".to_string(), + FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback".to_string(), } } } @@ -400,6 +404,7 @@ pub mod filters { }, FfiType::ForeignBytes => "ForeignBytes".into(), FfiType::ForeignCallback => "ForeignCallback".into(), + FfiType::ForeignExecutorHandle | FfiType::ForeignExecutorCallback => todo!(), }) } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt new file mode 100644 index 0000000000..4f26817f31 --- /dev/null +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt @@ -0,0 +1,60 @@ +{{ self.add_import("kotlinx.coroutines.CoroutineScope") }} +{{ self.add_import("kotlinx.coroutines.delay") }} +{{ self.add_import("kotlinx.coroutines.launch") }} + +// Callback function to execute a Rust task. The Kotlin code schedules these in a coroutine then +// invokes them. +internal interface UniFfiRustTaskCallback : com.sun.jna.Callback { + fun invoke(rustTaskData: Pointer?) +} + +object UniFfiForeignExecutorCallback : com.sun.jna.Callback { + internal fun invoke(handle: USize, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) { + if (rustTask == null) { + FfiConverterForeignExecutor.drop(handle) + } else { + val coroutineScope = FfiConverterForeignExecutor.lift(handle) + coroutineScope.launch { + if (delayMs > 0) { + delay(delayMs.toLong()) + } + rustTask.invoke(rustTaskData) + } + } + } +} + +public object FfiConverterForeignExecutor: FfiConverter { + internal val handleMap = UniFfiHandleMap() + + internal fun drop(handle: USize) { + handleMap.remove(handle) + } + + internal fun register(lib: _UniFFILib) { + lib.uniffi_foreign_executor_callback_set(UniFfiForeignExecutorCallback) + } + + // 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 lift(value: USize): CoroutineScope { + return handleMap.get(value) ?: throw RuntimeException("unknown handle in FfiConverterForeignExecutor.lift") + } + + override fun read(buf: ByteBuffer): CoroutineScope { + return lift(USize.readFromBuffer(buf)) + } + + override fun lower(value: CoroutineScope): USize { + return handleMap.insert(value) + } + + override fun write(value: CoroutineScope, buf: ByteBuffer) { + lower(value).writeToBuffer(buf) + } +} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt index d7e61ef4c7..0d6d485507 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt @@ -64,3 +64,83 @@ object NullCallStatusErrorHandler: CallStatusErrorHandler { 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() + 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) { + map.remove(handle) + } +} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt index 5037b7c696..014e95eef5 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt @@ -77,6 +77,9 @@ {%- when Type::CallbackInterface(name) %} {% include "CallbackInterfaceTemplate.kt" %} +{%- when Type::ForeignExecutor %} +{% include "ForeignExecutorTemplate.kt" %} + {%- when Type::Timestamp %} {% include "TimestampHelper.kt" %} @@ -92,3 +95,17 @@ {%- else %} {%- endmatch %} {%- endfor %} + +// Add imports for async functions. This doesn't really have anything to do with types, but +// add_import() is only available in Types.kt. +{%- if ci.has_async_fns() %} +{{ self.add_import("kotlin.coroutines.Continuation") }} +{{ self.add_import("kotlin.coroutines.resume") }} +{{ self.add_import("kotlin.coroutines.resumeWithException") }} +{{ self.add_import("kotlin.coroutines.suspendCoroutine") }} +{{ self.add_import("kotlinx.coroutines.coroutineScope") }} +{{ self.add_import("kotlinx.coroutines.CoroutineScope") }} +{{ self.add_import("kotlinx.coroutines.launch") }} +{{ self.add_import("kotlinx.coroutines.sync.Semaphore") }} +{{ self.add_import("kotlinx.coroutines.sync.withPermit") }} +{%- endif %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt index 9744358827..1db5639cc4 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt @@ -18,6 +18,7 @@ package {{ config.package_name() }}; // helpers directly inline like we're doing here. import com.sun.jna.Library +import com.sun.jna.IntegerType import com.sun.jna.Native import com.sun.jna.Pointer import com.sun.jna.Structure @@ -26,17 +27,6 @@ import com.sun.jna.ptr.* import java.nio.ByteBuffer import java.nio.ByteOrder import java.util.concurrent.ConcurrentHashMap -{%- if ci.has_async_fns() %} -import kotlin.coroutines.Continuation -import kotlin.coroutines.resume -import kotlin.coroutines.resumeWithException -import kotlin.coroutines.suspendCoroutine -import kotlinx.coroutines.coroutineScope -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Semaphore -import kotlinx.coroutines.sync.withPermit -{%- endif %} {%- for req in self.imports() %} {{ req.render() }} diff --git a/uniffi_bindgen/src/bindings/python/gen_python/executor.rs b/uniffi_bindgen/src/bindings/python/gen_python/executor.rs new file mode 100644 index 0000000000..c9446881e7 --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/gen_python/executor.rs @@ -0,0 +1,21 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use crate::backend::{CodeOracle, CodeType}; + +pub struct ForeignExecutorCodeType; + +impl CodeType for ForeignExecutorCodeType { + fn type_label(&self, _oracle: &dyn CodeOracle) -> String { + "asyncio.BaseEventLoop".into() + } + + fn canonical_name(&self, _oracle: &dyn CodeOracle) -> String { + "ForeignExecutor".into() + } + + fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String { + nm.to_string() + } +} diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index 23e748c934..f294d6ed7a 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -20,6 +20,7 @@ mod compounds; mod custom; mod enum_; mod error; +mod executor; mod external; mod miscellany; mod object; @@ -299,7 +300,7 @@ impl PythonCodeOracle { Type::CallbackInterface(id) => { Box::new(callback_interface::CallbackInterfaceCodeType::new(id)) } - + Type::ForeignExecutor => Box::new(executor::ForeignExecutorCodeType), Type::Optional(inner) => Box::new(compounds::OptionalCodeType::new(*inner)), Type::Sequence(inner) => Box::new(compounds::SequenceCodeType::new(*inner)), Type::Map(key, value) => Box::new(compounds::MapCodeType::new(*key, *value)), @@ -363,6 +364,9 @@ impl CodeOracle for PythonCodeOracle { }, FfiType::ForeignBytes => "ForeignBytes".to_string(), FfiType::ForeignCallback => "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(), } } } diff --git a/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py new file mode 100644 index 0000000000..2af0626ad1 --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py @@ -0,0 +1,42 @@ +# FFI code for the ForeignExecutor type + +{{ self.add_import("asyncio") }} + +class {{ ffi_converter_name }}: + _pointer_manager = UniFfiPointerManager() + + @classmethod + def lower(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 write(cls, eventloop, buf): + buf.writeCSizeT(cls.lower(eventloop)) + + @classmethod + def read(cls, buf): + return cls.lift(buf.readCSizeT()) + + @classmethod + def lift(cls, value): + return cls._pointer_manager.lookup(value) + +@UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T +def uniffi_executor_callback(eventloop_address, delay, task_ptr, task_data): + if task_ptr is None: + {{ ffi_converter_name }}._pointer_manager.release_pointer(eventloop_address) + else: + eventloop = {{ ffi_converter_name }}._pointer_manager.lookup(eventloop_address) + callback = UNIFFI_RUST_TASK(task_ptr) + if delay == 0: + # This can be called from any thread, so make sure to use `call_soon_threadsafe' + eventloop.call_soon_threadsafe(callback, task_data) + else: + # For delayed tasks, we use `call_soon_threadsafe()` + `call_later()` to make the + # operation threadsafe + eventloop.call_soon_threadsafe(eventloop.call_later, delay / 1000.0, callback, task_data) + +# Register the callback with the scaffolding +_UniFFILib.uniffi_foreign_executor_callback_set(uniffi_executor_callback) diff --git a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py index 44f0ab2307..1a5d2e89b6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py @@ -1,13 +1,33 @@ -# This is how we find and load the dynamic library provided by the component. -# For now we just look it up by name. -# -# XXX TODO: This will probably grow some magic for resolving megazording in future. -# E.g. we might start by looking for the named component in `libuniffi.so` and if -# that fails, fall back to loading it separately from `lib${componentName}.so`. +# Define some ctypes FFI types that we use in the library + +""" +ctypes type for the foreign executor callback. This is a built-in interface for scheduling +tasks + +Args: + executor: opaque c_size_t value representing the eventloop + delay: delay in ms + task: function pointer to the task callback + task_data: void pointer to the task callback data + +Normally we should call task(task_data) after the detail. +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(None, ctypes.c_size_t, 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 +""" +UNIFFI_RUST_TASK = ctypes.CFUNCTYPE(None, ctypes.c_void_p) from pathlib import Path def loadIndirect(): + """ + This is how we find and load the dynamic library provided by the component. + For now we just look it up by name. + """ if sys.platform == "darwin": libname = "lib{}.dylib" elif sys.platform.startswith("win"): diff --git a/uniffi_bindgen/src/bindings/python/templates/PointerManager.py b/uniffi_bindgen/src/bindings/python/templates/PointerManager.py new file mode 100644 index 0000000000..df5df842e0 --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/templates/PointerManager.py @@ -0,0 +1,65 @@ +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): + ctypes.pythonapi.Py_DecRef(ctypes.cast(address, ctypes.py_object)) + + 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: + del self._map[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 +else: + UniFfiPointerManager = UniFfiPointerManagerGeneral diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py index ba050b9d8b..ad4a65bc8c 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py @@ -137,6 +137,8 @@ def readFloat(self): def readDouble(self): return self._unpack_from(8, ">d") + def readCSizeT(self): + return self._unpack_from(ctypes.sizeof(ctypes.c_size_t) , "@N") class RustBufferBuilder(object): """ @@ -204,3 +206,6 @@ def writeFloat(self, v): def writeDouble(self, v): self._pack_into(8, ">d", v) + + def writeCSizeT(self, v): + self._pack_into(ctypes.sizeof(ctypes.c_size_t) , "@N", v) diff --git a/uniffi_bindgen/src/bindings/python/templates/Types.py b/uniffi_bindgen/src/bindings/python/templates/Types.py index 6e9765af80..7500f21679 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Types.py +++ b/uniffi_bindgen/src/bindings/python/templates/Types.py @@ -88,6 +88,9 @@ {%- when Type::External { name, crate_name, kind } %} {%- include "ExternalTemplate.py" %} +{%- when Type::ForeignExecutor %} +{%- include "ForeignExecutorTemplate.py" %} + {%- else %} {%- endmatch %} {%- endfor %} diff --git a/uniffi_bindgen/src/bindings/python/templates/macros.py b/uniffi_bindgen/src/bindings/python/templates/macros.py index d11fcae921..20f885cdbd 100644 --- a/uniffi_bindgen/src/bindings/python/templates/macros.py +++ b/uniffi_bindgen/src/bindings/python/templates/macros.py @@ -61,7 +61,7 @@ {%- for arg in func.arguments() %} {{ arg.type_().borrow()|ffi_type_name }}, {%- endfor %} - ctypes.POINTER(RustCallStatus), + {%- if func.has_rust_call_status_arg() %}ctypes.POINTER(RustCallStatus),{% endif %} {% endmacro -%} {# diff --git a/uniffi_bindgen/src/bindings/python/templates/wrapper.py b/uniffi_bindgen/src/bindings/python/templates/wrapper.py index ba54e1c0d8..668655f31b 100644 --- a/uniffi_bindgen/src/bindings/python/templates/wrapper.py +++ b/uniffi_bindgen/src/bindings/python/templates/wrapper.py @@ -29,6 +29,7 @@ {%- endif %} import contextvars import enum +import platform {%- for req in self.imports() %} {{ req.render() }} {%- endfor %} @@ -38,6 +39,7 @@ {% include "RustBufferTemplate.py" %} {% include "Helpers.py" %} +{% include "PointerManager.py" %} {% include "RustBufferHelper.py" %} {%- if ci.has_async_fns() %} {% include "RustFutureTemplate.py" %} diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs index 7fb9c72fbf..321e9fd604 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -96,6 +96,12 @@ mod filters { FfiType::RustBuffer(_) => "RustBuffer.by_value".to_string(), FfiType::ForeignBytes => "ForeignBytes".to_string(), FfiType::ForeignCallback => unimplemented!("Callback interfaces are not implemented"), + FfiType::ForeignExecutorCallback => { + unimplemented!("Foreign executors are not implemented") + } + FfiType::ForeignExecutorHandle => { + unimplemented!("Foreign executors are not implemented") + } }) } @@ -192,6 +198,7 @@ mod filters { } Type::External { .. } => panic!("No support for external types, yet"), Type::Custom { .. } => panic!("No support for custom types, yet"), + Type::ForeignExecutor => unimplemented!("Foreign executors are not implemented"), }) } @@ -225,6 +232,7 @@ mod filters { ), Type::External { .. } => panic!("No support for lowering external types, yet"), Type::Custom { .. } => panic!("No support for lowering custom types, yet"), + Type::ForeignExecutor => unimplemented!("Foreign executors are not implemented"), }) } @@ -257,6 +265,7 @@ mod filters { ), Type::External { .. } => panic!("No support for lifting external types, yet"), Type::Custom { .. } => panic!("No support for lifting custom types, yet"), + Type::ForeignExecutor => unimplemented!("Foreign executors are not implemented"), }) } } diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/executor.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/executor.rs new file mode 100644 index 0000000000..3d66ac5e1d --- /dev/null +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/executor.rs @@ -0,0 +1,22 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use crate::backend::{CodeOracle, CodeType}; + +pub struct ForeignExecutorCodeType; + +impl CodeType for ForeignExecutorCodeType { + fn type_label(&self, _oracle: &dyn CodeOracle) -> String { + // On Swift, we define a struct to represent a ForeignExecutor + "UniFfiForeignExecutor".into() + } + + fn canonical_name(&self, _oracle: &dyn CodeOracle) -> String { + "ForeignExecutor".into() + } + + fn initialization_fn(&self, _oracle: &dyn CodeOracle) -> Option { + Some("uniffiInitForeignExecutor".into()) + } +} diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs index 6e6f839dac..9411f3f1b2 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -21,6 +21,7 @@ mod compounds; mod custom; mod enum_; mod error; +mod executor; mod external; mod miscellany; mod object; @@ -319,7 +320,7 @@ impl SwiftCodeOracle { Type::CallbackInterface(id) => { Box::new(callback_interface::CallbackInterfaceCodeType::new(id)) } - + Type::ForeignExecutor => Box::new(executor::ForeignExecutorCodeType), Type::Optional(inner) => Box::new(compounds::OptionalCodeType::new(*inner)), Type::Sequence(inner) => Box::new(compounds::SequenceCodeType::new(*inner)), Type::Map(key, value) => Box::new(compounds::MapCodeType::new(*key, *value)), @@ -374,7 +375,9 @@ impl CodeOracle for SwiftCodeOracle { FfiType::RustArcPtr(_) => "void*_Nonnull".into(), FfiType::RustBuffer(_) => "RustBuffer".into(), FfiType::ForeignBytes => "ForeignBytes".into(), - FfiType::ForeignCallback => "ForeignCallback _Nonnull".to_string(), + FfiType::ForeignCallback => "ForeignCallback _Nonnull".into(), + FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback _Nonnull".into(), + FfiType::ForeignExecutorHandle => "size_t".into(), } } } @@ -446,7 +449,9 @@ pub mod filters { FfiType::RustArcPtr(_) => "UnsafeMutableRawPointer".into(), FfiType::RustBuffer(_) => "RustBuffer".into(), FfiType::ForeignBytes => "ForeignBytes".into(), - FfiType::ForeignCallback => "ForeignCallback _Nonnull".to_string(), + FfiType::ForeignCallback => "ForeignCallback _Nonnull".into(), + FfiType::ForeignExecutorHandle => "Int".into(), + FfiType::ForeignExecutorCallback => "ForeignExecutorCallback _Nonnull".into(), }) } diff --git a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h index 2fc759d343..f277036979 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h +++ b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include // The following structs are used to implement the lowest level @@ -30,6 +31,17 @@ typedef struct RustBuffer typedef int32_t (*ForeignCallback)(uint64_t, int32_t, const uint8_t *_Nonnull, int32_t, RustBuffer *_Nonnull); +// Task defined in Rust that Swift executes +typedef void (*UniFfiRustTaskCallback)(const void * _Nullable); +// Callback to execute Rust tasks using a Swift Task +// +// Args: +// executor: ForeignExecutor lowered into a size_t value +// delay: Delay in MS +// task: UniFfiRustTaskCallback to call +// task_data: data to pass the task callback +typedef void (*UniFfiForeignExecutorCallback)(size_t, uint32_t, UniFfiRustTaskCallback _Nullable, const void * _Nullable); + typedef struct ForeignBytes { int32_t len; diff --git a/uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift new file mode 100644 index 0000000000..54aec08556 --- /dev/null +++ b/uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift @@ -0,0 +1,52 @@ +// Encapsulates an executor that can run Rust tasks +// +// On Swift, `Task.detached` can handle this we just need to know what priority to send it. +public struct UniFfiForeignExecutor { + var priority: TaskPriority + + public init(priority: TaskPriority) { + self.priority = priority + } +} + +fileprivate struct FfiConverterForeignExecutor: FfiConverter { + typealias SwiftType = UniFfiForeignExecutor + // Rust uses a pointer to represent the FfiConverterForeignExecutor, but we only need a u8. + // let's use `Int`, which is equivalent to `size_t` + typealias FfiType = Int + + static func lift(_ value: FfiType) throws -> SwiftType { + UniFfiForeignExecutor(priority: TaskPriority(rawValue: numericCast(value))) + } + static func lower(_ value: SwiftType) -> FfiType { + numericCast(value.priority.rawValue) + } + + static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> SwiftType { + fatalError("FfiConverterForeignExecutor.read not implemented yet") + } + static func write(_ value: SwiftType, into buf: inout [UInt8]) { + fatalError("FfiConverterForeignExecutor.read not implemented yet") + } +} + + +fileprivate func uniffiForeignExecutorCallback(executorHandle: Int, delayMs: UInt32, rustTask: UniFfiRustTaskCallback?, taskData: UnsafeRawPointer?) { + if let rustTask = rustTask { + let executor = try! FfiConverterForeignExecutor.lift(executorHandle) + Task.detached(priority: executor.priority) { + if delayMs != 0 { + let nanoseconds: UInt64 = numericCast(delayMs * 1000000) + try! await Task.sleep(nanoseconds: nanoseconds) + } + rustTask(taskData) + } + + } + // No else branch: when rustTask is null, we should drop the foreign executor. However, since + // its just a value type, we don't need to do anything here. +} + +fileprivate func uniffiInitForeignExecutor() { + uniffi_foreign_executor_callback_set(uniffiForeignExecutorCallback) +} diff --git a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift index 8f5ed8efdd..33983186fa 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift @@ -58,7 +58,7 @@ private func rustCallWithError } private func makeRustCall(_ callback: (UnsafeMutablePointer) -> T, errorHandler: (RustBuffer) throws -> Error) throws -> T { - uniffiCheckFfiVersionMismatch() + uniffiEnsureInitialized() var callStatus = RustCallStatus.init() let returnedVal = callback(&callStatus) switch callStatus.code { diff --git a/uniffi_bindgen/src/bindings/swift/templates/Types.swift b/uniffi_bindgen/src/bindings/swift/templates/Types.swift index 10fb6ec790..4e85c925da 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Types.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Types.swift @@ -61,6 +61,9 @@ {%- when Type::CallbackInterface(name) %} {%- include "CallbackInterfaceTemplate.swift" %} +{%- when Type::ForeignExecutor %} +{%- include "ForeignExecutorTemplate.swift" %} + {%- when Type::Custom { name, builtin } %} {%- include "CustomType.swift" %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift index 0e63ce24fb..8aa85a9195 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift @@ -23,32 +23,37 @@ import {{ config.ffi_module_name() }} {%- include "TopLevelFunctionTemplate.swift" %} {%- endfor %} -private enum CheckVersionResult { +private enum InitializationResult { case ok case contractVersionMismatch case apiChecksumMismatch } // Use a global variables to perform the versioning checks. Swift ensures that // the code inside is only computed once. -private var checkVersionResult: CheckVersionResult { +private var initializationResult: InitializationResult { // Get the bindings contract version from our ComponentInterface let bindings_contract_version = {{ ci.uniffi_contract_version() }} // Get the scaffolding contract version by calling the into the dylib let scaffolding_contract_version = {{ ci.ffi_uniffi_contract_version().name() }}() if bindings_contract_version != scaffolding_contract_version { - return CheckVersionResult.contractVersionMismatch + return InitializationResult.contractVersionMismatch } {%- for (name, expected_checksum) in ci.iter_checksums() %} if ({{ name }}() != {{ expected_checksum }}) { - return CheckVersionResult.apiChecksumMismatch + return InitializationResult.apiChecksumMismatch } {%- endfor %} - return CheckVersionResult.ok + + {% for fn in self.initialization_fns() -%} + {{ fn }}() + {% endfor -%} + + return InitializationResult.ok } -private func uniffiCheckFfiVersionMismatch() { - switch checkVersionResult { +private func uniffiEnsureInitialized() { + switch initializationResult { case .ok: break case .contractVersionMismatch: diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index bb30809959..abaa6dd317 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -45,9 +45,13 @@ pub enum FfiType { /// A borrowed reference to some raw bytes owned by foreign language code. /// The provider of this reference must keep it alive for the duration of the receiving call. ForeignBytes, - /// A pointer to a single function in to the foreign language. - /// This function contains all the machinery to make callbacks work on the foreign language side. + /// 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, // 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. } diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 7eba98d6c6..feb41bd05e 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -419,8 +419,9 @@ impl ComponentInterface { self.iter_user_ffi_function_definitions() .cloned() .chain(self.iter_rust_buffer_ffi_function_definitions()) - .chain(iter::once(self.ffi_uniffi_contract_version())) .chain(self.iter_checksum_ffi_functions()) + .chain(self.ffi_foreign_executor_callback_set()) + .chain([self.ffi_uniffi_contract_version()]) } /// List all FFI functions definitions for user-defined interfaces @@ -455,6 +456,27 @@ impl ComponentInterface { .into_iter() } + /// The ffi_foreign_executor_callback_set FFI function + /// + /// We only include this in the FFI if the `ForeignExecutor` type is actually used + pub fn ffi_foreign_executor_callback_set(&self) -> Option { + if self.types.contains(&Type::ForeignExecutor) { + Some(FfiFunction { + name: "uniffi_foreign_executor_callback_set".into(), + arguments: vec![FfiArgument { + name: "callback".into(), + type_: FfiType::ForeignExecutorCallback, + }], + return_type: None, + is_async: false, + has_rust_call_status_arg: false, + is_object_free_function: false, + }) + } else { + None + } + } + /// List all API checksums to check /// /// Returns a list of (export_symbol_name, checksum) items @@ -983,6 +1005,7 @@ fn convert_type(s: &uniffi_meta::Type) -> Type { Ty::String => Type::String, Ty::SystemTime => Type::Timestamp, Ty::Duration => Type::Duration, + Ty::ForeignExecutor => Type::ForeignExecutor, Ty::Record { name } => Type::Record(name.clone()), Ty::Enum { name } => Type::Enum(name.clone()), Ty::ArcObject { diff --git a/uniffi_bindgen/src/interface/types/mod.rs b/uniffi_bindgen/src/interface/types/mod.rs index 0a0d37b99f..d9f9483b64 100644 --- a/uniffi_bindgen/src/interface/types/mod.rs +++ b/uniffi_bindgen/src/interface/types/mod.rs @@ -90,6 +90,7 @@ pub enum Type { // How the object is implemented. imp: ObjectImpl, }, + ForeignExecutor, // Types defined in the component API, each of which has a string name. Record(String), Enum(String), @@ -154,6 +155,7 @@ impl Type { Type::CallbackInterface(nm) => format!("CallbackInterface{nm}"), Type::Timestamp => "Timestamp".into(), Type::Duration => "Duration".into(), + Type::ForeignExecutor => "ForeignExecutor".into(), // Recursive types. // These add a prefix to the name of the underlying type. // The component API definition cannot give names to recursive types, so as long as the @@ -213,6 +215,7 @@ impl From<&Type> for FfiType { Type::Object { name, .. } => FfiType::RustArcPtr(name.to_owned()), // Callback interfaces are passed as opaque integer handles. Type::CallbackInterface(_) => FfiType::UInt64, + Type::ForeignExecutor => FfiType::ForeignExecutorHandle, // Other types are serialized into a bytebuffer and deserialized on the other side. Type::Enum(_) | Type::Error(_) @@ -343,6 +346,11 @@ impl TypeUniverse { Ok(()) } + /// Check if a [Type] is present + pub fn contains(&self, type_: &Type) -> bool { + self.all_known_types.contains(type_) + } + /// Iterator over all the known types in this universe. pub fn iter_known_types(&self) -> impl Iterator { self.all_known_types.iter() diff --git a/uniffi_bindgen/src/interface/types/resolver.rs b/uniffi_bindgen/src/interface/types/resolver.rs index 35ae640254..3156d4479d 100644 --- a/uniffi_bindgen/src/interface/types/resolver.rs +++ b/uniffi_bindgen/src/interface/types/resolver.rs @@ -216,6 +216,7 @@ pub(in super::super) fn resolve_builtin_type(name: &str) -> Option { "f64" => Some(Type::Float64), "timestamp" => Some(Type::Timestamp), "duration" => Some(Type::Duration), + "ForeignExecutor" => Some(Type::ForeignExecutor), _ => None, } } diff --git a/uniffi_bindgen/src/scaffolding/mod.rs b/uniffi_bindgen/src/scaffolding/mod.rs index 3047540ddf..0f3bc2ac24 100644 --- a/uniffi_bindgen/src/scaffolding/mod.rs +++ b/uniffi_bindgen/src/scaffolding/mod.rs @@ -45,6 +45,7 @@ mod filters { Type::Enum(name) | Type::Record(name) | Type::Error(name) => format!("r#{name}"), Type::Object { name, imp } => format!("std::sync::Arc<{}>", imp.rust_name_for(name)), Type::CallbackInterface(name) => format!("Box"), + Type::ForeignExecutor => "::uniffi::ForeignExecutor".into(), Type::Optional(t) => format!("std::option::Option<{}>", type_rs(t)?), Type::Sequence(t) => format!("std::vec::Vec<{}>", type_rs(t)?), Type::Map(k, v) => format!( @@ -75,9 +76,11 @@ mod filters { FfiType::Float32 => "f32".into(), FfiType::Float64 => "f64".into(), FfiType::RustArcPtr(_) => "*const std::os::raw::c_void".into(), - FfiType::RustBuffer(_) => "uniffi::RustBuffer".into(), - FfiType::ForeignBytes => "uniffi::ForeignBytes".into(), - FfiType::ForeignCallback => "uniffi::ForeignCallback".into(), + FfiType::RustBuffer(_) => "::uniffi::RustBuffer".into(), + FfiType::ForeignBytes => "::uniffi::ForeignBytes".into(), + FfiType::ForeignCallback => "::uniffi::ForeignCallback".into(), + FfiType::ForeignExecutorHandle => "::uniffi::ForeignExecutorHandle".into(), + FfiType::ForeignExecutorCallback => "::uniffi::ForeignExecutorCallback".into(), }) } diff --git a/uniffi_core/src/ffi/ffidefault.rs b/uniffi_core/src/ffi/ffidefault.rs index d3ca9438d8..1f86f6b13b 100644 --- a/uniffi_core/src/ffi/ffidefault.rs +++ b/uniffi_core/src/ffi/ffidefault.rs @@ -51,6 +51,12 @@ 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/foreignexecutor.rs b/uniffi_core/src/ffi/foreignexecutor.rs new file mode 100644 index 0000000000..5db403c613 --- /dev/null +++ b/uniffi_core/src/ffi/foreignexecutor.rs @@ -0,0 +1,435 @@ +/* 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/. */ + +//! Schedule tasks using a foreign executor. + +use std::{ + cell::UnsafeCell, + future::Future, + panic, + pin::Pin, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, Mutex, + }, + task::{Context, Poll, Waker}, +}; + +/// 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 {} + +/// Callback to schedule a Rust call with a `ForeignExecutor`. The bindings code registers exactly +/// one of these with the Rust code. +/// +/// Delay is an approximate amount of ms to wait before scheduling the call. Delay is usually 0, +/// which means schedule sometime soon. +/// +/// As a special case, when Rust drops the foreign executor, with 'task=null'`. The foreign +/// bindings should release the reference to the executor that was reserved for Rust. +/// +/// This callback can be invoked from any thread, including threads created by Rust. +pub type ForeignExecutorCallback = extern "C" fn( + executor: ForeignExecutorHandle, + delay: u32, + task: Option, + task_data: *const (), +); + +// Option should use the null pointer optimization and be represented in C as a +// regular pointer. Let's check that. +static_assertions::assert_eq_size!(usize, Option); + +/// Callback for a Rust task, this is what the foreign executor invokes +pub type RustTaskCallback = extern "C" fn(*const ()); + +static FOREIGN_EXECUTOR_CALLBACK: AtomicUsize = AtomicUsize::new(0); + +/// Set the global ForeignExecutorCallback. This is called by the foreign bindings, normally +/// during initialization. +#[no_mangle] +pub extern "C" fn uniffi_foreign_executor_callback_set(callback: ForeignExecutorCallback) { + FOREIGN_EXECUTOR_CALLBACK.store(callback as usize, Ordering::Relaxed); +} + +fn get_foreign_executor_callback() -> ForeignExecutorCallback { + match FOREIGN_EXECUTOR_CALLBACK.load(Ordering::Relaxed) { + 0 => panic!("FOREIGN_EXECUTOR_CALLBACK not set"), + // SAFETY: The below call is okay because we only store values in + // FOREIGN_EXECUTOR_CALLBACK that were cast from a ForeignExecutorCallback. + n => unsafe { std::mem::transmute(n) }, + } +} + +/// Schedule Rust calls using a foreign executor +#[derive(Debug)] +pub struct ForeignExecutor { + pub(crate) handle: ForeignExecutorHandle, +} + +impl ForeignExecutor { + pub fn new(executor: ForeignExecutorHandle) -> Self { + Self { handle: executor } + } + + /// Schedule a closure to be run. + /// + /// This method can be used for "fire-and-forget" style calls, where the calling code doesn't + /// need to await the result. + /// + /// Closure requirements: + /// - Send: since the closure will likely run on a different thread + /// - 'static: since it runs at an arbitrary time, so all references need to be 'static + /// - panic::UnwindSafe: if the closure panics, it should not corrupt any data + pub fn schedule(&self, delay: u32, task: F) { + ScheduledTask::new(task).schedule_callback(self.handle, delay) + } + + /// Schedule a closure to be run and get a Future for the result + /// + /// Closure requirements: + /// - Send: since the closure will likely run on a different thread + /// - 'static: since it runs at an arbitrary time, so all references need to be 'static + /// - panic::UnwindSafe: if the closure panics, it should not corrupt any data + pub fn run T + Send + 'static + panic::UnwindSafe, T>( + &self, + delay: u32, + closure: F, + ) -> impl Future { + let future = RunFuture::new(closure); + future.schedule_callback(self.handle, delay); + future + } +} + +/// Low-level schedule interface +/// +/// When using this function, take care to ensure that the ForeignExecutor that holds the +/// ForeignExecutorHandle has not been dropped. +pub(crate) fn schedule_raw( + handle: ForeignExecutorHandle, + delay: u32, + callback: RustTaskCallback, + data: *const (), +) { + (get_foreign_executor_callback())(handle, delay, Some(callback), data) +} + +impl Drop for ForeignExecutor { + fn drop(&mut self) { + (get_foreign_executor_callback())(self.handle, 0, None, std::ptr::null()) + } +} +/// Struct that handles the ForeignExecutor::schedule() method +struct ScheduledTask { + task: F, +} + +impl ScheduledTask +where + F: FnOnce() + Send + 'static + panic::UnwindSafe, +{ + fn new(task: F) -> Self { + Self { task } + } + + fn schedule_callback(self, handle: ForeignExecutorHandle, delay: u32) { + let leaked_ptr = Box::leak(Box::new(self)); + schedule_raw( + handle, + delay, + Self::callback, + leaked_ptr as *const Self as *const (), + ); + } + + extern "C" fn callback(data: *const ()) { + run_task(unsafe { Box::from_raw(data as *mut Self).task }); + } +} + +/// Struct that handles the ForeignExecutor::run() method +struct RunFuture { + inner: Arc>, +} + +// State inside the RunFuture Arc<> +struct RunFutureInner { + // SAFETY: we only access this once in the scheduled callback + task: UnsafeCell>, + mutex: Mutex>, +} + +// State inside the RunFuture Mutex<> +struct RunFutureInner2 { + result: Option, + waker: Option, +} + +impl RunFuture +where + F: FnOnce() -> T + Send + 'static + panic::UnwindSafe, +{ + fn new(task: F) -> Self { + Self { + inner: Arc::new(RunFutureInner { + task: UnsafeCell::new(Some(task)), + mutex: Mutex::new(RunFutureInner2 { + result: None, + waker: None, + }), + }), + } + } + + fn schedule_callback(&self, handle: ForeignExecutorHandle, delay: u32) { + let raw_ptr = Arc::into_raw(Arc::clone(&self.inner)); + schedule_raw(handle, delay, Self::callback, raw_ptr as *const ()); + } + + extern "C" fn callback(data: *const ()) { + unsafe { + let inner = Arc::from_raw(data as *const RunFutureInner); + let task = (*inner.task.get()).take().unwrap(); + if let Some(result) = run_task(task) { + let mut inner2 = inner.mutex.lock().unwrap(); + inner2.result = Some(result); + if let Some(waker) = inner2.waker.take() { + waker.wake(); + } + } + } + } +} + +impl Future for RunFuture { + type Output = T; + + fn poll(self: Pin<&mut Self>, context: &mut Context<'_>) -> Poll { + let mut inner2 = self.inner.mutex.lock().unwrap(); + match inner2.result.take() { + Some(v) => Poll::Ready(v), + None => { + inner2.waker = Some(context.waker().clone()); + Poll::Pending + } + } + } +} + +/// Run a scheduled task, catching any panics. +/// +/// If there are panics, then we will log a warning and return None. +fn run_task T + panic::UnwindSafe, T>(task: F) -> Option { + match panic::catch_unwind(task) { + Ok(v) => Some(v), + Err(cause) => { + let message = if let Some(s) = cause.downcast_ref::<&'static str>() { + (*s).to_string() + } else if let Some(s) = cause.downcast_ref::() { + s.clone() + } else { + "Unknown panic!".to_string() + }; + log::warn!("Error calling UniFFI callback function: {message}"); + None + } + } +} + +#[cfg(test)] +pub use test::MockExecutor; + +#[cfg(test)] +mod test { + use super::*; + use std::sync::{ + atomic::{AtomicU32, Ordering}, + Once, + }; + use std::task::Wake; + + static MOCK_EXECUTOR_INIT: Once = Once::new(); + + // Executor for testing that stores scheduled calls in a Vec + pub struct MockExecutor { + pub calls: &'static Mutex, *const ())>>, + pub executor: Option, + } + + impl MockExecutor { + pub fn new() -> Self { + // Create a boxed call list and immediately leak it, this will be our mock executor + let calls = Box::leak(Box::new(Mutex::new(Vec::new()))); + let executor = ForeignExecutor { + handle: unsafe { std::mem::transmute(calls as *const _) }, + }; + // Setup a callback to handle our handles + MOCK_EXECUTOR_INIT + .call_once(|| uniffi_foreign_executor_callback_set(mock_executor_callback)); + + Self { + calls, + executor: Some(executor), + } + } + + pub fn call_count(&self) -> usize { + self.calls.lock().unwrap().len() + } + + pub fn run_all_calls(&self) { + let mut calls = self.calls.lock().unwrap(); + for (_delay, callback, data) in calls.drain(..) { + callback.unwrap()(data); + } + } + + pub fn schedule_raw(&self, delay: u32, callback: RustTaskCallback, data: *const ()) { + let handle = self.executor.as_ref().unwrap().handle; + schedule_raw(handle, delay, callback, data) + } + + pub fn schedule( + &self, + delay: u32, + closure: F, + ) { + self.executor.as_ref().unwrap().schedule(delay, closure) + } + + pub fn run T + Send + panic::UnwindSafe + 'static, T>( + &self, + delay: u32, + closure: F, + ) -> impl Future { + self.executor.as_ref().unwrap().run(delay, closure) + } + + pub fn drop_executor(&mut self) { + self.executor = None; + } + } + + impl Default for MockExecutor { + fn default() -> Self { + Self::new() + } + } + + // Mock executor callback pushes calls to a ScheduledCalls + extern "C" fn mock_executor_callback( + executor: ForeignExecutorHandle, + delay: u32, + task: Option, + task_data: *const (), + ) { + unsafe { + let calls: *mut Mutex, *const ())>> = + std::mem::transmute(executor); + calls + .as_ref() + .unwrap() + .lock() + .unwrap() + .push((delay, task, task_data)); + } + } + + #[test] + fn test_schedule_raw() { + extern "C" fn callback(data: *const ()) { + unsafe { + *(data as *mut u32) += 1; + } + } + + let executor = MockExecutor::new(); + + let value: u32 = 0; + assert_eq!(executor.call_count(), 0); + + executor.schedule_raw(0, callback, &value as *const u32 as *const ()); + assert_eq!(executor.call_count(), 1); + assert_eq!(value, 0); + + executor.run_all_calls(); + assert_eq!(executor.call_count(), 0); + assert_eq!(value, 1); + } + + #[test] + fn test_schedule() { + let executor = MockExecutor::new(); + let value = Arc::new(AtomicU32::new(0)); + assert_eq!(executor.call_count(), 0); + + let value2 = value.clone(); + executor.schedule(0, move || { + value2.fetch_add(1, Ordering::Relaxed); + }); + assert_eq!(executor.call_count(), 1); + assert_eq!(value.load(Ordering::Relaxed), 0); + + executor.run_all_calls(); + assert_eq!(executor.call_count(), 0); + assert_eq!(value.load(Ordering::Relaxed), 1); + } + + #[derive(Default)] + struct MockWaker { + wake_count: AtomicU32, + } + + impl Wake for MockWaker { + fn wake(self: Arc) { + self.wake_count.fetch_add(1, Ordering::Relaxed); + } + } + + #[test] + fn test_run() { + let executor = MockExecutor::new(); + let mock_waker = Arc::new(MockWaker::default()); + let waker = Waker::from(mock_waker.clone()); + let mut context = Context::from_waker(&waker); + assert_eq!(executor.call_count(), 0); + + let mut future = executor.run(0, move || "test-return-value"); + assert_eq!(executor.call_count(), 1); + assert_eq!(Pin::new(&mut future).poll(&mut context), Poll::Pending); + assert_eq!(mock_waker.wake_count.load(Ordering::Relaxed), 0); + + executor.run_all_calls(); + assert_eq!(executor.call_count(), 0); + assert_eq!(mock_waker.wake_count.load(Ordering::Relaxed), 1); + assert_eq!( + Pin::new(&mut future).poll(&mut context), + Poll::Ready("test-return-value") + ); + } + + #[test] + fn test_drop() { + let mut executor = MockExecutor::new(); + + executor.schedule(0, || {}); + assert_eq!(executor.call_count(), 1); + + executor.drop_executor(); + assert_eq!(executor.call_count(), 2); + let calls = executor.calls.lock().unwrap(); + let drop_call = calls.last().unwrap(); + assert_eq!(drop_call.1, None); + } +} diff --git a/uniffi_core/src/ffi/mod.rs b/uniffi_core/src/ffi/mod.rs index 72bd7b1822..cb308ea4c1 100644 --- a/uniffi_core/src/ffi/mod.rs +++ b/uniffi_core/src/ffi/mod.rs @@ -7,6 +7,7 @@ pub mod ffidefault; pub mod foreignbytes; pub mod foreigncallbacks; +pub mod foreignexecutor; pub mod rustbuffer; pub mod rustcalls; pub mod rustfuture; @@ -14,6 +15,7 @@ pub mod rustfuture; pub use ffidefault::FfiDefault; pub use foreignbytes::*; pub use foreigncallbacks::*; +pub use foreignexecutor::*; pub use rustbuffer::*; pub use rustcalls::*; pub use rustfuture::*; diff --git a/uniffi_core/src/ffi_converter_impls.rs b/uniffi_core/src/ffi_converter_impls.rs index 3ce573eb9c..34c96dd782 100644 --- a/uniffi_core/src/ffi_converter_impls.rs +++ b/uniffi_core/src/ffi_converter_impls.rs @@ -475,6 +475,49 @@ unsafe impl> FfiConverter for std::sync::Arc { .concat_bool(false); } +/// FFI support for ForeignSchedulers +/// +/// These are passed over the FFI as opaque pointer-sized types representing the foreign executor. +/// The foreign bindings may use an actual pointer to the executor object, or a usized integer +/// handle. +unsafe impl FfiConverter for crate::ForeignExecutor { + type FfiType = crate::ForeignExecutorHandle; + + // Passing these back to the foreign bindings is currently not supported + fn lower(executor: Self) -> Self::FfiType { + executor.handle + } + + 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}"), + }; + } + + fn try_lift(executor: Self::FfiType) -> Result { + Ok(crate::ForeignExecutor::new(executor)) + } + + 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 ())) + } + + ffi_converter_default_return!(UT); + + const TYPE_ID_META: MetadataBuffer = + MetadataBuffer::from_code(metadata::codes::TYPE_FOREIGN_EXECUTOR); +} + /// Support `Result<>` via the FFI. /// /// This is currently supported for function returns. Lifting/lowering Result<> arguments is not diff --git a/uniffi_core/src/metadata.rs b/uniffi_core/src/metadata.rs index cddc8f0407..cd1d4474d7 100644 --- a/uniffi_core/src/metadata.rs +++ b/uniffi_core/src/metadata.rs @@ -63,6 +63,7 @@ pub mod codes { pub const TYPE_CUSTOM: u8 = 22; pub const TYPE_RESULT: u8 = 23; pub const TYPE_FUTURE: u8 = 24; + pub const TYPE_FOREIGN_EXECUTOR: u8 = 25; pub const TYPE_UNIT: u8 = 255; } diff --git a/uniffi_meta/src/lib.rs b/uniffi_meta/src/lib.rs index 51fe6eda08..d2b90bc928 100644 --- a/uniffi_meta/src/lib.rs +++ b/uniffi_meta/src/lib.rs @@ -196,6 +196,7 @@ pub enum Type { Bool, String, Duration, + ForeignExecutor, SystemTime, Enum { name: String, diff --git a/uniffi_meta/src/reader.rs b/uniffi_meta/src/reader.rs index 7b2a556766..2d176fca03 100644 --- a/uniffi_meta/src/reader.rs +++ b/uniffi_meta/src/reader.rs @@ -33,7 +33,7 @@ impl<'a> MetadataReader<'a> { // Read a top-level metadata item // - // This consumes self because MetadataReader is only intented to read a single item. + // This consumes self because MetadataReader is only intended to read a single item. fn read_metadata(mut self) -> Result { let value = self.read_u8()?; Ok(match value { @@ -99,6 +99,7 @@ impl<'a> MetadataReader<'a> { codes::TYPE_STRING => Type::String, codes::TYPE_DURATION => Type::Duration, codes::TYPE_SYSTEM_TIME => Type::SystemTime, + codes::TYPE_FOREIGN_EXECUTOR => Type::ForeignExecutor, codes::TYPE_RECORD => Type::Record { name: self.read_string()?, }, From 884403ddb0e994e1807b49119e9ec2090e2ac27e Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Sat, 8 Apr 2023 20:19:13 -0400 Subject: [PATCH 2/5] Reworked the future handling The new system for async functions is: 1. You make a call to a generated async function in the foreign bindings. 2. That function suspends itself, then makes a scaffolding call. In addition to the normal arguments, it passes a `ForeignExecutor` and callback. 3. Rust uses the `ForeignExecutor` to schedules polls of the Future until it's ready. Then invokes the callback. 4. The callback resumes the suspended async function from (2), closing the loop. This means that Rust manages the polling and the lifecycle of the Future/waker. This leads to a nice simplification of both the Rust and Foreign sides of the FFI. One complication here is the handling of unit returns over the FFI. With sync functions, a unit return can simply translate into a void return with the C-ABI. However, a void callback parameter is not valid, so we use a u8 as a placeholder value. Split up the error handling code so that we can use it for both sync and async functions. Added `FFIType.canonical_name()`. This is used by some of the languages to define future callback handler types. Re-enabled the futures tests. Hopefully we can get the docker image worker. Lowered the delay time for the tests to make them run faster. Give the Kotlin tests a bit more tolerance for slowness. I was finding they failed sometimes on my machine. --- .../futures/tests/bindings/test_futures.kts | 85 +- .../futures/tests/bindings/test_futures.py | 40 +- .../src/bindings/kotlin/gen_kotlin/mod.rs | 62 +- .../bindings/kotlin/templates/AsyncTypes.kt | 43 + .../src/bindings/kotlin/templates/Helpers.kt | 18 +- .../templates/NamespaceLibraryTemplate.kt | 26 - .../kotlin/templates/ObjectTemplate.kt | 37 +- .../kotlin/templates/RustFutureTemplate.kt | 19 - .../templates/TopLevelFunctionTemplate.kt | 69 +- .../src/bindings/kotlin/templates/Types.kt | 12 +- .../src/bindings/kotlin/templates/macros.kt | 171 +--- .../src/bindings/kotlin/templates/wrapper.kt | 1 - .../src/bindings/python/gen_python/mod.rs | 23 + .../bindings/python/templates/AsyncTypes.py | 36 + .../src/bindings/python/templates/Helpers.py | 25 +- .../templates/NamespaceLibraryTemplate.py | 32 +- .../python/templates/ObjectTemplate.py | 52 +- .../python/templates/PointerManager.py | 7 +- .../python/templates/RustFutureTemplate.py | 127 --- .../templates/TopLevelFunctionTemplate.py | 50 +- .../src/bindings/python/templates/Types.py | 4 + .../src/bindings/python/templates/macros.py | 11 +- .../src/bindings/python/templates/wrapper.py | 3 - .../src/bindings/ruby/gen_ruby/mod.rs | 3 + .../src/bindings/swift/gen_swift/mod.rs | 42 + .../bindings/swift/templates/AsyncTypes.swift | 29 + .../swift/templates/BridgingHeaderTemplate.h | 31 +- .../swift/templates/ErrorTemplate.swift | 5 + .../templates/ForeignExecutorTemplate.swift | 4 + .../bindings/swift/templates/Helpers.swift | 36 +- .../swift/templates/ObjectTemplate.swift | 99 +- .../templates/TopLevelFunctionTemplate.swift | 90 +- .../src/bindings/swift/templates/Types.swift | 4 + .../src/bindings/swift/templates/macros.swift | 15 +- uniffi_bindgen/src/interface/ffi.rs | 73 +- uniffi_bindgen/src/interface/function.rs | 32 +- uniffi_bindgen/src/interface/mod.rs | 39 +- uniffi_bindgen/src/interface/object.rs | 7 +- uniffi_bindgen/src/scaffolding/mod.rs | 4 + uniffi_core/src/ffi/foreignexecutor.rs | 4 + uniffi_core/src/ffi/rustcalls.rs | 2 +- uniffi_core/src/ffi/rustfuture.rs | 863 ++++++++---------- uniffi_core/src/ffi_converter_impls.rs | 48 +- uniffi_core/src/lib.rs | 41 + uniffi_macros/src/export/scaffolding.rs | 77 +- 45 files changed, 1194 insertions(+), 1307 deletions(-) create mode 100644 uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt delete mode 100644 uniffi_bindgen/src/bindings/kotlin/templates/RustFutureTemplate.kt create mode 100644 uniffi_bindgen/src/bindings/python/templates/AsyncTypes.py delete mode 100644 uniffi_bindgen/src/bindings/python/templates/RustFutureTemplate.py create mode 100644 uniffi_bindgen/src/bindings/swift/templates/AsyncTypes.swift diff --git a/fixtures/futures/tests/bindings/test_futures.kts b/fixtures/futures/tests/bindings/test_futures.kts index 29f466e89a..93d3a2e1cb 100644 --- a/fixtures/futures/tests/bindings/test_futures.kts +++ b/fixtures/futures/tests/bindings/test_futures.kts @@ -11,6 +11,18 @@ runBlocking { println("init time: ${time}ms") } +fun assertReturnsImmediately(actualTime: Long, testName: String) { + assert(actualTime <= 4) { + "unexpected $testName time: ${actualTime}ms" + } +} + +fun assertApproximateTime(actualTime: Long, expectedTime: Int, testName: String) { + assert(actualTime >= expectedTime && actualTime <= expectedTime + 100) { + "unexpected $testName time: ${actualTime}ms" + } +} + // Test `always_ready`. runBlocking { val time = measureTimeMillis { @@ -19,9 +31,7 @@ runBlocking { assert(result == true) } - print("always_ready: ${time}ms") - assert(time <= 4) - println(" ... ok") + assertReturnsImmediately(time, "always_ready") } // Test `void`. @@ -32,64 +42,54 @@ runBlocking { assert(result == Unit) } - print("void: ${time}ms") - assert(time <= 4) - println(" ... ok") + assertReturnsImmediately(time, "void") } // Test `sleep`. runBlocking { val time = measureTimeMillis { - sleep(2000U) + sleep(200U) } - print("sleep: ${time}ms") - assert(time > 2000 && time < 2100) - println(" ... ok") + assertApproximateTime(time, 200, "sleep") } // Test sequential futures. runBlocking { val time = measureTimeMillis { - val resultAlice = sayAfter(1000U, "Alice") - val resultBob = sayAfter(2000U, "Bob") + val resultAlice = sayAfter(100U, "Alice") + val resultBob = sayAfter(200U, "Bob") assert(resultAlice == "Hello, Alice!") assert(resultBob == "Hello, Bob!") } - print("sequential futures: ${time}ms") - assert(time > 3000 && time < 3100) - println(" ... ok") + assertApproximateTime(time, 300, "sequential future") } // Test concurrent futures. runBlocking { val time = measureTimeMillis { - val resultAlice = async { sayAfter(1000U, "Alice") } - val resultBob = async { sayAfter(2000U, "Bob") } + val resultAlice = async { sayAfter(100U, "Alice") } + val resultBob = async { sayAfter(200U, "Bob") } assert(resultAlice.await() == "Hello, Alice!") assert(resultBob.await() == "Hello, Bob!") } - print("concurrent futures: ${time}ms") - assert(time > 2000 && time < 2100) - println(" ... ok") + assertApproximateTime(time, 200, "concurrent future") } // Test async methods. runBlocking { val megaphone = newMegaphone() val time = measureTimeMillis { - val resultAlice = megaphone.sayAfter(2000U, "Alice") + val resultAlice = megaphone.sayAfter(200U, "Alice") assert(resultAlice == "HELLO, ALICE!") } - print("async methods: ${time}ms") - assert(time > 2000 && time < 2100) - println(" ... ok") + assertApproximateTime(time, 200, "async methods") } // Test async method returning optional object @@ -104,21 +104,19 @@ runBlocking { // Test with the Tokio runtime. runBlocking { val time = measureTimeMillis { - val resultAlice = sayAfterWithTokio(2000U, "Alice") + val resultAlice = sayAfterWithTokio(200U, "Alice") assert(resultAlice == "Hello, Alice (with Tokio)!") } - print("with tokio runtime: ${time}ms") - assert(time > 2000 && time < 2100) - println(" ... ok") + assertApproximateTime(time, 200, "with tokio runtime") } // Test fallible function/method. runBlocking { val time1 = measureTimeMillis { try { - val result = fallibleMe(false) + fallibleMe(false) assert(true) } catch (exception: Exception) { assert(false) // should never be reached @@ -126,12 +124,12 @@ runBlocking { } print("fallible function (with result): ${time1}ms") - assert(time1 < 10) + assert(time1 < 100) println(" ... ok") val time2 = measureTimeMillis { try { - val result = fallibleMe(true) + fallibleMe(true) assert(false) // should never be reached } catch (exception: Exception) { assert(true) @@ -139,14 +137,14 @@ runBlocking { } print("fallible function (with exception): ${time2}ms") - assert(time2 < 60) + assert(time2 < 100) println(" ... ok") val megaphone = newMegaphone() val time3 = measureTimeMillis { try { - val result = megaphone.fallibleMe(false) + megaphone.fallibleMe(false) assert(true) } catch (exception: Exception) { assert(false) // should never be reached @@ -154,12 +152,12 @@ runBlocking { } print("fallible method (with result): ${time3}ms") - assert(time3 < 10) + assert(time3 < 100) println(" ... ok") val time4 = measureTimeMillis { try { - val result = megaphone.fallibleMe(true) + megaphone.fallibleMe(true) assert(false) // should never be reached } catch (exception: Exception) { assert(true) @@ -167,7 +165,7 @@ runBlocking { } print("fallible method (with exception): ${time4}ms") - assert(time4 < 60) + assert(time4 < 100) println(" ... ok") } @@ -176,27 +174,24 @@ runBlocking { val time = measureTimeMillis { val result = newMyRecord("foo", 42U) - assert(result is MyRecord) assert(result.a == "foo") assert(result.b == 42U) } print("record: ${time}ms") - assert(time < 10) + assert(time < 100) println(" ... ok") } // Test a broken sleep. runBlocking { val time = measureTimeMillis { - brokenSleep(1000U, 0U) // calls the waker twice immediately - sleep(1000U) // wait for possible failure + brokenSleep(100U, 0U) // calls the waker twice immediately + sleep(100U) // wait for possible failure - brokenSleep(1000U, 1000U) // calls the waker a second time after 1s - sleep(2000U) // wait for possible failure + brokenSleep(100U, 100U) // calls the waker a second time after 1s + sleep(200U) // wait for possible failure } - print("broken sleep: ${time}ms") - assert(time > 5000 && time < 5100) - println(" ... ok") + assertApproximateTime(time, 500, "broken sleep") } diff --git a/fixtures/futures/tests/bindings/test_futures.py b/fixtures/futures/tests/bindings/test_futures.py index 0811ec437d..b83821ff0d 100644 --- a/fixtures/futures/tests/bindings/test_futures.py +++ b/fixtures/futures/tests/bindings/test_futures.py @@ -45,12 +45,12 @@ async def test(): def test_sequential_futures(self): async def test(): t0 = now() - result_alice = await say_after(1000, 'Alice') - result_bob = await say_after(2000, 'Bob') + result_alice = await say_after(100, 'Alice') + result_bob = await say_after(200, 'Bob') t1 = now() t_delta = (t1 - t0).total_seconds() - self.assertTrue(t_delta > 3 and t_delta < 3.1) + self.assertTrue(t_delta > 0.3 and t_delta < 0.31) self.assertEqual(result_alice, 'Hello, Alice!') self.assertEqual(result_bob, 'Hello, Bob!') @@ -58,8 +58,8 @@ async def test(): def test_concurrent_tasks(self): async def test(): - alice = asyncio.create_task(say_after(1000, 'Alice')) - bob = asyncio.create_task(say_after(2000, 'Bob')) + alice = asyncio.create_task(say_after(100, 'Alice')) + bob = asyncio.create_task(say_after(200, 'Bob')) t0 = now() result_alice = await alice @@ -67,7 +67,7 @@ async def test(): t1 = now() t_delta = (t1 - t0).total_seconds() - self.assertTrue(t_delta > 2 and t_delta < 2.1) + self.assertTrue(t_delta > 0.2 and t_delta < 0.21) self.assertEqual(result_alice, 'Hello, Alice!') self.assertEqual(result_bob, 'Hello, Bob!') @@ -77,11 +77,11 @@ def test_async_methods(self): async def test(): megaphone = new_megaphone() t0 = now() - result_alice = await megaphone.say_after(2000, 'Alice') + result_alice = await megaphone.say_after(200, 'Alice') t1 = now() t_delta = (t1 - t0).total_seconds() - self.assertTrue(t_delta > 2 and t_delta < 2.1) + self.assertTrue(t_delta > 0.2 and t_delta < 0.21) self.assertEqual(result_alice, 'HELLO, ALICE!') asyncio.run(test()) @@ -89,11 +89,11 @@ async def test(): def test_with_tokio_runtime(self): async def test(): t0 = now() - result_alice = await say_after_with_tokio(2000, 'Alice') + result_alice = await say_after_with_tokio(200, 'Alice') t1 = now() t_delta = (t1 - t0).total_seconds() - self.assertTrue(t_delta > 2 and t_delta < 2.1) + self.assertTrue(t_delta > 0.2 and t_delta < 0.21) self.assertEqual(result_alice, 'Hello, Alice (with Tokio)!') asyncio.run(test()) @@ -106,7 +106,7 @@ async def test(): t_delta = (t1 - t0).total_seconds() self.assertTrue(t_delta > 0 and t_delta < 0.1) - self.assertEqual(result.value, 42) + self.assertEqual(result, 42) try: result = await fallible_me(True) @@ -122,7 +122,7 @@ async def test(): t_delta = (t1 - t0).total_seconds() self.assertTrue(t_delta > 0 and t_delta < 0.1) - self.assertEqual(result.value, 42) + self.assertEqual(result, 42) try: result = await megaphone.fallible_me(True) @@ -141,5 +141,21 @@ async def test(): asyncio.run(test()) + def test_cancel(self): + async def test(): + # Create a task + task = asyncio.create_task(say_after(200, 'Alice')) + # Wait to ensure that the polling has started, then cancel the task + await asyncio.sleep(0.1) + task.cancel() + # Wait long enough for the Rust callback to fire. This shouldn't cause an exception, + # even though the task is cancelled. + await asyncio.sleep(0.2) + # Awaiting the task should result in a CancelledError. + with self.assertRaises(asyncio.CancelledError): + await task + + asyncio.run(test()) + if __name__ == '__main__': unittest.main() diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index a5cede607d..acfaadc0d3 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -323,6 +323,10 @@ impl CodeOracle for KotlinCodeOracle { FfiType::ForeignCallback => "ForeignCallback".to_string(), FfiType::ForeignExecutorHandle => "USize".to_string(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback".to_string(), + FfiType::FutureCallback { return_type } => { + format!("UniFfiFutureCallback{}", return_type.canonical_name(),) + } + FfiType::FutureCallbackData => "USize".to_string(), } } } @@ -369,6 +373,37 @@ pub mod filters { Ok(format!("{}.read", codetype.ffi_converter_name(oracle()))) } + pub fn error_handler(result_type: &ResultType) -> Result { + match &result_type.throws_type { + Some(error_type) => type_name(error_type), + None => Ok("NullCallStatusErrorHandler".into()), + } + } + + pub fn future_callback_handler(result_type: &ResultType) -> Result { + Ok(format!( + "UniFfiFutureCallbackHandler{}{}", + match &result_type.return_type { + Some(return_type) => return_type.type_label(oracle()), + None => "Void".into(), + }, + match &result_type.throws_type { + Some(throws_type) => format!("_{}", throws_type.type_label(oracle())), + None => "".into(), + }, + )) + } + + pub fn future_continuation_type(result_type: &ResultType) -> Result { + Ok(format!( + "Continuation<{}>", + match &result_type.return_type { + Some(t) => type_name(t)?, + None => "Unit".into(), + } + )) + } + pub fn render_literal( literal: &Literal, codetype: &impl CodeType, @@ -381,33 +416,6 @@ pub mod filters { Ok(oracle().ffi_type_label(type_)) } - /// Get the type that a type is lowered into. This is subtly different than `type_ffi`. - /// - /// For example, if we need to pre-allocate a Kotlin value that will store - /// an FFI lowered value, this method is your friend. - pub fn type_ffi_lowered(ffi_type: &FfiType) -> Result { - Ok(match ffi_type { - FfiType::Int8 => "Byte".into(), - FfiType::UInt8 => "Byte".into(), - FfiType::Int16 => "Short".into(), - FfiType::UInt16 => "Short".into(), - FfiType::Int32 => "Int".into(), - FfiType::UInt32 => "Int".into(), - FfiType::Int64 => "Long".into(), - FfiType::UInt64 => "Long".into(), - FfiType::Float32 => "Float".into(), - FfiType::Float64 => "Double".into(), - FfiType::RustArcPtr(_) => "Pointer".into(), - FfiType::RustBuffer(maybe_suffix) => match maybe_suffix { - Some(suffix) => format!("RustBuffer{suffix}"), - None => "RustBuffer".into(), - }, - FfiType::ForeignBytes => "ForeignBytes".into(), - FfiType::ForeignCallback => "ForeignCallback".into(), - FfiType::ForeignExecutorHandle | FfiType::ForeignExecutorCallback => todo!(), - }) - } - /// Get the idiomatic Kotlin rendering of a class name (for enums, records, errors, etc). pub fn class_name(nm: &str) -> Result { Ok(oracle().class_name(nm)) diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt b/uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt new file mode 100644 index 0000000000..e388e7e5ee --- /dev/null +++ b/uniffi_bindgen/src/bindings/kotlin/templates/AsyncTypes.kt @@ -0,0 +1,43 @@ +// Async return type handlers + +{# add imports that we use #} +{{ self.add_import("kotlin.coroutines.Continuation") }} +{{ self.add_import("kotlin.coroutines.resume") }} +{{ self.add_import("kotlin.coroutines.resumeWithException") }} + +{# We use these in the generated functions, which don't have access to add_import() -- might as well add it here #} +{{ self.add_import("kotlin.coroutines.suspendCoroutine") }} +{{ self.add_import("kotlinx.coroutines.coroutineScope") }} + + +// FFI type for callback handlers +{%- for callback_param in ci.iter_future_callback_params() %} +internal interface UniFfiFutureCallback{{ callback_param.canonical_name() }} : com.sun.jna.Callback { + // Note: callbackData is always 0. We could pass Rust a pointer/usize to represent the + // continuation, but with JNA it's easier to just store it in the callback handler. + fun invoke(_callbackData: USize, returnValue: {{ callback_param|ffi_type_name }}, callStatus: RustCallStatus.ByValue); +} +{%- endfor %} + +// Callback handlers for an async call. These are invoked by Rust when the future is ready. They +// lift the return value or error and resume the suspended function. +{%- for result_type in ci.iter_async_result_types() %} +{%- let callback_param = result_type.future_callback_param() %} + +internal class {{ result_type|future_callback_handler }}(val continuation: {{ result_type|future_continuation_type }}) + : UniFfiFutureCallback{{ callback_param.canonical_name() }} { + override fun invoke(_callbackData: USize, returnValue: {{ callback_param|ffi_type_name }}, callStatus: RustCallStatus.ByValue) { + try { + checkCallStatus({{ result_type|error_handler }}, callStatus) + {%- match result_type.return_type %} + {%- when Some(return_type) %} + continuation.resume({{ return_type|lift_fn }}(returnValue)) + {%- when None %} + continuation.resume(Unit) + {%- endmatch %} + } catch (e: Throwable) { + continuation.resumeWithException(e) + } + } +} +{%- endfor %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt index 0d6d485507..c928564a0b 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt @@ -3,19 +3,21 @@ // Error runtime. @Structure.FieldOrder("code", "error_buf") internal open class RustCallStatus : Structure() { - @JvmField var code: Int = 0 + @JvmField var code: Byte = 0 @JvmField var error_buf: RustBuffer.ByValue = RustBuffer.ByValue() + class ByValue: RustCallStatus(), Structure.ByValue + fun isSuccess(): Boolean { - return code == 0 + return code == 0.toByte() } fun isError(): Boolean { - return code == 1 + return code == 1.toByte() } fun isPanic(): Boolean { - return code == 2 + return code == 2.toByte() } } @@ -34,8 +36,14 @@ interface CallStatusErrorHandler { private inline fun rustCallWithError(errorHandler: CallStatusErrorHandler, callback: (RustCallStatus) -> U): U { var status = RustCallStatus(); val return_value = callback(status) + checkCallStatus(errorHandler, status) + return return_value +} + +// Check RustCallStatus and throw an error if the call wasn't successful +private fun checkCallStatus(errorHandler: CallStatusErrorHandler, status: RustCallStatus) { if (status.isSuccess()) { - return return_value + return } else if (status.isError()) { throw errorHandler.lift(status.error_buf) } else if (status.isPanic()) { diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt index 2bd2078ad3..c8fcde3e34 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/NamespaceLibraryTemplate.kt @@ -28,38 +28,12 @@ internal interface _UniFFILib : Library { {% endfor -%} } } - - {%- if ci.has_async_fns() %} - internal val FUTURE_WAKER_ENVIRONMENTS: ConcurrentHashMap> by lazy { - ConcurrentHashMap(8) - } - {%- endif %} } {% for func in ci.iter_ffi_function_definitions() -%} - {%- if func.is_async() %} - fun {{ func.name() }}( - {%- call kt::arg_list_ffi_decl(func) %} - ): RustFuture - - fun {{ func.name() }}_poll( - rustFuture: RustFuture, - waker: RustFutureWaker, - wakerEnv: RustFutureWakerEnvironmentCStructure?, - polledResult: {% match func.return_type() %}{% when Some with (return_type) %}{{ return_type|type_ffi_lowered }}{% when None %}Pointer{% endmatch %}ByReference, - _uniffi_out_err: RustCallStatus - ): Boolean - - fun {{ func.name() }}_drop( - `rust_future`: RustFuture, - _uniffi_out_err: RustCallStatus - ) - {%- else %} fun {{ func.name() }}( {%- call kt::arg_list_ffi_decl(func) %} ): {% match func.return_type() %}{% when Some with (return_type) %}{{ return_type.borrow()|ffi_type_name }}{% when None %}Unit{% endmatch %} - {%- endif -%} - {% endfor %} } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index 5e7a3bbd07..520fa4f20f 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -51,11 +51,42 @@ class {{ type_name }}( {% for meth in obj.methods() -%} {%- match meth.throws_type() -%} {%- when Some with (throwable) %} - @Throws({{ throwable|type_name }}::class) + @Throws({{ throwable|type_name }}::class) {%- else -%} {%- endmatch -%} - {%- if meth.is_async() -%} - {%- call kt::async_meth(meth) -%} + {%- if meth.is_async() %} + @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 %} { + // Create a new `CoroutineScope` for this operation, suspend the coroutine, and call the + // scaffolding function, passing it one of the callback handlers from `AsyncTypes.kt`. + // + // Make sure to retain a reference to the callback handler to ensure that it's not GCed before + // it's invoked + var callbackHolder: {{ func.result_type().borrow()|future_callback_handler }}? = null + return coroutineScope { + val scope = this + return@coroutineScope suspendCoroutine { continuation -> + try { + val callback = {{ meth.result_type().borrow()|future_callback_handler }}(continuation) + callbackHolder = callback + callWithPointer { thisPtr -> + rustCall { status -> + _UniFFILib.INSTANCE.{{ meth.ffi_func().name() }}( + thisPtr, + {% call kt::arg_list_lowered(meth) %} + FfiConverterForeignExecutor.lower(scope), + callback, + USize(0), + status, + ) + } + } + } catch (e: Exception) { + continuation.resumeWithException(e) + } + } + } + } {%- else -%} {%- match meth.return_type() -%} {%- when Some with (return_type) -%} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/RustFutureTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/RustFutureTemplate.kt deleted file mode 100644 index a3054adeae..0000000000 --- a/uniffi_bindgen/src/bindings/kotlin/templates/RustFutureTemplate.kt +++ /dev/null @@ -1,19 +0,0 @@ - -typealias RustFuture = Pointer - -interface RustFutureWaker: Callback { - fun callback(envCStructure: RustFutureWakerEnvironmentCStructure?) -} - -class RustFutureWakerEnvironment( - val rustFuture: RustFuture, - val continuation: Continuation, - val waker: RustFutureWaker, - val selfAsCStructure: RustFutureWakerEnvironmentCStructure, - val coroutineScope: CoroutineScope, -) - -@Structure.FieldOrder("hash") -class RustFutureWakerEnvironmentCStructure: Structure() { - @JvmField var hash: Int = 0 -} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/TopLevelFunctionTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/TopLevelFunctionTemplate.kt index edf5103f30..a4dfb6bcdc 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/TopLevelFunctionTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/TopLevelFunctionTemplate.kt @@ -1,28 +1,57 @@ {%- if func.is_async() %} - {%- match func.throws_type() -%} - {%- when Some with (throwable) %} - @Throws({{ throwable|type_name }}::class) - {%- else -%} - {%- endmatch %} +{%- match func.throws_type() -%} +{%- when Some with (throwable) %} +@Throws({{ throwable|type_name }}::class) +{%- else -%} +{%- endmatch %} + +@Suppress("ASSIGNED_BUT_NEVER_ACCESSED_VARIABLE") +suspend fun {{ func.name()|fn_name }}({%- call kt::arg_list_decl(func) -%}){% match func.return_type() %}{% when Some with (return_type) %} : {{ return_type|type_name }}{% when None %}{%- endmatch %} { + // Create a new `CoroutineScope` for this operation, suspend the coroutine, and call the + // scaffolding function, passing it one of the callback handlers from `AsyncTypes.kt`. + // + // Make sure to retain a reference to the callback handler to ensure that it's not GCed before + // it's invoked + var callbackHolder: {{ func.result_type().borrow()|future_callback_handler }}? = null + return coroutineScope { + val scope = this + return@coroutineScope suspendCoroutine { continuation -> + try { + val callback = {{ func.result_type().borrow()|future_callback_handler }}(continuation) + callbackHolder = callback + rustCall { status -> + _UniFFILib.INSTANCE.{{ func.ffi_func().name() }}( + {% call kt::arg_list_lowered(func) %} + FfiConverterForeignExecutor.lower(scope), + callback, + USize(0), + status, + ) + } + } catch (e: Exception) { + continuation.resumeWithException(e) + } + } + } +} - {%- call kt::async_func(func) -%} {%- else %} - {%- match func.throws_type() -%} - {%- when Some with (throwable) %} - @Throws({{ throwable|type_name }}::class) - {%- else -%} - {%- endmatch -%} +{%- match func.throws_type() -%} +{%- when Some with (throwable) %} +@Throws({{ throwable|type_name }}::class) +{%- else -%} +{%- endmatch -%} - {%- match func.return_type() -%} - {%- when Some with (return_type) %} +{%- match func.return_type() -%} +{%- when Some with (return_type) %} - fun {{ func.name()|fn_name }}({%- call kt::arg_list_decl(func) -%}): {{ return_type|type_name }} { - return {{ return_type|lift_fn }}({% call kt::to_ffi_call(func) %}) - } - {% when None %} +fun {{ func.name()|fn_name }}({%- call kt::arg_list_decl(func) -%}): {{ return_type|type_name }} { + return {{ return_type|lift_fn }}({% call kt::to_ffi_call(func) %}) +} +{% when None %} - fun {{ func.name()|fn_name }}({% call kt::arg_list_decl(func) %}) = - {% call kt::to_ffi_call(func) %} +fun {{ func.name()|fn_name }}({% call kt::arg_list_decl(func) %}) = + {% call kt::to_ffi_call(func) %} - {% endmatch %} +{% endmatch %} {%- endif %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt index 014e95eef5..6dbac3570e 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Types.kt @@ -96,16 +96,6 @@ {%- endmatch %} {%- endfor %} -// Add imports for async functions. This doesn't really have anything to do with types, but -// add_import() is only available in Types.kt. {%- if ci.has_async_fns() %} -{{ self.add_import("kotlin.coroutines.Continuation") }} -{{ self.add_import("kotlin.coroutines.resume") }} -{{ self.add_import("kotlin.coroutines.resumeWithException") }} -{{ self.add_import("kotlin.coroutines.suspendCoroutine") }} -{{ self.add_import("kotlinx.coroutines.coroutineScope") }} -{{ self.add_import("kotlinx.coroutines.CoroutineScope") }} -{{ self.add_import("kotlinx.coroutines.launch") }} -{{ self.add_import("kotlinx.coroutines.sync.Semaphore") }} -{{ self.add_import("kotlinx.coroutines.sync.withPermit") }} +{% include "AsyncTypes.kt" %} {%- endif %} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt b/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt index fc2c91b997..7e373c8c59 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt @@ -1,7 +1,7 @@ {# // Template to call into rust. Used in several places. // Variable names in `arg_list_decl` should match up with arg lists -// passed to rust via `_arg_list_ffi_call` +// passed to rust via `arg_list_lowered` #} {%- macro to_ffi_call(func) -%} @@ -11,7 +11,7 @@ {%- else %} rustCall() {%- endmatch %} { _status -> - _UniFFILib.INSTANCE.{{ func.ffi_func().name() }}({% call _arg_list_ffi_call(func) -%}{% if func.arguments().len() > 0 %},{% endif %} _status) + _UniFFILib.INSTANCE.{{ func.ffi_func().name() }}({% call arg_list_lowered(func) -%} _status) } {%- endmacro -%} @@ -23,14 +23,15 @@ rustCall() {%- endmatch %} { _status -> _UniFFILib.INSTANCE.{{ func.ffi_func().name() }}( - {{- prefix }}, {% call _arg_list_ffi_call(func) %}{% if func.arguments().len() > 0 %}, {% endif %} _status) + {{- prefix }}, + {% call arg_list_lowered(func) %} + _status) } {%- endmacro %} -{%- macro _arg_list_ffi_call(func) %} +{%- macro arg_list_lowered(func) %} {%- for arg in func.arguments() %} - {{- arg|lower_fn }}({{ arg.name()|var_name }}) - {%- if !loop.last %}, {% endif %} + {{- arg|lower_fn }}({{ arg.name()|var_name }}), {%- endfor %} {%- endmacro -%} @@ -80,161 +81,3 @@ fun {{ func.name()|fn_name }}( {%- call arg_list_ffi_decl(func) %} ){%- match func.return_type() -%}{%- when Some with (type_) %}: {{ type_|ffi_type_name }}{% when None %}: Unit{% endmatch %} {% endmacro %} -{%- macro async_func(func) -%} - {%- call _async_func_or_method(func, false) -%} -{%- endmacro -%} - -// Asny function and method. -{%- macro async_meth(meth) -%} - {% call _async_func_or_method(meth, true) -%} -{%- endmacro -%} - -{%- macro _async_func_or_method(func, is_meth) -%} -{% if is_meth %}override {% endif -%} -suspend fun {{ func.name()|fn_name }}( - {%- if is_meth -%} - {%- call arg_list_protocol(func) %} - {%- else -%} - {%- call arg_list_decl(func) %} - {%- endif -%} -) -{%- match func.return_type() %} -{%- when Some with (return_type) -%} - : {{ return_type|type_name }} -{%- when None %} -{%- endmatch %} { - {# JNA defines callbacks as a class with a `callback` method -#} - class Waker: RustFutureWaker { - private val lock = Semaphore(1) - - override fun callback(envCStructure: RustFutureWakerEnvironmentCStructure?) { - if (envCStructure == null) { - return; - } - - val hash = envCStructure.hash - val env = _UniFFILib.FUTURE_WAKER_ENVIRONMENTS.get(hash) - - if (env == null) { - {# The future has been resolved already -#} - return - } - - {# Schedule a poll -#} - env.coroutineScope.launch { - {# Run one poll at a time -#} - lock.withPermit { - if (!_UniFFILib.FUTURE_WAKER_ENVIRONMENTS.containsKey(hash)) { - {# The future has been resolved by a previous call -#} - return@withPermit - } - - {# Cast the continuation to its appropriate type -#} - @Suppress("UNCHECKED_CAST") - val continuation = {% match func.return_type() -%} - {%- when Some with (return_type) -%} - env.continuation as Continuation<{{ return_type|type_name }}> - {%- when None -%} - env.continuation as Continuation - {%- endmatch %} - - {# Declare the `polledResult` variable: `_T_ByReference` where -#} - {#- `_T_` is the return type, or `PointerByReference` -#} - val polledResult = {% match func.ffi_func().return_type() -%} - {%- when Some with (return_type) -%} - {{ return_type|type_ffi_lowered }} - {%- when None -%} - Pointer - {%- endmatch %}ByReference() - - {# Try to poll, catch exceptions if the future has thrown -#} - try { - {# Poll the future! -#} - val isReady = {% match func.throws_type() -%} - {%- when Some with (error) -%} - rustCallWithError({{ error|type_name }}) - {%- when None -%} - rustCall() - {%- endmatch %} - { _status -> - _UniFFILib.INSTANCE.{{ func.ffi_func().name() }}_poll( - env.rustFuture, - env.waker, - env.selfAsCStructure, - polledResult, - _status - ) - } - - {# If the future ready? -#} - if (isReady) { - {# Resume the continuation with the lifted value if any, `Unit` otherwise -#} - continuation.resume( - {% match func.return_type() -%} - {%- when Some with (return_type) -%} - {{ return_type|lift_fn}}(polledResult.getValue()) - {%- when None -%} - Unit - {%- endmatch %} - ) - - {# Clean up the environment and the future -#} - _UniFFILib.FUTURE_WAKER_ENVIRONMENTS.remove(hash) - rustCall() { _status -> - _UniFFILib.INSTANCE.{{ func.ffi_func().name() }}_drop(env.rustFuture, _status) - } - } - } catch (exception: Exception) { - {# Resume the continuation with the caught exception -#} - continuation.resumeWithException(exception) - - {# Clean up the environment and the future -#} - _UniFFILib.FUTURE_WAKER_ENVIRONMENTS.remove(hash) - rustCall() { _status -> - _UniFFILib.INSTANCE.{{ func.ffi_func().name() }}_drop(env.rustFuture, _status) - } - } - } - } - } - } - - {# Declare the `result` variable that will receive the result of the future -#} - val result: {% match func.return_type() %}{% when Some with (return_type) %}{{ return_type|type_name }}{% when None %}Unit{% endmatch %} - - {# Get the coroutine scope -#} - coroutineScope { - {# Suspend the coroutine, and get a continuation -#} - result = suspendCoroutine< - {%- match func.return_type() %} - {%- when Some with (return_type) -%} - {{ return_type|type_name }} - {%- when None -%} - Unit - {%- endmatch -%} - > { continuation -> - {# Create the future -#} - val rustFuture = {% if is_meth -%} - callWithPointer { - {% call to_ffi_call_with_prefix("it", func) %} - } - {%- else -%} - {%- call to_ffi_call(func) -%} - {%- endif %} - - {# Create the waker environment -#} - val env = RustFutureWakerEnvironment(rustFuture, continuation, Waker(), RustFutureWakerEnvironmentCStructure(), this) - val envHash = env.hashCode() - env.selfAsCStructure.hash = envHash - - _UniFFILib.FUTURE_WAKER_ENVIRONMENTS.put(envHash, env) - - {# Call the waker to schedule a poll -#} - env.waker.callback(env.selfAsCStructure) - } - } - - {# We have a result if no exception caught, let's return it! -#} - return result -} -{%- endmacro -%} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt index 1db5639cc4..4cc3fd8ad5 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt @@ -33,7 +33,6 @@ import java.util.concurrent.ConcurrentHashMap {%- endfor %} {% include "RustBufferTemplate.kt" %} -{% if ci.has_async_fns() %}{% include "RustFutureTemplate.kt" %}{% endif %} {% 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 f294d6ed7a..bb0a1db9f1 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -367,6 +367,13 @@ impl CodeOracle for PythonCodeOracle { // Pointer to an `asyncio.EventLoop` instance FfiType::ForeignExecutorHandle => "ctypes.c_size_t".to_string(), FfiType::ForeignExecutorCallback => "UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T".to_string(), + FfiType::FutureCallback { return_type } => { + format!( + "uniffi_future_callback_t({})", + self.ffi_type_label(return_type), + ) + } + FfiType::FutureCallbackData => "ctypes.c_size_t".to_string(), } } } @@ -409,6 +416,22 @@ pub mod filters { Ok(format!("{}.write", ffi_converter_name(codetype)?)) } + // Name of the callback function we pass to Rust to complete an async call + pub fn async_callback_fn(result_type: &ResultType) -> Result { + let return_string = match &result_type.return_type { + Some(t) => t.canonical_name().to_snake_case(), + None => "void".into(), + }; + let throws_string = match &result_type.throws_type { + Some(t) => t.canonical_name().to_snake_case(), + None => "void".into(), + }; + Ok(format!( + "uniffi_async_callback_{}__{}", + return_string, throws_string + )) + } + pub fn literal_py( literal: &Literal, codetype: &impl CodeType, diff --git a/uniffi_bindgen/src/bindings/python/templates/AsyncTypes.py b/uniffi_bindgen/src/bindings/python/templates/AsyncTypes.py new file mode 100644 index 0000000000..202b60610e --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/templates/AsyncTypes.py @@ -0,0 +1,36 @@ +# Callback handlers for async returns + +UniFfiPyFuturePointerManager = UniFfiPointerManager() + +# Callback handlers for an async calls. These are invoked by Rust when the future is ready. They +# lift the return value or error and resolve the future, causing the async function to resume. +{%- for result_type in ci.iter_async_result_types() %} +@uniffi_future_callback_t( + {%- match result_type.return_type -%} + {%- when Some(return_type) -%} + {{ return_type.ffi_type().borrow()|ffi_type_name }} + {%- when None -%} + ctypes.c_uint8 + {%- endmatch -%} +) +def {{ result_type|async_callback_fn }}(future_ptr, result, call_status): + future = UniFfiPyFuturePointerManager.release_pointer(future_ptr) + if future.cancelled(): + return + try: + {%- match result_type.throws_type %} + {%- when Some(throws_type) %} + uniffi_check_call_status({{ throws_type|ffi_converter_name }}, call_status) + {%- when None %} + uniffi_check_call_status(None, call_status) + {%- endmatch %} + + {%- match result_type.return_type %} + {%- when Some(return_type) %} + future.set_result({{ return_type|lift_fn }}(result)) + {%- when None %} + future.set_result(None) + {%- endmatch %} + except BaseException as e: + future.set_exception(e) +{%- endfor %} diff --git a/uniffi_bindgen/src/bindings/python/templates/Helpers.py b/uniffi_bindgen/src/bindings/python/templates/Helpers.py index 6525bc3466..2eba809794 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Helpers.py +++ b/uniffi_bindgen/src/bindings/python/templates/Helpers.py @@ -41,8 +41,31 @@ def rust_call_with_error(error_ffi_converter, fn, *args): args_with_error = args + (ctypes.byref(call_status),) result = fn(*args_with_error) + uniffi_check_call_status(error_ffi_converter, call_status) + return result + +def rust_call_async(scaffolding_fn, callback_fn, *args): + # Call the scaffolding function, passing it a callback handler for `AsyncTypes.py` and a pointer + # to a python Future object. The async function then awaits the Future. + uniffi_eventloop = asyncio.get_running_loop() + uniffi_py_future = uniffi_eventloop.create_future() + uniffi_call_status = RustCallStatus(code=RustCallStatus.CALL_SUCCESS, error_buf=RustBuffer(0, 0, None)) + scaffolding_fn(*args, + FfiConverterForeignExecutor._pointer_manager.new_pointer(uniffi_eventloop), + callback_fn, + # Note: It's tempting to skip the pointer manager and just use a `py_object` pointing to a + # local variable like we do in Swift. However, Python doesn't use cooperative cancellation + # -- asyncio can cancel a task at anytime. This means if we use a local variable, the Rust + # callback could fire with a dangling pointer. + UniFfiPyFuturePointerManager.new_pointer(uniffi_py_future), + ctypes.byref(uniffi_call_status), + ) + uniffi_check_call_status(None, uniffi_call_status) + return uniffi_py_future + +def uniffi_check_call_status(error_ffi_converter, call_status): if call_status.code == RustCallStatus.CALL_SUCCESS: - return result + pass elif call_status.code == RustCallStatus.CALL_ERROR: if error_ffi_converter is None: call_status.error_buf.free() diff --git a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py index 1a5d2e89b6..ab6ba409a3 100644 --- a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py @@ -21,6 +21,12 @@ """ UNIFFI_RUST_TASK = ctypes.CFUNCTYPE(None, ctypes.c_void_p) +def uniffi_future_callback_t(return_type): + """ + Factory function to create callback function types for async functions + """ + return ctypes.CFUNCTYPE(None, ctypes.c_size_t, return_type, RustCallStatus) + from pathlib import Path def loadIndirect(): @@ -71,34 +77,8 @@ def uniffi_check_api_checksums(lib): _UniFFILib = loadIndirect() {%- for func in ci.iter_ffi_function_definitions() %} -{%- if func.is_async() %} - -_UniFFILib.{{ func.name() }}.argtypes = ( - {%- call py::arg_list_ffi_decl(func) -%} -) -_UniFFILib.{{ func.name() }}.restype = ctypes.POINTER(RustFuture) - -_UniFFILib.{{ func.name() }}_poll.argtypes = ( - ctypes.POINTER(RustFuture), - FUTURE_WAKER_T, - FUTURE_WAKER_ENVIRONMENT_T, - {% match func.return_type() %}{% when Some with (type_) %}ctypes.POINTER({{ type_|ffi_type_name }}){% when None %}ctypes.c_void_p{% endmatch %}, - {%- if func.has_rust_call_status_arg() %}ctypes.POINTER(RustCallStatus), {% endif %} -) -_UniFFILib.{{ func.name() }}_poll.restype = ctypes.c_bool - -_UniFFILib.{{ func.name() }}_drop.argtypes = ( - ctypes.POINTER(RustFuture), - ctypes.POINTER(RustCallStatus), -) -_UniFFILib.{{ func.name() }}_drop.restype = None - -{%- else %} - _UniFFILib.{{ func.name() }}.argtypes = ( {%- call py::arg_list_ffi_decl(func) -%} ) _UniFFILib.{{ func.name() }}.restype = {% match func.return_type() %}{% when Some with (type_) %}{{ type_|ffi_type_name }}{% when None %}None{% endmatch %} - -{%- endif %} {%- endfor %} diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index 82572320e5..d8c4ea7aa2 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -35,54 +35,16 @@ def {{ cons.name()|fn_name }}(cls, {% call py::arg_list_decl(cons) %}): {% for meth in obj.methods() -%} {% if meth.is_async() %} + async def {{ meth.name()|fn_name }}(self, {% call py::arg_list_decl(meth) %}): {%- call py::setup_args_extra_indent(meth) %} - {#- Get the `RustFuture`. -#} - rust_future = {% call py::to_ffi_call_with_prefix("self._pointer", meth) %} - future = None - - def trampoline() -> (FuturePoll, any): - nonlocal rust_future - - {% match meth.ffi_func().return_type() -%} - {%- when Some with (return_type) -%} - polled_result = {{ return_type|ffi_type_name }}() - polled_result_ref = ctypes.byref(polled_result) - {% when None %} - polled_result_ref = ctypes.c_void_type() - {% endmatch %} - - is_ready = {% match meth.throws_type() -%} - {%- when Some with (error) -%} - rust_call_with_error({{ error|ffi_converter_name }}, - {%- when None -%} - rust_call( - {%- endmatch %} - _UniFFILib.{{ meth.ffi_func().name() }}_poll, - rust_future, - future._future_ffi_waker(), - ctypes.c_void_p(), - polled_result_ref, - ) - - if is_ready is True: - result = {% match meth.return_type() %}{% when Some with (return_type) %}{{ return_type|lift_fn }}(polled_result){% when None %}None{% endmatch %} - - return (FuturePoll.DONE, result) - else: - return (FuturePoll.PENDING, None) - - {# Create our own Python `Future` and poll it. -#} - future = Future(trampoline) - future.init() - - {# Let's wait on it. -#} - result = await future - - {# Drop the `rust_future`. -#} - rust_call(_UniFFILib.{{ meth.ffi_func().name() }}_drop, rust_future) + return await rust_call_async( + _UniFFILib.{{ func.ffi_func().name() }}, + {{ func.result_type().borrow()|async_callback_fn }}, + self._pointer, + {% call py::arg_list_lowered(func) %} + ) - return result {% else %} {%- match meth.return_type() -%} diff --git a/uniffi_bindgen/src/bindings/python/templates/PointerManager.py b/uniffi_bindgen/src/bindings/python/templates/PointerManager.py index df5df842e0..932821f010 100644 --- a/uniffi_bindgen/src/bindings/python/templates/PointerManager.py +++ b/uniffi_bindgen/src/bindings/python/templates/PointerManager.py @@ -22,7 +22,10 @@ def new_pointer(self, obj): return id(obj) def release_pointer(self, address): - ctypes.pythonapi.Py_DecRef(ctypes.cast(address, ctypes.py_object)) + 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 @@ -52,7 +55,7 @@ def new_pointer(self, obj): def release_pointer(self, handle): with self._lock: - del self._map[handle] + return self._map.pop(handle) def lookup(self, handle): with self._lock: diff --git a/uniffi_bindgen/src/bindings/python/templates/RustFutureTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RustFutureTemplate.py deleted file mode 100644 index ef461247e3..0000000000 --- a/uniffi_bindgen/src/bindings/python/templates/RustFutureTemplate.py +++ /dev/null @@ -1,127 +0,0 @@ - -{# See `uniffi::RustFutureForeignWakerEnvironment` #} -FUTURE_WAKER_ENVIRONMENT_T = ctypes.c_void_p -{# See `uniffi::RustFutureForeignWakerFunction` #} -FUTURE_WAKER_T = ctypes.CFUNCTYPE(ctypes.c_uint8, FUTURE_WAKER_ENVIRONMENT_T) - -# `RustFuture` is an opaque type. -class RustFuture(ctypes.Structure): - pass - -class FuturePoll(enum.Enum): - PENDING = 0 - DONE = 1 - -class Future: - def __init__(self, future_trampoline: any): - self._asyncio_future_blocking = False - self._loop = asyncio.get_event_loop() - self._state = FuturePoll.PENDING - self._result = None - self._exception = None - self._exception_traceback = None - self._waker = None - self._ffi_waker = None - self._callbacks = [] - - # The foreign waker, which will be called by Rust. - def waker(_env: ctypes.c_void_p): - def scheduled_waker(): - try: - state, result = (future_trampoline)() - - if state == FuturePoll.DONE: - self.set_result(result) - except BaseException as exception: - self.set_exception(exception) - - # Ask the executor to schedule to poll the future. - self._loop.call_soon_threadsafe(scheduled_waker) - - return 0 - - self._waker = waker - self._ffi_waker = FUTURE_WAKER_T(waker) - - def init(self): - # Poll the future. - (self._waker)(ctypes.c_void_p()) - - def _future_waker(self) -> any: - return self._waker - - def _future_ffi_waker(self) -> FUTURE_WAKER_T: - return self._ffi_waker - - def done(self) -> bool: - return self._state == FuturePoll.DONE - - def result(self) -> any: - if self._state != FuturePoll.DONE: - raise RuntimeError('Result is not ready') - - if self._exception is not None: - raise self._exception.with_traceback(self._exception_traceback) - - return self._result - - def set_result(self, result: any): - if self._state != FuturePoll.PENDING: - raise RuntimeError('This future has already been resolved') - - self._result = result - self._state = FuturePoll.DONE - self.__schedule_callbacks() - - def set_exception(self, exception: any): - self._exception = exception - self._exception_traceback = exception.__traceback__ - self._state = FuturePoll.DONE - self.__schedule_callbacks() - - def __schedule_callbacks(self): - callbacks = self._callbacks[:] - - if not callbacks: - return - - self._callbacks[:] = [] - - for callback, context in callbacks: - self._loop.call_soon_threadsafe(callback, self, context=context) - - def add_done_callback(self, callback, *, context=None): - if self._state != FuturePoll.PENDING: - self._loop.call_soon_threadsafe(callback, self, context=context) - else: - if context is None: - context = contextvars.copy_context() - - self._callbacks.append((callback, context)) - - def remove_done_callback(self, callback): - filtered_callbacks = [(other_callback, context) - for (other_callback, context) in self._callbacks - if other_callback != callback] - removed_count = len(self._callbacks) - len(filtered_callbacks) - - if removed_count: - self._callbacks[:] = filtered_callbacks - - return removed_count - - def cancel(self, msg=None): - pass # TODO - - def __await__(self): - if not self.done(): - self._asyncio_future_blocking = True - yield self - - if not self.done(): - raise RuntimeError('await wasn\'t used with future') - - return self.result() - - __iter__ = __await__ - diff --git a/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py b/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py index c2eae26585..1175d1f2ab 100644 --- a/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py @@ -2,52 +2,12 @@ async def {{ func.name()|fn_name }}({%- call py::arg_list_decl(func) -%}): {%- call py::setup_args(func) %} - {# Get the `RustFuture`. -#} - rust_future = {% call py::to_ffi_call(func) %} - future = None + return await rust_call_async( + _UniFFILib.{{ func.ffi_func().name() }}, + {{ func.result_type().borrow()|async_callback_fn }}, + {% call py::arg_list_lowered(func) %} + ) - def trampoline() -> (FuturePoll, any): - nonlocal rust_future - - {% match func.ffi_func().return_type() -%} - {%- when Some with (return_type) -%} - polled_result = {{ return_type|ffi_type_name }}() - polled_result_ref = ctypes.byref(polled_result) - {%- when None -%} - polled_result_ref = ctypes.c_void_p() - {%- endmatch %} - - is_ready = {% match func.throws_type() -%} - {%- when Some with (error) -%} - rust_call_with_error({{ error|ffi_converter_name }}, - {%- when None -%} - rust_call( - {%- endmatch %} - _UniFFILib.{{ func.ffi_func().name() }}_poll, - rust_future, - future._future_ffi_waker(), - ctypes.c_void_p(), - polled_result_ref, - ) - - if is_ready is True: - result = {% match func.return_type() %}{% when Some with (return_type) %}{{ return_type|lift_fn }}(polled_result){% when None %}None{% endmatch %} - - return (FuturePoll.DONE, result) - else: - return (FuturePoll.PENDING, None) - - {# Create our own Python `Future` and poll it. -#} - future = Future(trampoline) - future.init() - - {# Let's wait on it. -#} - result = await future - - {# Drop the `rust_future`. -#} - rust_call(_UniFFILib.{{ func.ffi_func().name() }}_drop, rust_future) - - return result {%- else %} {%- match func.return_type() -%} {%- when Some with (return_type) %} diff --git a/uniffi_bindgen/src/bindings/python/templates/Types.py b/uniffi_bindgen/src/bindings/python/templates/Types.py index 7500f21679..e73ac9ace5 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Types.py +++ b/uniffi_bindgen/src/bindings/python/templates/Types.py @@ -94,3 +94,7 @@ {%- else %} {%- endmatch %} {%- endfor %} + +{%- if ci.has_async_fns() %} +{%- include "AsyncTypes.py" %} +{%- endif %} diff --git a/uniffi_bindgen/src/bindings/python/templates/macros.py b/uniffi_bindgen/src/bindings/python/templates/macros.py index 20f885cdbd..6029c463b4 100644 --- a/uniffi_bindgen/src/bindings/python/templates/macros.py +++ b/uniffi_bindgen/src/bindings/python/templates/macros.py @@ -1,7 +1,7 @@ {# // Template to call into rust. Used in several places. // Variable names in `arg_list_decl` should match up with arg lists -// passed to rust via `_arg_list_ffi_call` +// passed to rust via `arg_list_lowered` #} {%- macro to_ffi_call(func) -%} @@ -12,7 +12,7 @@ rust_call( {%- endmatch -%} _UniFFILib.{{ func.ffi_func().name() }}, - {%- call _arg_list_ffi_call(func) -%} + {%- call arg_list_lowered(func) -%} ) {%- endmacro -%} @@ -26,11 +26,11 @@ {%- endmatch -%} _UniFFILib.{{ func.ffi_func().name() }}, {{- prefix }}, - {%- call _arg_list_ffi_call(func) -%} + {%- call arg_list_lowered(func) -%} ) {%- endmacro -%} -{%- macro _arg_list_ffi_call(func) %} +{%- macro arg_list_lowered(func) %} {%- for arg in func.arguments() %} {{ arg|lower_fn }}({{ arg.name()|var_name }}) {%- if !loop.last %},{% endif %} @@ -61,7 +61,8 @@ {%- for arg in func.arguments() %} {{ arg.type_().borrow()|ffi_type_name }}, {%- endfor %} - {%- if func.has_rust_call_status_arg() %}ctypes.POINTER(RustCallStatus),{% endif %} + {%- if func.has_rust_call_status_arg() %} + ctypes.POINTER(RustCallStatus),{% endif %} {% endmacro -%} {# diff --git a/uniffi_bindgen/src/bindings/python/templates/wrapper.py b/uniffi_bindgen/src/bindings/python/templates/wrapper.py index 668655f31b..061a967c66 100644 --- a/uniffi_bindgen/src/bindings/python/templates/wrapper.py +++ b/uniffi_bindgen/src/bindings/python/templates/wrapper.py @@ -41,9 +41,6 @@ {% include "Helpers.py" %} {% include "PointerManager.py" %} {% include "RustBufferHelper.py" %} -{%- if ci.has_async_fns() %} -{% include "RustFutureTemplate.py" %} -{%- endif %} # Contains loading, initialization code, # and the FFI Function declarations in a com.sun.jna.Library. diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs index 321e9fd604..e4654da947 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -102,6 +102,9 @@ mod filters { FfiType::ForeignExecutorHandle => { unimplemented!("Foreign executors are not implemented") } + FfiType::FutureCallback { .. } | FfiType::FutureCallbackData => { + unimplemented!("Async functions are not implemented") + } }) } diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs index 9411f3f1b2..544b593db3 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -378,6 +378,11 @@ impl CodeOracle for SwiftCodeOracle { FfiType::ForeignCallback => "ForeignCallback _Nonnull".into(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback _Nonnull".into(), FfiType::ForeignExecutorHandle => "size_t".into(), + FfiType::FutureCallback { return_type } => format!( + "UniFfiFutureCallback{} _Nonnull", + return_type.canonical_name() + ), + FfiType::FutureCallbackData => "void* _Nonnull".into(), } } } @@ -452,6 +457,11 @@ pub mod filters { FfiType::ForeignCallback => "ForeignCallback _Nonnull".into(), FfiType::ForeignExecutorHandle => "Int".into(), FfiType::ForeignExecutorCallback => "ForeignExecutorCallback _Nonnull".into(), + FfiType::FutureCallback { return_type } => format!( + "UniFfiFutureCallback{} _Nonnull", + return_type.canonical_name() + ), + FfiType::FutureCallbackData => "UnsafeMutableRawPointer".into(), }) } @@ -474,4 +484,36 @@ pub mod filters { pub fn enum_variant_swift(nm: &str) -> Result { Ok(oracle().enum_variant_name(nm)) } + + pub fn error_handler(result: &ResultType) -> Result { + Ok(match &result.throws_type { + Some(t) => format!("{}.lift", ffi_converter_name(t)?), + None => "nil".into(), + }) + } + + /// Name of the callback function to handle an async result + pub fn future_callback(result: &ResultType) -> Result { + Ok(format!( + "uniffiFutureCallbackHandler{}{}", + match &result.return_type { + Some(t) => t.canonical_name(), + None => "Void".into(), + }, + match &result.throws_type { + Some(t) => t.canonical_name(), + None => "".into(), + } + )) + } + + pub fn future_continuation_type(result: &ResultType) -> Result { + Ok(format!( + "CheckedContinuation<{}, Error>", + match &result.return_type { + Some(return_type) => type_name(return_type)?, + None => "()".into(), + } + )) + } } diff --git a/uniffi_bindgen/src/bindings/swift/templates/AsyncTypes.swift b/uniffi_bindgen/src/bindings/swift/templates/AsyncTypes.swift new file mode 100644 index 0000000000..077ea4fcee --- /dev/null +++ b/uniffi_bindgen/src/bindings/swift/templates/AsyncTypes.swift @@ -0,0 +1,29 @@ +// Callbacks for async functions + +// 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. +{%- for result_type in ci.iter_async_result_types() %} +fileprivate func {{ result_type|future_callback }}( + rawContinutation: UnsafeRawPointer, + returnValue: {{ result_type.future_callback_param().borrow()|type_ffi_lowered }}, + callStatus: RustCallStatus) { + + let continuation = rawContinutation.bindMemory( + to: {{ result_type|future_continuation_type }}.self, + capacity: 1 + ) + + do { + try uniffiCheckCallStatus(callStatus: callStatus, errorHandler: {{ result_type|error_handler }}) + {%- match result_type.return_type %} + {%- when Some(return_type) %} + continuation.pointee.resume(returning: try {{ return_type|lift_fn }}(returnValue)) + {%- when None %} + continuation.pointee.resume(returning: ()) + {%- endmatch %} + } catch let error { + continuation.pointee.resume(throwing: error) + } +} + +{%- endfor %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h index f277036979..92a8dd049d 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h +++ b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h @@ -33,6 +33,7 @@ typedef int32_t (*ForeignCallback)(uint64_t, int32_t, const uint8_t *_Nonnull, i // Task defined in Rust that Swift executes typedef void (*UniFfiRustTaskCallback)(const void * _Nullable); + // Callback to execute Rust tasks using a Swift Task // // Args: @@ -48,12 +49,6 @@ typedef struct ForeignBytes const uint8_t *_Nullable data; } ForeignBytes; -// `RustFuture` is an opaque type. -typedef struct RustFuture RustFuture; - -typedef void rust_future_waker_environment_t; -typedef void (*rust_future_waker_t)(rust_future_waker_environment_t*_Nullable); - // Error definitions typedef struct RustCallStatus { int8_t code; @@ -64,26 +59,16 @@ typedef struct RustCallStatus { // ⚠️ increment the version suffix in all instances of UNIFFI_SHARED_HEADER_V4 in this file. ⚠️ #endif // def UNIFFI_SHARED_H -{%- for func in ci.iter_ffi_function_definitions() -%} -{%- if func.is_async() %} - -RustFuture*_Nonnull {{ func.name() }}({% call swift::arg_list_ffi_decl(func) %}); - -bool {{ func.name() }}_poll( - RustFuture*_Nonnull const, - rust_future_waker_t _Nonnull, - rust_future_waker_environment_t*_Nullable const, - {% match func.return_type() %}{% when Some with (type_) %}{{ type_|ffi_type_name }}{% when None %}void{% endmatch %}*_Nullable, - RustCallStatus*_Nonnull -); - -void {{ func.name() }}_drop(RustFuture*_Nonnull, RustCallStatus*_Nonnull); -{%- else %} +// Callbacks for UniFFI Futures +{%- for ffi_type in ci.iter_future_callback_params() %} +typedef void (*UniFfiFutureCallback{{ ffi_type.canonical_name() }})(const void * _Nonnull, {{ ffi_type|ffi_type_name }}, RustCallStatus); +{%- endfor %} +// Scaffolding functions +{%- for func in ci.iter_ffi_function_definitions() %} {% match func.return_type() -%}{%- when Some with (type_) %}{{ type_|ffi_type_name }}{% when None %}void{% endmatch %} {{ func.name() }}( {% call swift::arg_list_ffi_decl(func) %} ); -{%- endif -%} -{%- endfor -%} +{%- endfor %} {% import "macros.swift" as swift %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift index b2c4bfe492..ecf4163627 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift @@ -13,8 +13,13 @@ public enum {{ type_name }} { {% endfor %} {%- endif %} + + fileprivate static func uniffiErrorHandler(_ error: RustBuffer) throws -> Error { + return try {{ ffi_converter_name }}.lift(error) + } } + public struct {{ ffi_converter_name }}: FfiConverterRustBuffer { typealias SwiftType = {{ type_name }} diff --git a/uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift index 54aec08556..92276a4737 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ForeignExecutorTemplate.swift @@ -7,6 +7,10 @@ public struct UniFfiForeignExecutor { public init(priority: TaskPriority) { self.priority = priority } + + public init() { + self.priority = Task.currentPriority + } } fileprivate struct FfiConverterForeignExecutor: FfiConverter { diff --git a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift index 33983186fa..feccd6551e 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift @@ -44,29 +44,41 @@ fileprivate extension RustCallStatus { } private func rustCall(_ callback: (UnsafeMutablePointer) -> T) throws -> T { - try makeRustCall(callback, errorHandler: { - $0.deallocate() - return UniffiInternalError.unexpectedRustCallError - }) + try makeRustCall(callback, errorHandler: nil) } -private func rustCallWithError - (_ errorFfiConverter: F.Type, _ callback: (UnsafeMutablePointer) -> T) throws -> T - where F.SwiftType: Error, F.FfiType == RustBuffer - { - try makeRustCall(callback, errorHandler: { return try errorFfiConverter.lift($0) }) +private func rustCallWithError( + _ errorHandler: @escaping (RustBuffer) throws -> Error, + _ callback: (UnsafeMutablePointer) -> T) throws -> T { + try makeRustCall(callback, errorHandler: errorHandler) } -private func makeRustCall(_ callback: (UnsafeMutablePointer) -> T, errorHandler: (RustBuffer) throws -> Error) throws -> T { +private func makeRustCall( + _ callback: (UnsafeMutablePointer) -> T, + errorHandler: ((RustBuffer) throws -> Error)? +) throws -> T { uniffiEnsureInitialized() var callStatus = RustCallStatus.init() let returnedVal = callback(&callStatus) + try uniffiCheckCallStatus(callStatus: callStatus, errorHandler: errorHandler) + return returnedVal +} + +private func uniffiCheckCallStatus( + callStatus: RustCallStatus, + errorHandler: ((RustBuffer) throws -> Error)? +) throws { switch callStatus.code { case CALL_SUCCESS: - return returnedVal + return case CALL_ERROR: - throw try errorHandler(callStatus.errorBuf) + if let errorHandler = errorHandler { + throw try errorHandler(callStatus.errorBuf) + } else { + callStatus.errorBuf.deallocate() + throw UniffiInternalError.unexpectedRustCallError + } case CALL_PANIC: // When the rust code sees a panic, it tries to construct a RustBuffer diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index 08b7093721..ebb78c224c 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -44,16 +44,24 @@ public class {{ type_name }}: {{ obj.name() }}Protocol { {%- if meth.is_async() %} public func {{ meth.name()|fn_name }}({%- call swift::arg_list_decl(meth) -%}) async {% call swift::throws(meth) %}{% match meth.return_type() %}{% when Some with (return_type) %} -> {{ return_type|type_name }}{% when None %}{% endmatch %} { - let future = {% call swift::to_ffi_call_with_prefix("self.pointer", meth) %} - - return {% if meth.throws() -%} - try await withCheckedThrowingContinuation - {%- else -%} - await withCheckedContinuation - {%- endif -%} - { continuation in - let env = Unmanaged.passRetained(_UniFFI_{{ obj.name() }}_{{ meth.name()|class_name }}_Env(rustyFuture: future, continuation: continuation)) - _UniFFI_{{ obj.name() }}_{{ meth.name()|class_name }}_waker(raw_env: env.toOpaque()) + // Suspend the function and call the scaffolding function, passing it a callback handler from + // `AsyncTypes.swift` + // + // Make sure to hold on to a reference to the continuation in the top-level scope so that + // it's not freed before the callback is invoked. + var continuation: {{ meth.result_type().borrow()|future_continuation_type }}? = nil + return {% call swift::try(meth) %} await withCheckedThrowingContinuation { + continuation = $0 + try! rustCall() { + {{ meth.ffi_func().name() }}( + self.pointer, + {% call swift::arg_list_lowered(meth) %} + FfiConverterForeignExecutor.lower(UniFfiForeignExecutor()), + {{ meth.result_type().borrow()|future_callback }}, + &continuation, + $0 + ) + } } } @@ -80,77 +88,6 @@ public class {{ type_name }}: {{ obj.name() }}Protocol { {% endfor %} } -{% for meth in obj.methods() -%} -{%- if meth.is_async() -%} - -fileprivate class _UniFFI_{{ obj.name() }}_{{ meth.name()|class_name }}_Env { - var rustFuture: OpaquePointer - var continuation: CheckedContinuation<{% match meth.return_type() %}{% when Some with (return_type) %}{{ return_type|type_name }}{% when None %}(){% endmatch %}, {% if meth.throws() %}Error{% else %}Never{% endif %}> - - init(rustyFuture: OpaquePointer, continuation: CheckedContinuation<{% match meth.return_type() %}{% when Some with (return_type) %}{{ return_type|type_name }}{% when None %}(){% endmatch %}, {% if meth.throws() %}Error{% else %}Never{% endif %}>) { - self.rustFuture = rustyFuture - self.continuation = continuation - } - - deinit { - try! rustCall { - {{ meth.ffi_func().name() }}_drop(self.rustFuture, $0) - } - } -} - -fileprivate func _UniFFI_{{ obj.name() }}_{{ meth.name()|class_name }}_waker(raw_env: UnsafeMutableRawPointer?) { - Task { - let env = Unmanaged<_UniFFI_{{ obj.name() }}_{{ meth.name()|class_name }}_Env>.fromOpaque(raw_env!) - let env_ref = env.takeUnretainedValue() - let polledResult = {% match meth.ffi_func().return_type() -%} - {%- when Some with (return_type) -%} - UnsafeMutablePointer<{{ return_type|type_ffi_lowered }}>.allocate(capacity: 1) - {%- when None -%} - UnsafeMutableRawPointer.allocate(byteCount: 0, alignment: 0) - {%- endmatch %} - {% if meth.throws() -%}do { - {%- endif %} - - let isReady = {% match meth.throws_type() -%} - {%- when Some with (error) -%} - try rustCallWithError({{ error|ffi_converter_name }}.self) { - {%- when None -%} - try! rustCall() { - {%- endmatch %} - {{ meth.ffi_func().name() }}_poll( - env_ref.rustFuture, - _UniFFI_{{ obj.name() }}_{{ meth.name()|class_name }}_waker, - env.toOpaque(), - polledResult, - $0 - ) - } - - if isReady { - env_ref.continuation.resume(returning: {% match meth.return_type() -%} - {%- when Some with (return_type) -%} - try! {{ return_type|lift_fn }}(polledResult.move()) - {%- when None -%} - () - {%- endmatch -%} - ) - polledResult.deallocate() - env.release() - } - {%- if meth.throws() %} - } catch { - env_ref.continuation.resume(throwing: error) - polledResult.deallocate() - env.release() - } - {%- endif %} - } -} - -{% endif -%} -{%- endfor %} - public struct {{ ffi_converter_name }}: FfiConverter { typealias FfiType = UnsafeMutableRawPointer typealias SwiftType = {{ type_name }} diff --git a/uniffi_bindgen/src/bindings/swift/templates/TopLevelFunctionTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/TopLevelFunctionTemplate.swift index 72914b74f9..e3c87ca336 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/TopLevelFunctionTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/TopLevelFunctionTemplate.swift @@ -1,81 +1,23 @@ {%- if func.is_async() %} -fileprivate class _UniFFI_{{ func.name()|class_name }}_Env { - var rustFuture: OpaquePointer - var continuation: CheckedContinuation<{% match func.return_type() %}{% when Some with (return_type) %}{{ return_type|type_name }}{% when None %}(){% endmatch %}, {% if func.throws() %}Error{% else %}Never{% endif %}> - - init(rustyFuture: OpaquePointer, continuation: CheckedContinuation<{% match func.return_type() %}{% when Some with (return_type) %}{{ return_type|type_name }}{% when None %}(){% endmatch %}, {% if func.throws() %}Error{% else %}Never{% endif %}>) { - self.rustFuture = rustyFuture - self.continuation = continuation - } - - deinit { - try! rustCall { - {{ func.ffi_func().name() }}_drop(self.rustFuture, $0) - } - } -} - -fileprivate func _UniFFI_{{ func.name()|class_name }}_waker(raw_env: UnsafeMutableRawPointer?) { - Task { - let env = Unmanaged<_UniFFI_{{ func.name()|class_name }}_Env>.fromOpaque(raw_env!) - let env_ref = env.takeUnretainedValue() - let polledResult = {% match func.ffi_func().return_type() -%} - {%- when Some with (return_type) -%} - UnsafeMutablePointer<{{ return_type|type_ffi_lowered }}>.allocate(capacity: 1) - {%- when None -%} - UnsafeMutableRawPointer.allocate(byteCount: 0, alignment: 0) - {%- endmatch %} - {% if func.throws() -%}do { - {%- endif %} - - let isReady = {% match func.throws_type() -%} - {%- when Some with (error) -%} - try rustCallWithError({{ error|ffi_converter_name }}.self) { - {%- when None -%} - try! rustCall() { - {%- endmatch %} - {{ func.ffi_func().name() }}_poll( - env_ref.rustFuture, - _UniFFI_{{ func.name()|class_name }}_waker, - env.toOpaque(), - polledResult, +public func {{ func.name()|fn_name }}({%- call swift::arg_list_decl(func) -%}) async {% call swift::throws(func) %}{% match func.return_type() %}{% when Some with (return_type) %} -> {{ return_type|type_name }}{% when None %}{% endmatch %} { + var continuation: {{ func.result_type().borrow()|future_continuation_type }}? = nil + // Suspend the function and call the scaffolding function, passing it a callback handler from + // `AsyncTypes.swift` + // + // Make sure to hold on to a reference to the continuation in the top-level scope so that + // it's not freed before the callback is invoked. + return {% call swift::try(func) %} await withCheckedThrowingContinuation { + continuation = $0 + try! rustCall() { + {{ func.ffi_func().name() }}( + {% call swift::arg_list_lowered(func) %} + FfiConverterForeignExecutor.lower(UniFfiForeignExecutor()), + {{ func.result_type().borrow()|future_callback }}, + &continuation, $0 ) } - - if isReady { - env_ref.continuation.resume(returning: {% match func.return_type() -%} - {%- when Some with (return_type) -%} - try! {{ return_type|lift_fn }}(polledResult.move()) - {%- when None -%} - () - {%- endmatch -%} - ) - polledResult.deallocate() - env.release() - } - {%- if func.throws() %} - } catch { - env_ref.continuation.resume(throwing: error) - polledResult.deallocate() - env.release() - } - {%- endif %} - } -} - -public func {{ func.name()|fn_name }}({%- call swift::arg_list_decl(func) -%}) async {% call swift::throws(func) %}{% match func.return_type() %}{% when Some with (return_type) %} -> {{ return_type|type_name }}{% when None %}{% endmatch %} { - let future = {% call swift::to_ffi_call(func) %} - - return {% if func.throws() -%} - try await withCheckedThrowingContinuation - {%- else -%} - await withCheckedContinuation - {%- endif -%} - { continuation in - let env = Unmanaged.passRetained(_UniFFI_{{ func.name()|class_name }}_Env(rustyFuture: future, continuation: continuation)) - _UniFFI_{{ func.name()|class_name }}_waker(raw_env: env.toOpaque()) } } @@ -97,4 +39,4 @@ public func {{ func.name()|fn_name }}({% call swift::arg_list_decl(func) %}) {% } {% endmatch %} -{%- endif %} \ No newline at end of file +{%- endif %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/Types.swift b/uniffi_bindgen/src/bindings/swift/templates/Types.swift index 4e85c925da..ffe4b8a4af 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Types.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Types.swift @@ -91,3 +91,7 @@ {%- else %} {%- endmatch %} {%- endfor %} + +{%- if ci.has_async_fns() %} +{%- include "AsyncTypes.swift" %} +{%- endif %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/macros.swift b/uniffi_bindgen/src/bindings/swift/templates/macros.swift index 6894f691e9..4ad9136859 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/macros.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/macros.swift @@ -1,18 +1,18 @@ {# // Template to call into rust. Used in several places. // Variable names in `arg_list_decl` should match up with arg lists -// passed to rust via `_arg_list_ffi_call` +// passed to rust via `arg_list_lowered` #} {%- macro to_ffi_call(func) -%} {%- call try(func) -%} {%- match func.throws_type() -%} {%- when Some with (e) -%} - rustCallWithError({{ e|ffi_converter_name }}.self) { + rustCallWithError({{ e|ffi_converter_name }}.lift) { {%- else -%} rustCall() { {%- endmatch %} - {{ func.ffi_func().name() }}({% call _arg_list_ffi_call(func) -%}{% if func.arguments().len() > 0 %}, {% endif %}$0) + {{ func.ffi_func().name() }}({% call arg_list_lowered(func) -%} $0) } {%- endmacro -%} @@ -20,20 +20,19 @@ {% call try(func) %} {%- match func.throws_type() %} {%- when Some with (e) %} - rustCallWithError({{ e|ffi_converter_name }}.self) { + rustCallWithError({{ e|ffi_converter_name }}.lift) { {%- else %} rustCall() { {% endmatch %} {{ func.ffi_func().name() }}( - {{- prefix }}, {% call _arg_list_ffi_call(func) -%}{% if func.arguments().len() > 0 %}, {% endif %}$0 + {{- prefix }}, {% call arg_list_lowered(func) -%} $0 ) } {%- endmacro %} -{%- macro _arg_list_ffi_call(func) %} +{%- macro arg_list_lowered(func) %} {%- for arg in func.arguments() %} - {{ arg|lower_fn }}({{ arg.name()|var_name }}) - {%- if !loop.last %}, {% endif -%} + {{ arg|lower_fn }}({{ arg.name()|var_name }}), {%- endfor %} {%- endmacro -%} diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index abaa6dd317..5376ce692a 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -18,7 +18,7 @@ /// For the types that involve memory allocation, we make a distinction between /// "owned" types (the recipient must free it, or pass it to someone else) and /// "borrowed" types (the sender must keep it alive for the duration of the call). -#[derive(Debug, Clone, Hash, PartialEq, Eq)] +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub enum FfiType { // N.B. there are no booleans at this layer, since they cause problems for JNA. UInt8, @@ -52,10 +52,47 @@ pub enum FfiType { ForeignExecutorHandle, /// Pointer to the callback function that's invoked to schedule calls with a ForeignExecutor ForeignExecutorCallback, + /// Pointer to a callback function to complete an async Rust function + FutureCallback { + /// Note: `return_type` is not optional because we have a void callback parameter like we + /// can have a void return. Instead, we use `UInt8` as a placeholder value. + return_type: Box, + }, + /// Opaque pointer passed to the FutureCallback + FutureCallbackData, // 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. } +impl FfiType { + pub fn canonical_name(&self) -> String { + match self { + Self::UInt8 => "UInt8".into(), + Self::Int8 => "Int8".into(), + Self::UInt16 => "UInt16".into(), + Self::Int16 => "Int16".into(), + Self::UInt32 => "UInt32".into(), + Self::Int32 => "Int32".into(), + Self::UInt64 => "UInt64".into(), + Self::Int64 => "Int64".into(), + Self::Float32 => "Float32".into(), + Self::Float64 => "Float64".into(), + Self::RustArcPtr(name) => format!("RustArcPtr{name}"), + Self::RustBuffer(maybe_suffix) => { + format!("RustBuffer{}", maybe_suffix.as_deref().unwrap_or_default()) + } + Self::ForeignBytes => "ForeignBytes".into(), + Self::ForeignCallback => "ForeignCallback".into(), + Self::ForeignExecutorHandle => "ForeignExecutorHandle".into(), + Self::ForeignExecutorCallback => "ForeignExecutorCallback".into(), + Self::FutureCallback { return_type } => { + format!("FutureCallback{}", return_type.canonical_name(),) + } + Self::FutureCallbackData => "FutureCallbackData".into(), + } + } +} + /// Represents an "extern C"-style function that will be part of the FFI. /// /// These can't be declared explicitly in the UDL, but rather, are derived automatically @@ -98,6 +135,40 @@ impl FfiFunction { pub fn is_object_free_function(&self) -> bool { self.is_object_free_function } + + pub fn init( + &mut self, + return_type: Option, + args: impl IntoIterator, + ) { + self.arguments = args.into_iter().collect(); + if self.is_async() { + self.arguments.extend([ + // Used to schedule polls + FfiArgument { + name: "uniffi_executor".into(), + type_: FfiType::ForeignExecutorHandle, + }, + // Invoked when the future is ready + FfiArgument { + name: "uniffi_callback".into(), + type_: FfiType::FutureCallback { + return_type: Box::new(return_type.unwrap_or(FfiType::UInt8)), + }, + }, + // Data pointer passed to the callback + FfiArgument { + name: "uniffi_callback_data".into(), + type_: FfiType::FutureCallbackData, + }, + ]); + // Async scaffolding functions never return values. Instead, the callback is invoked + // when the Future is ready. + self.return_type = None; + } else { + self.return_type = return_type; + } + } } impl Default for FfiFunction { diff --git a/uniffi_bindgen/src/interface/function.rs b/uniffi_bindgen/src/interface/function.rs index 00f3eedf14..f15c6353f0 100644 --- a/uniffi_bindgen/src/interface/function.rs +++ b/uniffi_bindgen/src/interface/function.rs @@ -37,7 +37,7 @@ use anyhow::{bail, Result}; use uniffi_meta::Checksum; use super::attributes::{ArgumentAttributes, Attribute, FunctionAttributes}; -use super::ffi::{FfiArgument, FfiFunction}; +use super::ffi::{FfiArgument, FfiFunction, FfiType}; use super::literal::{convert_default_value, Literal}; use super::types::{ObjectImpl, Type, TypeIterator}; use super::{convert_type, APIConverter, ComponentInterface}; @@ -124,9 +124,10 @@ impl Function { if self.ffi_func.name.is_empty() { self.ffi_func.name = uniffi_meta::fn_symbol_name(ci_namespace, &self.name); } - - self.ffi_func.arguments = self.arguments.iter().map(|arg| arg.into()).collect(); - self.ffi_func.return_type = self.return_type.as_ref().map(|rt| rt.into()); + self.ffi_func.init( + self.return_type.as_ref().map(Into::into), + self.arguments.iter().map(Into::into), + ); Ok(()) } } @@ -280,11 +281,34 @@ impl APIConverter for weedle::argument::SingleArgument<'_> { } } +/// Combines the return and throws type of a function/method +#[derive(Debug, PartialOrd, Ord, PartialEq, Eq)] +pub struct ResultType { + pub return_type: Option, + pub throws_type: Option, +} + +impl ResultType { + /// Get the `T` parameters for the `FutureCallback` for this ResultType + pub fn future_callback_param(&self) -> FfiType { + match &self.return_type { + Some(t) => t.ffi_type(), + None => FfiType::UInt8, + } + } +} + /// Implemented by function-like types (Function, Method, Constructor) pub trait Callable { fn arguments(&self) -> Vec<&Argument>; fn return_type(&self) -> Option; fn throws_type(&self) -> Option; + fn result_type(&self) -> ResultType { + ResultType { + return_type: self.return_type(), + throws_type: self.throws_type(), + } + } } impl Callable for Function { diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index feb41bd05e..2ea51a32c9 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -45,7 +45,7 @@ //! * Error messages and general developer experience leave a lot to be desired. use std::{ - collections::{btree_map::Entry, BTreeMap, HashSet}, + collections::{btree_map::Entry, BTreeMap, BTreeSet, HashSet}, convert::TryFrom, iter, }; @@ -64,7 +64,7 @@ pub use enum_::Enum; mod error; pub use error::Error; mod function; -pub use function::{Argument, Callable, Function}; +pub use function::{Argument, Callable, Function, ResultType}; mod literal; pub use literal::{Literal, Radix}; mod namespace; @@ -216,6 +216,19 @@ impl ComponentInterface { self.errors.get(name) } + /// Get the definitions for every Method type in the interface. + pub fn iter_callables(&self) -> impl Iterator { + self.function_definitions() + .iter() + .map(|f| f as &dyn Callable) + .chain(self.objects.iter().flat_map(|o| { + o.constructors() + .into_iter() + .map(|c| c as &dyn Callable) + .chain(o.methods().into_iter().map(|m| m as &dyn Callable)) + })) + } + /// Should we generate read (and lift) functions for errors? /// /// This is a workaround for the fact that lower/write can't be generated for some errors, @@ -411,6 +424,24 @@ impl ComponentInterface { self.iter_ffi_function_definitions().any(|f| f.is_async()) } + /// Iterate over `T` parameters of the `FutureCallback` callbacks in this interface + pub fn iter_future_callback_params(&self) -> impl Iterator { + let unique_results = self + .iter_callables() + .map(|c| c.result_type().future_callback_param()) + .collect::>(); + unique_results.into_iter() + } + + /// Iterate over return/throws types for async functions + pub fn iter_async_result_types(&self) -> impl Iterator { + let unique_results = self + .iter_callables() + .map(|c| c.result_type()) + .collect::>(); + unique_results.into_iter() + } + /// List the definitions of all FFI functions in the interface. /// /// The set of FFI functions is derived automatically from the set of higher-level types @@ -626,6 +657,10 @@ impl ComponentInterface { if !matches!(self.types.get_type_definition(defn.name()), None) { bail!("Conflicting type definition for \"{}\"", defn.name()); } + if defn.is_async() { + // Async functions depend on the foreign executor + self.types.add_known_type(&Type::ForeignExecutor)?; + } self.functions.push(defn); Ok(()) diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index 7d648f99b5..932a86391a 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -504,9 +504,10 @@ impl Method { self.ffi_func.name = uniffi_meta::method_symbol_name(ci_namespace, obj_name, &self.name); } - - self.ffi_func.arguments = self.full_arguments().iter().map(Into::into).collect(); - self.ffi_func.return_type = self.return_type.as_ref().map(Into::into); + self.ffi_func.init( + self.return_type.as_ref().map(Into::into), + self.full_arguments().iter().map(Into::into), + ); Ok(()) } diff --git a/uniffi_bindgen/src/scaffolding/mod.rs b/uniffi_bindgen/src/scaffolding/mod.rs index 0f3bc2ac24..60dc7a1f55 100644 --- a/uniffi_bindgen/src/scaffolding/mod.rs +++ b/uniffi_bindgen/src/scaffolding/mod.rs @@ -80,6 +80,10 @@ mod filters { FfiType::ForeignBytes => "::uniffi::ForeignBytes".into(), FfiType::ForeignCallback => "::uniffi::ForeignCallback".into(), FfiType::ForeignExecutorHandle => "::uniffi::ForeignExecutorHandle".into(), + FfiType::FutureCallback { return_type } => { + format!("::uniffi::FutureCallback<{}>", type_ffi(return_type)?) + } + FfiType::FutureCallbackData => "*const ()".into(), FfiType::ForeignExecutorCallback => "::uniffi::ForeignExecutorCallback".into(), }) } diff --git a/uniffi_core/src/ffi/foreignexecutor.rs b/uniffi_core/src/ffi/foreignexecutor.rs index 5db403c613..de129f3a39 100644 --- a/uniffi_core/src/ffi/foreignexecutor.rs +++ b/uniffi_core/src/ffi/foreignexecutor.rs @@ -284,6 +284,10 @@ mod test { } } + pub fn handle(&self) -> Option { + self.executor.as_ref().map(|e| e.handle) + } + pub fn call_count(&self) -> usize { self.calls.lock().unwrap().len() } diff --git a/uniffi_core/src/ffi/rustcalls.rs b/uniffi_core/src/ffi/rustcalls.rs index 90230d1687..edaf480273 100644 --- a/uniffi_core/src/ffi/rustcalls.rs +++ b/uniffi_core/src/ffi/rustcalls.rs @@ -111,7 +111,7 @@ where /// If the call succeeds this returns Some(v) and doesn't touch out_status /// If the call fails (including Err results), this returns None and updates out_status /// -/// This contains the shared code between `rust_call` and `uniffi_rustfuture_poll`. +/// This contains the shared code between `rust_call` and `rustfuture::do_wake`. pub(crate) fn rust_call_with_out_status( out_status: &mut RustCallStatus, callback: F, diff --git a/uniffi_core/src/ffi/rustfuture.rs b/uniffi_core/src/ffi/rustfuture.rs index f8fbbbcfaf..75a5787fe0 100644 --- a/uniffi_core/src/ffi/rustfuture.rs +++ b/uniffi_core/src/ffi/rustfuture.rs @@ -1,26 +1,23 @@ -//! [`RustFuture`] represents a [`Future`] that can be sent over FFI safely-ish. -//! -//! The [`RustFuture`] type holds an inner `Future>`, and -//! thus is parameterized by `T` and `E`. On the `RustFuture` type itself, there -//! is no constraint over those generic types (constraints are present in the -//! [`uniffi_rustfuture_poll`] function, where `T: FfiReturn`, see later -//! to learn more). Every function or method that returns a `Future` must -//! transform the result into a `Result`. This is done by the procedural -//! macros automatically: `Future` is transformed into `RustFuture`, and `Future>` is transformed into -//! `RustFuture`. +/* 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/. */ + +//! [`RustFuture`] represents a [`Future`] that can be sent to the foreign code over FFI. //! -//! This type may not be instantiated directly, but _via_ the procedural macros, -//! such as `#[uniffi::export]`. A `RustFuture` is created, boxed, and then -//! manipulated by (hidden) helper functions, resp. [`uniffi_rustfuture_poll`] -//! and [`uniffi_rustfuture_drop`]. Because the `RustFuture` type contains a -//! generic parameters `T` and `E`, the procedural macros will do a -//! monomorphisation phase so that all the API has all their types statically -//! known. +//! This type is not instantiated directly, but via the procedural macros, such as `#[uniffi::export]`. //! //! # The big picture //! -//! This section will explain how the entire workflow works. +//! What happens when you call an async function exported from the Rust API? +//! +//! 1. You make a call to a generated async function in the foreign bindings. +//! 2. That function suspends itself, then makes a scaffolding call. In addition to the normal +//! arguments, it passes a `ForeignExecutor` and callback. +//! 3. Rust uses the `ForeignExecutor` to schedules polls of the Future until it's ready. Then +//! invokes the callback. +//! 4. The callback resumes the suspended async function from (2), closing the loop. +//! +//! # Anatomy of an async call //! //! Let's consider the following Rust function: //! @@ -31,113 +28,48 @@ //! } //! ``` //! -//! In Rust, this `async fn` syntax is strictly equivalent to: +//! In Rust, this `async fn` syntax is strictly equivalent to a normal function that returns a +//! `Future`: //! //! ```rust,ignore //! #[uniffi::export] //! fn hello() -> impl Future { /* … */ } //! ``` //! -//! Once this is understood, it becomes obvious that an `async` function -//! returns a `Future`. -//! -//! This function will not be modified, but new functions with a C ABI will be -//! created, as such: +//! `uniffi-bindgen` will generate a scaffolding function for each exported async function: //! //! ```rust,ignore -//! /// The `hello` function, as seen from the outside. It returns a `Future`, or -//! /// more precisely, a `RustFuture` that wraps the returned future. +//! // The `hello` function, as seen from the outside. It inputs 3 extra arguments: +//! // - executor: used to schedule polls of the future +//! // - callback: invoked when the future is ready +//! // - callback_data: opaque pointer that's passed to the callback. It points to any state needed to +//! // resume the async function. //! #[no_mangle] //! pub extern "C" fn _uniffi_hello( -//! call_status: &mut ::uniffi::RustCallStatus -//! ) -> Option>> { -//! ::uniffi::call_with_output(call_status, || { -//! Some(Box::new(::uniffi::RustFuture::new(async move { -//! Ok(hello().await) -//! }))) -//! }) -//! } -//! ``` -//! -//! This function returns an `Option>`: -//! -//! * Why an `Option`? Because in case of an error, it must return a default -//! value, in this case `None`, otherwise `Some`. If we were returning `Box` -//! directly, it would leak a pointer. -//! -//! * Why `Box`? Because Rust doesn't own the future, it's passed to the -//! foreign language, and the foreign language is responsible to manage it. -//! -//! * Finally, this function calls the real Rust `hello` function. It -//! transforms its result into a `Result` if it's needed. -//! -//! The second generated function is the _poll function_: -//! -//! ```rust,ignore -//! /// The function to poll the `RustFuture` returned by `_uniffi_hello`. -//! #[no_mangle] -//! pub extern "C" fn _uniffi_hello_poll( -//! future: Option<&mut ::uniffi::RustFuture>, -//! waker: Option>, -//! waker_environment: *const ::uniffi::RustFutureForeignWakerEnvironment, -//! polled_result: &mut ::FfiType, -//! call_status:: &mut ::uniffi::RustCallStatus, -//! ) -> bool { -//! ::uniffi::ffi::uniffi_rustfuture_poll(future, waker, waker_environment, polled_result, call_status) -//! } -//! ``` -//! -//! Let's analyse this function because it's an important one: -//! -//! * First off, this _poll FFI function_ forwards everything to -//! [`uniffi_rustfuture_poll`]. The latter is generic, while the former has been -//! monomorphised by the procedural macro. -//! -//! * Second, it receives the `RustFuture` from `_uniffi_hello` as an -//! `Option<&mut RustFuture<_>>`. It doesn't take ownership of the `RustFuture`! -//! It borrows it (mutably). It's wrapped inside an `Option` to check whether -//! it's a null pointer or not; it's defensive programming here, `null` will -//! make the Rust code to panic gracefully. -//! -//! * Third, it receives a _waker_ as a pair of a _function pointer_ plus its -//! _environment_, if any; a null pointer is purposely allowed for the environment. -//! This waker function lives on the foreign language side. We will come back -//! to it in a second. -//! -//! * Fourth, it receives an in-out `polled_result` argument, that is filled with the -//! polled result if the future is ready. -//! -//! * Firth, the classical `call_status`, which is part of the calling API of `uniffi`. -//! -//! * Finally, the function returns `true` if the future is ready, `false` if pending. -//! -//! Please don't forget to read [`uniffi_rustfuture_poll`] to learn how -//! `polled_result` + `call_status` + the returned bool type are used to indicate -//! in which state the future is. -//! -//! So, everytime this function is called, it polls the `RustFuture` after -//! having reconstituted a valid [`Waker`] for it. As said earlier, we will come -//! back to it. -//! -//! The last generated function is the _drop function_: -//! -//! ```rust,ignore -//! #[no_mangle] -//! pub extern "C" fn _uniffi_hello_drop( -//! future: Option>>, -//! call_status: &mut ::uniffi::RustCallStatus +//! // ...If the function inputted arguments, the lowered versions would go here +//! uniffi_executor: ForeignExecutor, +//! uniffi_callback: >::FutureCallback, +//! uniffi_callback_data: *const (), +//! uniffi_call_status: &mut ::uniffi::RustCallStatus //! ) { -//! ::uniffi::ffi::uniffi_rustfuture_drop(future, call_status) +//! ::uniffi::call_with_output(uniffi_call_status, || { +//! let uniffi_rust_future = RustFuture::<_, bool, crate::UniFFITag,>::new( +//! future: hello(), +//! executor, +//! callback, +//! callback_data, +//! ); +//! uniffi_rust_future.wake(); +//! }) //! } //! ``` //! -//! First off, this _drop function_ is responsible to drop the `RustFuture`. It's -//! clear by looking at its signature: It receives an `Option>>`, -//! i.e. it takes ownership of the `RustFuture` _via_ `Box`! -//! -//! Similarly to the _poll function_, it forwards everything to -//! [`uniffi_rustfuture_drop`], which is the generic version of the monomorphised _drop -//! function_. +//! Rust will continue to poll the future until it's ready, after that. The callback will +//! eventually be invoked with these arguments: +//! - callback_data +//! - FfiConverter::ReturnType (the type that would be returned by a sync function) +//! - RustCallStatus (used to signal errors/panics when executing the future) +//! - Rust will stop polling the future, even if it's waker is invoked again. //! //! ## How does `Future` work exactly? //! @@ -175,112 +107,6 @@ //! //! “awakening” is done by using the `Waker`. //! -//! ## Awaken from the foreign language land -//! -//! Our _poll function_ receives a waker function pointer, along with a waker -//! environment. We said that the waker function lives on the foreign language -//! side. That's really important. It cannot live inside Rust because Rust -//! isn't aware of which foreign language it is run from, and thus doesn't know -//! which executor is used. It is UniFFI's job to write a proper foreign waker -//! function that will use the native foreign language's executor provided -//! by the foreign language itself (e.g. `Task` in Swift) or by some common -//! libraries (e.g. `asyncio` in Python), to ask to poll the future again. -//! -//! We expect the waker to be the same per future. This property is not -//! checked, but the current foreign language implementations provided by UniFFI -//! guarantee that the waker is the same per future everytime. Changing the -//! waker for the same future leads to undefined behaviours, and may panic at some -//! point or leak data. -//! -//! The waker must live longer than the `RustFuture`, so its lifetime's range -//! must include `_uniffi_hello()` to `_uniffi_hello_drop()`, otherwise it leads to -//! undefined behaviours. -//! -//! ## The workflow -//! -//! 1. The foreign language starts by calling the regular FFI function -//! `_uniffi_hello`. It gets an `Option>>`. -//! -//! 2. The foreign language immediately polls the future by using the `_uniffi_hello_poll` -//! function. It passes a function pointer to the waker function, implemented -//! inside the foreign language, along with its environment if any. -//! -//! - Either the future is ready and computes a value, in this case the _poll -//! function_ will lift the value and will drop the future with the _drop function_ -//! (`_uniffi_hello_drop`), -//! -//! - or the future is pending (not ready), and is responsible to register -//! the waker (as explained above). -//! -//! 3. When the waker is called, it calls the _poll function_, so we basically jump -//! to point 2 of this list. -//! -//! There is an important subtlety though. Imagine the following Rust code: -//! -//! ```rust,ignore -//! let mut shared_state: MutexGuard<_> = a_shared_state.lock().unwrap(); -//! -//! if let Some(waker) = shared_state.waker.take() { -//! waker.wake(); -//! } -//! ``` -//! -//! This code will call the waker. That's nice and all. However, when the waker -//! function is called by `waker.wake()`, this code above has not returned yet. -//! And the waker function, as designed so far, will call the _poll function_ -//! of the Rust `Future` which… may use the same lock (`a_shared_state`), -//! which is not released yet: there is a dead-lock! Rust is not responsible of -//! that, kind of. Rust **must ignore how the executor works**, all `Future`s -//! are executor-agnostic by design. To avoid creating problems, the waker -//! must “cut” the flow, so that Rust code can continue to run as expected, and -//! after that, the _poll function_ must be called. -//! -//! Put differently, the waker function must call the _poll function_ _as -//! soon as possible_, not _immediately_. It actually makes sense: The waker -//! must signal the executor to schedule a poll for a specific `Future` when -//! possible; it's not an inline operation. The implementation of the waker -//! must be very careful of that. -//! -//! With a diagram (because this comment would look so much clever with a diagram), -//! it looks like this: -//! -//! ```text -//! ┌────────────────────┐ -//! │ │ -//! │ Calling hello │ -//! │ │ -//! └─────────┬──────────┘ -//! │ -//! ▼ fn waker ◄──┐ -//! ┌────────────────────────────────┐ │ -//! │ │ │ -//! │ Ask the executor to schedule │ │ -//! │ this as soon as possible │ │ -//! │ │ │ -//! │ ┌──────────────────────────┐ │ │ -//! │ │ │ │ │ -//! │ │ Polling the RustFuture │ │ │ -//! │ │ Pass pointer to waker ──┼──┼──┘ -//! │ │ │ │ -//! │ └────────────┬─────────────┘ │ -//! │ │ │ -//! └───────────────┼────────────────┘ -//! │ -//! ▼ -//! ┌──── The future is ─────┐ -//! │ │ -//! Ready Pending -//! │ │ -//! ▼ ▼ -//! ┌───────────────┐ ┌──────────────────────┐ -//! │ Lift result │ │ Nothing │ -//! │ Have fun │ │ Let's wait for the │ -//! └───────────────┘ │ waker to be called │ -//! └──────────────────────┘ -//! ``` -//! -//! That's all folks! -//! //! [`Future`]: https://doc.rust-lang.org/std/future/trait.Future.html //! [`Future::poll`]: https://doc.rust-lang.org/std/future/trait.Future.html#tymethod.poll //! [`Pol::Ready`]: https://doc.rust-lang.org/std/task/enum.Poll.html#variant.Ready @@ -289,318 +115,429 @@ //! [`Waker`]: https://doc.rust-lang.org/std/task/struct.Waker.html //! [`RawWaker`]: https://doc.rust-lang.org/std/task/struct.RawWaker.html -use crate::{ffi::rustcalls::rust_call_with_out_status, FfiConverter, RustCallStatus}; +use crate::{ + rust_call_with_out_status, schedule_raw, FfiConverter, FfiDefault, ForeignExecutor, + ForeignExecutorHandle, RustCallStatus, +}; use std::{ - ffi::c_void, + cell::UnsafeCell, future::Future, - mem::{ManuallyDrop, MaybeUninit}, panic, pin::Pin, - sync::Arc, + sync::{ + atomic::{AtomicU32, Ordering}, + Arc, + }, task::{Context, Poll, RawWaker, RawWakerVTable, Waker}, }; -/// `RustFuture` represents a [`Future`] that can be sent over FFI safely-ish. +/// Callback that we invoke when a `RustFuture` is ready. /// -/// See the module documentation to learn more. -#[repr(transparent)] -pub struct RustFuture(Pin + 'static>>); - -impl RustFuture { - /// Create a new `RustFuture` from a regular `Future`. - pub fn new(future: F) -> Self - where - F: Future + 'static, - { - Self(Box::pin(future)) - } +/// The foreign code passes a pointer to one of these callbacks along with an opaque data pointer. +/// When the future is ready, we invoke the callback. +pub type FutureCallback = + extern "C" fn(callback_data: *const (), result: T, status: RustCallStatus); - /// Create a new `RustFuture` from a Tokio `Future`. It needs an - /// intermediate compatibility layer to be handlded as a regular Rust - /// `Future`. - #[cfg(feature = "tokio")] - pub fn new_tokio(future: F) -> Self - where - F: Future + 'static, - { - Self(Box::pin(async_compat::Compat::new(future))) - } +/// Future that the foreign code is awaiting +/// +/// RustFuture is always stored inside a Pin>. The `Arc<>` allows it to be shared between +/// wakers and Pin<> signals that it must not move, since this would break any self-references in +/// the future. +pub struct RustFuture +where + // The future needs to be `Send`, since it will move to whatever thread the foreign executor + // chooses. However, it doesn't need to be `Sync', since we don't share references between + // threads (see do_wake()). + F: Future + Send, + T: FfiConverter, +{ + future: UnsafeCell, + executor: ForeignExecutor, + wake_counter: AtomicU32, + callback: T::FutureCallback, + callback_data: *const (), +} - /// Poll the future. - /// - /// There is one subtlety compared to `Future::poll` though: the - /// `foreign_waker` and `foreign_waker_environment` variables replace the - /// classical [`Context`]. We want the `RustFuture` **to be driven by the - /// foreign language's executor**. Hence the possibility for the foreign - /// language to provide its own waker function: Rust can signal something - /// has happened within the future and should be polled again. - /// - /// `Poll` is mapped to [`RustFuturePoll`]. - /// - /// [`Context`]: https://doc.rust-lang.org/std/task/struct.Context.html - fn poll( - &mut self, - foreign_waker: RustFutureForeignWakerFunction, - foreign_waker_environment: *const c_void, - ) -> Poll { - let waker = unsafe { - Waker::from_raw(RawWaker::new( - RustFutureForeignWaker::new(foreign_waker, foreign_waker_environment) - .into_unit_ptr(), - &RustFutureForeignRawWaker::VTABLE, - )) - }; - let mut context = Context::from_waker(&waker); +// Mark RustFuture as Send + Sync, since we will be sharing it between threads. This means we need +// to serialize access to any fields that aren't Send + Sync (`future`, `callback`, and +// `callback_data`). See `do_wake()` for details on this. - self.0.as_mut().poll(&mut context) - } +unsafe impl Send for RustFuture +where + F: Future + Send, + T: FfiConverter, +{ } -#[cfg(test)] -mod tests_rust_future { - use super::*; - use std::mem; +unsafe impl Sync for RustFuture +where + F: Future + Send, + T: FfiConverter, +{ +} - #[test] - fn test_rust_future_size() { - let pointer_size = mem::size_of::<*const u8>(); - let rust_future_size = pointer_size * 2; +impl RustFuture +where + F: Future + Send, + T: FfiConverter, +{ + pub fn new( + future: F, + executor_handle: ForeignExecutorHandle, + callback: T::FutureCallback, + callback_data: *const (), + ) -> Pin> { + let executor = + >::try_lift(executor_handle) + .expect("Error lifting ForeignExecutorHandle"); + Arc::pin(Self { + future: UnsafeCell::new(future), + wake_counter: AtomicU32::new(0), + executor, + callback, + callback_data, + }) + } - assert_eq!(mem::size_of::>(), rust_future_size); - assert_eq!(mem::size_of::>(), rust_future_size); + /// Wake up soon and poll our future. + /// + /// This method ensures that a call to `do_wake()` is scheduled. One one call will be scheduled + /// at any time, even if `wake_soon` called multiple times from multiple threads. + pub fn wake(self: Pin>) { + if self.wake_counter.fetch_add(1, Ordering::Relaxed) == 0 { + self.schedule_do_wake(); + } } -} -/// Type alias to a function with a C ABI. It defines the shape of the foreign -/// language's waker which is called by the [`RustFuture`] to signal the -/// foreign language that something has happened. See the module documentation -/// to learn more. -pub type RustFutureForeignWakerFunction = unsafe extern "C" fn(env: *const c_void); + fn schedule_do_wake(self: Pin>) { + unsafe { + let handle = self.executor.handle; + let raw_ptr = Arc::into_raw(Pin::into_inner_unchecked(self)) as *const (); + // SAFETY: The `into_raw()` / `from_raw()` contract guarantees that our executor cannot + // be dropped before we call `from_raw()` on the raw pointer. This means we can safely + // use its handle to schedule a callback. + schedule_raw(handle, 0, Self::wake_callback, raw_ptr); + } + } -#[derive(Debug)] -struct RustFutureForeignWaker { - waker: RustFutureForeignWakerFunction, - waker_environment: *const c_void, -} + extern "C" fn wake_callback(self_ptr: *const ()) { + unsafe { + Pin::new_unchecked(Arc::from_raw(self_ptr as *const Self)).do_wake(); + }; + } -impl RustFutureForeignWaker { - fn new(waker: RustFutureForeignWakerFunction, waker_environment: *const c_void) -> Self { - Self { - waker, - waker_environment, - } + // Does the work for wake, we take care to ensure this always runs in a serialized fashion. + fn do_wake(self: Pin>) { + // Store 1 in `waker_counter`, which we'll use at the end of this call. + self.wake_counter.store(1, Ordering::Relaxed); + + // Pin<&mut> from our UnsafeCell. &mut is is safe, since this is the only reference we + // ever take to `self.future` and calls to this function are serialized. Pin<> is safe + // since we never move the future out of `self.future`. + let future = unsafe { Pin::new_unchecked(&mut *self.future.get()) }; + let waker = self.make_waker(); + + // Run the poll and lift the result if it's ready + let mut out_status = RustCallStatus::default(); + let result: Option> = rust_call_with_out_status( + &mut out_status, + // This closure uses a `&mut F` value, which means it's not UnwindSafe by default. If + // the closure panics, the future may be in an invalid state. + // + // However, we can safely use `AssertUnwindSafe` since a panic will lead the `Err()` + // case below. In that case, we will never run `do_wake()` again and will no longer + // access the future. + panic::AssertUnwindSafe(|| match future.poll(&mut Context::from_waker(&waker)) { + Poll::Pending => Ok(Poll::Pending), + Poll::Ready(v) => T::lower_return(v).map(Poll::Ready), + }), + ); + + // All the main work is done, time to finish up + match result { + Some(Poll::Pending) => { + // Since we set `wake_counter` to 1 at the start of this function... + // - If it's > 1 now, then some other code tried to wake us while we were polling + // the future. Schedule another poll in this case. + // - If it's still 1, then exit after decrementing it. The next call to `wake()` + // will schedule a poll. + if self.wake_counter.fetch_sub(1, Ordering::Relaxed) > 1 { + self.schedule_do_wake(); + } + } + // Success! Call our callback. + // + // Don't decrement `wake_counter'. This way, if wake() is called in the future, we + // will just ignore it + Some(Poll::Ready(v)) => { + T::invoke_future_callback(self.callback, self.callback_data, v, out_status); + } + // Error/panic polling the future. Call the callback with a default value. + // `out_status` contains the error code and serialized error. Again, don't decrement + // `wake_counter'. + None => { + T::invoke_future_callback( + self.callback, + self.callback_data, + T::ReturnType::ffi_default(), + out_status, + ); + } + }; } - fn into_unit_ptr(self) -> *const () { - Arc::into_raw(Arc::new(self)) as *const () + fn make_waker(self: &Pin>) -> Waker { + // This is safe as long as we implement the waker interface correctly. + unsafe { + Waker::from_raw(RawWaker::new( + self.clone().into_raw(), + &Self::RAW_WAKER_VTABLE, + )) + } } - unsafe fn increment_reference_count(me: *const ()) { - Arc::increment_strong_count(me as *const Self); + /// SAFETY: Ensure that all calls to `into_raw()` are balanced with a call to `from_raw()` + fn into_raw(self: Pin>) -> *const () { + unsafe { Arc::into_raw(Pin::into_inner_unchecked(self)) as *const () } } - unsafe fn from_unit_ptr(me: *const ()) -> Arc { - Arc::from_raw(me as *const Self) + /// Consume a pointer to get an arc + /// + /// SAFETY: The pointer must have come from `into_raw()` or was cloned with `raw_clone()`. + /// Once a pointer is passed into this function, it should no longer be used. + fn from_raw(self_ptr: *const ()) -> Pin> { + unsafe { Pin::new_unchecked(Arc::from_raw(self_ptr as *const Self)) } } -} -/// Zero-sized type to create the VTable -/// ([Virtual method table](https://en.wikipedia.org/wiki/Virtual_method_table)) -/// for the `RawWaker`. -struct RustFutureForeignRawWaker; - -impl RustFutureForeignRawWaker { - const VTABLE: RawWakerVTable = RawWakerVTable::new( - Self::clone_waker, - Self::wake, - Self::wake_by_ref, - Self::drop_waker, + // Implement the waker interface by defining a RawWakerVTable + // + // We could also handle this by implementing the `Wake` interface, but that uses an `Arc` + // instead of a `Pin>` and in theory it could try to move the `T` value out of the arc + // with something like `Arc::try_unwrap()` which would break the pinning contract with + // `Future`. Implementing this using `RawWakerVTable` allows us verify that this doesn't + // happen. + const RAW_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new( + Self::raw_clone, + Self::raw_wake, + Self::raw_wake_by_ref, + Self::raw_drop, ); /// This function will be called when the `RawWaker` gets cloned, e.g. when /// the `Waker` in which the `RawWaker` is stored gets cloned. - unsafe fn clone_waker(foreign_waker: *const ()) -> RawWaker { - RustFutureForeignWaker::increment_reference_count(foreign_waker); - - RawWaker::new(foreign_waker, &Self::VTABLE) + unsafe fn raw_clone(self_ptr: *const ()) -> RawWaker { + Arc::increment_strong_count(self_ptr as *const Self); + RawWaker::new(self_ptr, &Self::RAW_WAKER_VTABLE) } /// This function will be called when `wake` is called on the `Waker`. It /// must wake up the task associated with this `RawWaker`. - unsafe fn wake(foreign_waker: *const ()) { - let waker = RustFutureForeignWaker::from_unit_ptr(foreign_waker); - let func = waker.waker; - - func(waker.waker_environment); + unsafe fn raw_wake(self_ptr: *const ()) { + Self::from_raw(self_ptr).wake() } /// This function will be called when `wake_by_ref` is called on the /// `Waker`. It must wake up the task associated with this `RawWaker`. - unsafe fn wake_by_ref(foreign_waker: *const ()) { - let waker = ManuallyDrop::new(RustFutureForeignWaker::from_unit_ptr(foreign_waker)); - let func = waker.waker; - - func(waker.waker_environment); + unsafe fn raw_wake_by_ref(self_ptr: *const ()) { + // This could be optimized by only incrementing the strong count if we end up calling + // `schedule_do_wake()`, but it's not clear that's worth the extra complexity + Arc::increment_strong_count(self_ptr as *const Self); + Self::from_raw(self_ptr).wake() } /// This function gets called when a `RawWaker` gets dropped. - unsafe fn drop_waker(foreign_waker: *const ()) { - drop(RustFutureForeignWaker::from_unit_ptr(foreign_waker)); + /// This function gets called when a `RawWaker` gets dropped. + unsafe fn raw_drop(self_ptr: *const ()) { + drop(Self::from_raw(self_ptr)) } } #[cfg(test)] -mod tests_raw_waker_vtable { +mod tests { use super::*; - use std::{cell::RefCell, ptr}; + use crate::MockExecutor; + use std::sync::Weak; - // This entire `RustFuture` stuff assumes the waker lives in the foreign - // language, but for the sake of testing, we will fake it. - extern "C" fn my_waker(env: *const c_void) { - let env = ManuallyDrop::new(unsafe { Box::from_raw(env as *mut RefCell) }); - env.replace(true); + // Mock future that we can manually control using an Option<> + struct MockFuture(Option>); - // do something smart! - } - - #[test] - fn test_rust_future_foreign_waker_basic_manipulations() { - let foreign_waker_ptr = - RustFutureForeignWaker::new(my_waker as _, ptr::null()).into_unit_ptr(); - let foreign_waker: Arc = - unsafe { RustFutureForeignWaker::from_unit_ptr(foreign_waker_ptr) }; + impl Future for MockFuture { + type Output = Result; - assert_eq!(Arc::strong_count(&foreign_waker), 1); + fn poll(self: Pin<&mut Self>, _context: &mut Context<'_>) -> Poll { + match &self.0 { + Some(v) => Poll::Ready(v.clone()), + None => Poll::Pending, + } + } } - #[test] - fn test_clone_and_drop_waker() { - let foreign_waker_ptr = - RustFutureForeignWaker::new(my_waker as _, ptr::null()).into_unit_ptr(); - let foreign_waker = unsafe { RustFutureForeignWaker::from_unit_ptr(foreign_waker_ptr) }; - - let _ = unsafe { RustFutureForeignRawWaker::clone_waker(foreign_waker_ptr) }; - assert_eq!(Arc::strong_count(&foreign_waker), 2); + // Type alias for the RustFuture we'll use in our tests + type TestRustFuture = RustFuture, crate::UniFfiTag>; - unsafe { RustFutureForeignRawWaker::drop_waker(foreign_waker_ptr) }; - assert_eq!(Arc::strong_count(&foreign_waker), 1); + // Stores the result that we send to the foreign code + #[derive(Default)] + struct MockForeignResult { + value: i8, + status: RustCallStatus, } - #[test] - fn test_wake() { - let foreign_waker_environment_ptr = Box::into_raw(Box::new(RefCell::new(false))); - let foreign_waker_ptr = - RustFutureForeignWaker::new(my_waker as _, foreign_waker_environment_ptr as *const _) - .into_unit_ptr(); - let foreign_waker = unsafe { RustFutureForeignWaker::from_unit_ptr(foreign_waker_ptr) }; - - // Clone to increase the strong count, so that we can see if it's been dropped by `wake` later. - let _ = unsafe { RustFutureForeignRawWaker::clone_waker(foreign_waker_ptr) }; - assert_eq!(Arc::strong_count(&foreign_waker), 2); - - // Let's call the waker. - unsafe { RustFutureForeignRawWaker::wake(foreign_waker_ptr) }; - - // Has it been called? - let foreign_waker_environment = unsafe { Box::from_raw(foreign_waker_environment_ptr) }; - assert!(foreign_waker_environment.take()); + extern "C" fn mock_foreign_callback(data_ptr: *const (), value: i8, status: RustCallStatus) { + println!("mock_foreign_callback: {value} {data_ptr:?}"); + let result: &mut Option = + unsafe { &mut *(data_ptr as *mut Option) }; + *result = Some(MockForeignResult { value, status }); + } - // Has the waker been dropped? - assert_eq!(Arc::strong_count(&foreign_waker), 1); + // Bundles everything together so that we can run some tests + struct TestFutureEnvironment { + rust_future: Pin>, + executor: MockExecutor, + foreign_result: Pin>>, } - #[test] - fn test_wake_by_ref() { - let foreign_waker_environment_ptr = Box::into_raw(Box::new(RefCell::new(false))); - let foreign_waker_ptr = - RustFutureForeignWaker::new(my_waker as _, foreign_waker_environment_ptr as *const _) - .into_unit_ptr(); - let foreign_waker = unsafe { RustFutureForeignWaker::from_unit_ptr(foreign_waker_ptr) }; + impl TestFutureEnvironment { + fn new() -> Self { + let foreign_result = Box::pin(None); + let foreign_result_ptr = &*foreign_result as *const Option<_> as *const (); + let executor = MockExecutor::new(); + let rust_future = TestRustFuture::new( + MockFuture(None), + executor.handle().unwrap(), + mock_foreign_callback, + foreign_result_ptr, + ); + Self { + executor, + rust_future, + foreign_result, + } + } - // Clone to increase the strong count, so that we can see if it has not been dropped by `wake_by_ref` later. - let _ = unsafe { RustFutureForeignRawWaker::clone_waker(foreign_waker_ptr) }; - assert_eq!(Arc::strong_count(&foreign_waker), 2); + fn scheduled_call_count(&self) -> usize { + self.executor.call_count() + } - // Let's call the waker by reference. - unsafe { RustFutureForeignRawWaker::wake_by_ref(foreign_waker_ptr) }; + fn run_scheduled_calls(&self) { + self.executor.run_all_calls(); + } - // Has it been called? - let foreign_waker_environment = unsafe { Box::from_raw(foreign_waker_environment_ptr) }; - assert!(foreign_waker_environment.take()); + fn wake(&self) { + self.rust_future.clone().wake(); + } - // Has the waker not been dropped? - assert_eq!(Arc::strong_count(&foreign_waker), 2); + fn rust_future_weak(&self) -> Weak { + // It seems like there's not a great way to get an &Arc from a Pin, so we need to + // do a little dance here + Arc::downgrade(&Pin::into_inner(Clone::clone(&self.rust_future))) + } - // Dropping manually to avoid data leak. - unsafe { RustFutureForeignRawWaker::drop_waker(foreign_waker_ptr) }; + fn complete_future(&self, value: Result) { + unsafe { + (*self.rust_future.future.get()).0 = Some(value); + } + } } -} -const READY: bool = true; -const PENDING: bool = false; + #[test] + fn test_wake() { + let mut test_env = TestFutureEnvironment::new(); + // Initially, we shouldn't have a result and nothing should be scheduled + assert!(test_env.foreign_result.is_none()); + assert_eq!(test_env.scheduled_call_count(), 0); + + // wake() should schedule a call + test_env.wake(); + assert_eq!(test_env.scheduled_call_count(), 1); + + // When that call runs, we should still not have a result yet + test_env.run_scheduled_calls(); + assert!(test_env.foreign_result.is_none()); + assert_eq!(test_env.scheduled_call_count(), 0); + + // Multiple wakes should only result in 1 scheduled call + test_env.wake(); + test_env.wake(); + assert_eq!(test_env.scheduled_call_count(), 1); + + // Make the future ready, which should call mock_foreign_callback and set the result + test_env.complete_future(Ok(true)); + test_env.run_scheduled_calls(); + let result = test_env + .foreign_result + .take() + .expect("Expected result to be set"); + assert_eq!(result.value, 1); + assert_eq!(result.status.code, 0); + assert_eq!(test_env.scheduled_call_count(), 0); + + // Future wakes shouldn't schedule any calls + test_env.wake(); + assert_eq!(test_env.scheduled_call_count(), 0); + } -/// Poll a [`RustFuture`]. If the `RustFuture` is ready, the function returns -/// `true` and puts the result inside `polled_result`, otherwise it returns -/// `false` and _doesn't modify_ the value inside `polled_result`. A third -/// case exists: if the `RustFuture` is throwing an error, the function returns -/// `true` but doesn't modify `polled_result` either, however the value -/// of `call_status` is changed appropriately. It is summarized inside the -/// following table: -/// -/// | `RustFuture`'s state | `polled_result` | `call_status.code` | returned value | -/// |----------------------|-------------------------|--------------------|----------------| -/// | `Ready(Ok(T))` | is mapped to `T::lower` | `CALL_SUCCESS` | `true` | -/// | `Ready(Err(E))` | is not modified | `CALL_ERROR` | `true` | -/// | `Pending` | is not modified | `CALL_SUCCESS` | `false` | -/// -/// Please see the module documentation to learn more. -/// -/// # Panics -/// -/// The function panics if `future` or `waker` is a NULL pointer. -pub fn uniffi_rustfuture_poll( - future: Option<&mut RustFuture>, - waker: Option, - waker_environment: *const c_void, - polled_result: &mut MaybeUninit, - call_status: &mut RustCallStatus, -) -> bool -where - T: FfiConverter, -{ - // If polling the future panics, an error will be recorded in call_status and the future will - // be dropped, so there is no potential for observing any inconsistent state in it. - let mut future = panic::AssertUnwindSafe(future.expect("`future` is a null pointer")); - let waker = waker.expect("`waker` is a null pointer"); - - match rust_call_with_out_status(call_status, move || { - // Poll the future and convert the value into a Result - Ok(match future.poll(waker, waker_environment) { - Poll::Ready(v) => Poll::Ready(T::lower_return(v)?), - Poll::Pending => Poll::Pending, - }) - }) { - Some(Poll::Ready(v)) => { - polled_result.write(v); - READY + #[test] + fn test_error() { + let mut test_env = TestFutureEnvironment::new(); + test_env.complete_future(Err("Something went wrong".into())); + test_env.wake(); + test_env.run_scheduled_calls(); + let result = test_env + .foreign_result + .take() + .expect("Expected result to be set"); + assert_eq!(result.status.code, 1); + unsafe { + assert_eq!( + >::try_lift( + result.status.error_buf.assume_init() + ) + .unwrap(), + String::from("Something went wrong"), + ) } - Some(Poll::Pending) => PENDING, - // Error return or panic. `rust_call()` has updated `call_status` with the error status and - // set `error_buf`. Return READY, since we don't want the foreign code polling the future - // anymore - None => READY, + assert_eq!(test_env.scheduled_call_count(), 0); } -} -/// Drop a [`RustFuture`]. -/// -/// Because this function takes ownership of `future` (by `Box`ing it), the -/// future will be dropped naturally by the compiler at the end of the function -/// scope. -/// -/// Please see the module documentation to learn more. -pub fn uniffi_rustfuture_drop( - _future: Option>>, - _call_status: &mut RustCallStatus, -) { + #[test] + fn test_raw_clone_and_drop() { + let test_env = TestFutureEnvironment::new(); + let waker = test_env.rust_future.make_waker(); + let weak_ref = test_env.rust_future_weak(); + assert_eq!(weak_ref.strong_count(), 2); + let waker2 = waker.clone(); + assert_eq!(weak_ref.strong_count(), 3); + drop(waker); + assert_eq!(weak_ref.strong_count(), 2); + drop(waker2); + assert_eq!(weak_ref.strong_count(), 1); + drop(test_env); + assert_eq!(weak_ref.strong_count(), 0); + assert!(weak_ref.upgrade().is_none()); + } + + #[test] + fn test_raw_wake() { + let test_env = TestFutureEnvironment::new(); + let waker = test_env.rust_future.make_waker(); + let weak_ref = test_env.rust_future_weak(); + // `test_env` and `waker` both hold a strong reference to the `RustFuture` + assert_eq!(weak_ref.strong_count(), 2); + + // wake_by_ref() should schedule a wake + waker.wake_by_ref(); + assert_eq!(test_env.scheduled_call_count(), 1); + + // Once the wake runs, the strong could should not have changed + test_env.run_scheduled_calls(); + assert_eq!(weak_ref.strong_count(), 2); + + // wake() should schedule a wake + waker.wake(); + assert_eq!(test_env.scheduled_call_count(), 1); + + // Once the wake runs, the strong have decremented, since wake() consumes the waker + test_env.run_scheduled_calls(); + assert_eq!(weak_ref.strong_count(), 1); + } } diff --git a/uniffi_core/src/ffi_converter_impls.rs b/uniffi_core/src/ffi_converter_impls.rs index 34c96dd782..68861cad67 100644 --- a/uniffi_core/src/ffi_converter_impls.rs +++ b/uniffi_core/src/ffi_converter_impls.rs @@ -4,7 +4,8 @@ use crate::{ check_remaining, ffi_converter_default_return, ffi_converter_rust_buffer_lift_and_lower, - metadata, FfiConverter, Interface, MetadataBuffer, Result, RustBuffer, + metadata, FfiConverter, FutureCallback, Interface, MetadataBuffer, Result, RustBuffer, + RustCallStatus, }; /// This module contains builtin `FFIConverter` implementations. These cover: /// - Simple privitive types: u8, i32, String, Arc, etc @@ -113,24 +114,47 @@ unsafe impl FfiConverter for bool { const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_BOOL); } -/// Support for passing the unit type via the FFI. This is currently only used for void returns +/// Support unit-type returns via the FFI. +/// +/// This is currently supported for function returns. Unit types can not be passed as arguments. unsafe impl FfiConverter for () { - ffi_converter_default_return!(UT); - + // This actually isn't used, but we need to specify something type FfiType = (); + // Returning `()` is FFI-safe, since it gets translated into a void return + type ReturnType = (); + // However, we can't use `FutureCallback<()>` since passing `()` as an argument is not + // FFI-safe. So we used an arbitrary non-ZST type instead. + type FutureCallback = FutureCallback; fn try_lift(_: Self::FfiType) -> Result<()> { - Ok(()) + panic!("lifting the unit type is not allowed") } - fn lower(_: ()) -> Self::FfiType {} + fn lower(_: ()) -> Self::FfiType { + panic!("lowering the unit type is not allowed") + } - fn write(_: (), _: &mut Vec) {} + fn write(_: (), _: &mut Vec) { + panic!("writing the unit type is not allowed") + } fn try_read(_: &mut &[u8]) -> Result<()> { + panic!("reading the unit type is not allowed") + } + + fn lower_return(_: ()) -> Result { Ok(()) } + fn invoke_future_callback( + callback: Self::FutureCallback, + callback_data: *const (), + _return_value: (), + call_status: RustCallStatus, + ) { + callback(callback_data, 0, call_status) + } + const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_UNIT); } @@ -529,6 +553,7 @@ where { type FfiType = (); // Placeholder while lower/lift/serializing is unimplemented type ReturnType = R::ReturnType; + type FutureCallback = R::FutureCallback; fn try_lift(_: Self::FfiType) -> Result { unimplemented!(); @@ -553,6 +578,15 @@ where } } + fn invoke_future_callback( + callback: Self::FutureCallback, + callback_data: *const (), + return_value: Self::ReturnType, + call_status: RustCallStatus, + ) { + R::invoke_future_callback(callback, callback_data, return_value, call_status) + } + const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_RESULT) .concat(R::TYPE_ID_META) .concat(E::TYPE_ID_META); diff --git a/uniffi_core/src/lib.rs b/uniffi_core/src/lib.rs index 2b5e2b3c5d..3a48b5c6a0 100644 --- a/uniffi_core/src/lib.rs +++ b/uniffi_core/src/lib.rs @@ -49,6 +49,8 @@ pub use metadata::*; // so the consumer doesn't have to depend on them directly. pub mod deps { pub use anyhow; + #[cfg(feature = "tokio")] + pub use async_compat; pub use bytes; pub use log; pub use static_assertions; @@ -183,6 +185,12 @@ pub unsafe trait FfiConverter: Sized { /// This is usually the same as `FfiType`, but `Result<>` has specialized handling. type ReturnType: FfiDefault; + /// The `FutureCallback` type used for async functions + /// + /// This is almost always `FutureCallback`. The one exception is the + /// unit type, see that `FfiConverter` impl for details. + type FutureCallback: Copy; + /// Lower a rust value of the target type, into an FFI value of type Self::FfiType. /// /// This trait method is used for sending data from rust to the foreign language code, @@ -235,6 +243,14 @@ pub unsafe trait FfiConverter: Sized { /// from it (but will not mutate the actual contents of the slice). fn try_read(buf: &mut &[u8]) -> Result; + /// Invoke a `FutureCallback` to complete a async call + fn invoke_future_callback( + callback: Self::FutureCallback, + callback_data: *const (), + return_value: Self::ReturnType, + call_status: RustCallStatus, + ); + /// Type ID metadata, serialized into a [MetadataBuffer] const TYPE_ID_META: MetadataBuffer; } @@ -287,10 +303,20 @@ where macro_rules! ffi_converter_default_return { ($uniffi_tag:ty) => { type ReturnType = >::FfiType; + type FutureCallback = $crate::FutureCallback; fn lower_return(v: Self) -> ::std::result::Result { Ok(>::lower(v)) } + + fn invoke_future_callback( + callback: Self::FutureCallback, + callback_data: *const (), + return_value: Self::ReturnType, + call_status: $crate::RustCallStatus, + ) { + callback(callback_data, return_value, call_status); + } }; } @@ -333,6 +359,7 @@ macro_rules! ffi_converter_forward { unsafe impl $crate::FfiConverter<$new_impl_tag> for $T { type FfiType = <$T as $crate::FfiConverter<$existing_impl_tag>>::FfiType; type ReturnType = <$T as $crate::FfiConverter<$existing_impl_tag>>::FfiType; + type FutureCallback = <$T as $crate::FfiConverter<$existing_impl_tag>>::FutureCallback; fn lower(obj: $T) -> Self::FfiType { <$T as $crate::FfiConverter<$existing_impl_tag>>::lower(obj) @@ -356,6 +383,20 @@ macro_rules! ffi_converter_forward { <$T as $crate::FfiConverter<$existing_impl_tag>>::try_read(buf) } + fn invoke_future_callback( + callback: Self::FutureCallback, + callback_data: *const (), + return_value: Self::ReturnType, + call_status: $crate::RustCallStatus, + ) { + <$T as $crate::FfiConverter<$existing_impl_tag>>::invoke_future_callback( + callback, + callback_data, + return_value, + call_status, + ) + } + const TYPE_ID_META: ::uniffi::MetadataBuffer = <$T as $crate::FfiConverter<$existing_impl_tag>>::TYPE_ID_META; } diff --git a/uniffi_macros/src/export/scaffolding.rs b/uniffi_macros/src/export/scaffolding.rs index 428fba8d5e..1303aa87f0 100644 --- a/uniffi_macros/src/export/scaffolding.rs +++ b/uniffi_macros/src/export/scaffolding.rs @@ -99,14 +99,14 @@ pub(super) fn gen_method_scaffolding( bits.add_self_param(quote! { this: #ffi_converter::FfiType }); // This is followed by the method arguments bits.collect_params(sig.inputs.iter().skip(1), RECEIVER_ERROR); - // To call the method: - // - lift the `this` param to get the object - // - Add `.#ident` to get the method - bits.set_rust_fn_call(quote! { - #ffi_converter::try_lift(this).unwrap_or_else(|err| { + // Lift self before calling the method. This avoids any issues with temporary + // references. + bits.set_pre_fn_call(quote! { + let this = #ffi_converter::try_lift(this).unwrap_or_else(|err| { ::std::panic!("Failed to convert arg 'self': {}", err) - }).#ident + }); }); + bits.set_rust_fn_call(quote! { this.#ident }); bits } // Associated functions @@ -138,8 +138,10 @@ struct ScaffoldingBits { /// Tokenstream that represents the function to call /// /// For functions, this is simple the function ident. - /// For methods, this will lift for the `self` param, followed by the method name. + /// For methods, this will be `self.{method_name}`. rust_fn_call: Option, + /// Tokenstream of statements that we should have before `rust_fn_call` + pre_fn_call: Option, /// Parameters for the scaffolding function params: Vec, /// Expressions to lift the arguments in order to pass them to the exported function @@ -152,6 +154,7 @@ impl ScaffoldingBits { fn new() -> Self { Self { rust_fn_call: None, + pre_fn_call: None, params: vec![], param_lifts: vec![], arg_metadata_calls: vec![], @@ -233,6 +236,10 @@ impl ScaffoldingBits { self.rust_fn_call = Some(rust_fn_call); } + fn set_pre_fn_call(&mut self, pre_fn_call: TokenStream) { + self.pre_fn_call = Some(pre_fn_call) + } + fn add_self_param(&mut self, param: TokenStream) { self.params.insert(0, param); } @@ -371,6 +378,7 @@ fn gen_ffi_function( arguments: &ExportAttributeArguments, ) -> TokenStream { let name = ident_to_string(&sig.ident); + let pre_fn_call = &bits.pre_fn_call; let rust_fn_call = bits.rust_fn_call(); let fn_params = &bits.params; let return_ty = &sig.output; @@ -393,52 +401,39 @@ fn gen_ffi_function( ) -> <#return_ty as ::uniffi::FfiConverter>::ReturnType { ::uniffi::deps::log::debug!(#name); ::uniffi::rust_call(call_status, || { + #pre_fn_call <#return_ty as ::uniffi::FfiConverter>::lower_return(#rust_fn_call) }) } } } else { - let rust_future_ctor = match &arguments.async_runtime { - Some(AsyncRuntime::Tokio(_)) => quote! { new_tokio }, - None => quote! { new }, - }; - let ffi_poll_ident = format_ident!("{ffi_ident}_poll"); - let ffi_drop_ident = format_ident!("{ffi_ident}_drop"); + let mut future_expr = rust_fn_call; + if matches!(arguments.async_runtime, Some(AsyncRuntime::Tokio(_))) { + future_expr = quote! { ::uniffi::deps::async_compat::Compat::new(#future_expr) } + } quote! { #[doc(hidden)] #[no_mangle] pub extern "C" fn #ffi_ident( #(#fn_params,)* - call_status: &mut ::uniffi::RustCallStatus, - ) -> ::std::boxed::Box<::uniffi::RustFuture<#return_ty>> { - ::uniffi::deps::log::debug!(#name); - ::std::boxed::Box::new(::uniffi::RustFuture::#rust_future_ctor( - async move { #rust_fn_call.await } - )) - } - - // Monomorphised poll function. - #[doc(hidden)] - #[no_mangle] - pub extern "C" fn #ffi_poll_ident( - future: ::std::option::Option<&mut ::uniffi::RustFuture<#return_ty>>, - waker: ::std::option::Option<::uniffi::RustFutureForeignWakerFunction>, - waker_environment: *const ::std::ffi::c_void, - polled_result: &mut ::std::mem::MaybeUninit<<#return_ty as ::uniffi::FfiConverter>::ReturnType>, - call_status: &mut ::uniffi::RustCallStatus, - ) -> ::std::primitive::bool { - ::uniffi::ffi::uniffi_rustfuture_poll::<_, crate::UniFfiTag>(future, waker, waker_environment, polled_result, call_status) - } - - // Monomorphised drop function. - #[doc(hidden)] - #[no_mangle] - pub extern "C" fn #ffi_drop_ident( - future: ::std::option::Option<::std::boxed::Box<::uniffi::RustFuture<#return_ty>>>, - call_status: &mut ::uniffi::RustCallStatus, + uniffi_executor_handle: ::uniffi::ForeignExecutorHandle, + uniffi_callback: <#return_ty as ::uniffi::FfiConverter>::FutureCallback, + uniffi_callback_data: *const (), + uniffi_call_status: &mut ::uniffi::RustCallStatus, ) { - ::uniffi::ffi::uniffi_rustfuture_drop(future, call_status) + ::uniffi::deps::log::debug!(#name); + ::uniffi::rust_call(uniffi_call_status, || { + #pre_fn_call; + let uniffi_rust_future = ::uniffi::RustFuture::<_, #return_ty, crate::UniFfiTag>::new( + #future_expr, + uniffi_executor_handle, + uniffi_callback, + uniffi_callback_data + ); + uniffi_rust_future.wake(); + Ok(()) + }); } } } From 2807a357c9b0dd08cba2ec13cbb4c2e53dd12414 Mon Sep 17 00:00:00 2001 From: bendk Date: Mon, 24 Apr 2023 14:05:13 -0400 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Jonas Platte Co-authored-by: Ivan Enderlin --- fixtures/foreign-executor/Cargo.toml | 6 +++--- fixtures/foreign-executor/src/lib.rs | 1 + .../src/bindings/kotlin/gen_kotlin/mod.rs | 14 ++++++-------- .../src/bindings/python/gen_python/mod.rs | 5 +---- uniffi_bindgen/src/interface/ffi.rs | 2 +- uniffi_core/src/ffi/rustfuture.rs | 2 +- 6 files changed, 13 insertions(+), 17 deletions(-) diff --git a/fixtures/foreign-executor/Cargo.toml b/fixtures/foreign-executor/Cargo.toml index 6ee6bc02ca..adc6741fdb 100644 --- a/fixtures/foreign-executor/Cargo.toml +++ b/fixtures/foreign-executor/Cargo.toml @@ -10,10 +10,10 @@ crate-type = ["lib", "cdylib"] name = "uniffi_fixture_foreign_executor" [dependencies] -uniffi = {path = "../../uniffi"} +uniffi = { path = "../../uniffi" } [build-dependencies] -uniffi = {path = "../../uniffi", features = ["build"] } +uniffi = { path = "../../uniffi", features = ["build"] } [dev-dependencies] -uniffi = {path = "../../uniffi", features = ["bindgen-tests"] } +uniffi = { path = "../../uniffi", features = ["bindgen-tests"] } diff --git a/fixtures/foreign-executor/src/lib.rs b/fixtures/foreign-executor/src/lib.rs index 8e31946217..5b3f5a0427 100644 --- a/fixtures/foreign-executor/src/lib.rs +++ b/fixtures/foreign-executor/src/lib.rs @@ -61,6 +61,7 @@ impl ForeignExecutorTester { self.last_result.lock().unwrap().take() } } + #[derive(uniffi::Record)] pub struct TestResult { pub call_happened_in_different_thread: bool, diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index acfaadc0d3..2d4b33f642 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -324,7 +324,7 @@ impl CodeOracle for KotlinCodeOracle { FfiType::ForeignExecutorHandle => "USize".to_string(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback".to_string(), FfiType::FutureCallback { return_type } => { - format!("UniFfiFutureCallback{}", return_type.canonical_name(),) + format!("UniFfiFutureCallback{}", return_type.canonical_name()) } FfiType::FutureCallbackData => "USize".to_string(), } @@ -395,13 +395,11 @@ pub mod filters { } pub fn future_continuation_type(result_type: &ResultType) -> Result { - Ok(format!( - "Continuation<{}>", - match &result_type.return_type { - Some(t) => type_name(t)?, - None => "Unit".into(), - } - )) + let return_type_name = match &result_type.return_type { + Some(t) => type_name(t)?, + None => "Unit".into(), + }; + Ok(format!("Continuation<{return_type_name}>")) } pub fn render_literal( diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index bb0a1db9f1..28dac5f34a 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -426,10 +426,7 @@ pub mod filters { Some(t) => t.canonical_name().to_snake_case(), None => "void".into(), }; - Ok(format!( - "uniffi_async_callback_{}__{}", - return_string, throws_string - )) + Ok(format!("uniffi_async_callback_{return_string}__{throws_string}")) } pub fn literal_py( diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index 5376ce692a..19f64b92f8 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -86,7 +86,7 @@ impl FfiType { Self::ForeignExecutorHandle => "ForeignExecutorHandle".into(), Self::ForeignExecutorCallback => "ForeignExecutorCallback".into(), Self::FutureCallback { return_type } => { - format!("FutureCallback{}", return_type.canonical_name(),) + format!("FutureCallback{}", return_type.canonical_name()) } Self::FutureCallbackData => "FutureCallbackData".into(), } diff --git a/uniffi_core/src/ffi/rustfuture.rs b/uniffi_core/src/ffi/rustfuture.rs index 75a5787fe0..ce46755f67 100644 --- a/uniffi_core/src/ffi/rustfuture.rs +++ b/uniffi_core/src/ffi/rustfuture.rs @@ -201,7 +201,7 @@ where /// Wake up soon and poll our future. /// - /// This method ensures that a call to `do_wake()` is scheduled. One one call will be scheduled + /// This method ensures that a call to `do_wake()` is scheduled. Only one call will be scheduled /// at any time, even if `wake_soon` called multiple times from multiple threads. pub fn wake(self: Pin>) { if self.wake_counter.fetch_add(1, Ordering::Relaxed) == 0 { From f04ed3257de746b3d5cd7cf4c0329444a49866e2 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Mon, 24 Apr 2023 14:28:02 -0400 Subject: [PATCH 4/5] More fixes after review - Updated the Kotlin `future_callback_handler` function based on jplatte's feedback. - Ran `cargo fmt` - Go back to disabling the kotlin/swift async tests. The PR has a couple approvals and I don't want to block on this. --- .../tests/test_generated_bindings.rs | 7 +++++-- .../src/bindings/kotlin/gen_kotlin/mod.rs | 18 +++++++++--------- .../src/bindings/python/gen_python/mod.rs | 4 +++- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/fixtures/foreign-executor/tests/test_generated_bindings.rs b/fixtures/foreign-executor/tests/test_generated_bindings.rs index 46605c0e93..7cda53700b 100644 --- a/fixtures/foreign-executor/tests/test_generated_bindings.rs +++ b/fixtures/foreign-executor/tests/test_generated_bindings.rs @@ -1,5 +1,8 @@ uniffi::build_foreign_language_testcases!( "tests/bindings/test_foreign_executor.py", - "tests/bindings/test_foreign_executor.kts", - "tests/bindings/test_foreign_executor.swift", + // Disable due to the Docker image being outdated for now. + // Please see https://github.com/mozilla/uniffi-rs/pull/1409#issuecomment-1437170423 + // + // "tests/bindings/test_foreign_executor.kts", + // "tests/bindings/test_foreign_executor.swift", ); diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 2d4b33f642..74685ba704 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -381,16 +381,16 @@ pub mod filters { } pub fn future_callback_handler(result_type: &ResultType) -> Result { + let return_component = match &result_type.return_type { + Some(return_type) => return_type.type_label(oracle()), + None => "Void".into(), + }; + let throws_component = match &result_type.throws_type { + Some(throws_type) => format!("_{}", throws_type.type_label(oracle())), + None => "".into(), + }; Ok(format!( - "UniFfiFutureCallbackHandler{}{}", - match &result_type.return_type { - Some(return_type) => return_type.type_label(oracle()), - None => "Void".into(), - }, - match &result_type.throws_type { - Some(throws_type) => format!("_{}", throws_type.type_label(oracle())), - None => "".into(), - }, + "UniFfiFutureCallbackHandler{return_component}{throws_component}" )) } diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index 28dac5f34a..f1be6c8490 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -426,7 +426,9 @@ pub mod filters { Some(t) => t.canonical_name().to_snake_case(), None => "void".into(), }; - Ok(format!("uniffi_async_callback_{return_string}__{throws_string}")) + Ok(format!( + "uniffi_async_callback_{return_string}__{throws_string}" + )) } pub fn literal_py( From 2e5084c6e987b4d73d92885cdc8be9c0029eca4b Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Thu, 11 May 2023 17:37:23 -0400 Subject: [PATCH 5/5] Updates after pulling in the new changes from main - Enable the foreign executor test and make it work with the Swift version we're using in CI - Use canonical_name in future_callback_handler. This is what we should have been using all along, since `type_name()` can output invalid characters like `?`. The recent changes made this break the unit tests. --- .../tests/bindings/test_foreign_executor.swift | 2 +- fixtures/foreign-executor/tests/test_generated_bindings.rs | 7 ++----- uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift index 1c8f6b0030..4a230d0f60 100644 --- a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift +++ b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.swift @@ -6,7 +6,7 @@ import Foundation import fixture_foreign_executor func runTest(tester: ForeignExecutorTester, delay: UInt32) async -> TestResult { - let handle = Task { + let handle = Task { () -> TestResult in tester.scheduleTest(delay: delay) try! await Task.sleep(nanoseconds: numericCast((delay + 10) * 1000000)) return tester.getLastResult()! diff --git a/fixtures/foreign-executor/tests/test_generated_bindings.rs b/fixtures/foreign-executor/tests/test_generated_bindings.rs index 7cda53700b..46605c0e93 100644 --- a/fixtures/foreign-executor/tests/test_generated_bindings.rs +++ b/fixtures/foreign-executor/tests/test_generated_bindings.rs @@ -1,8 +1,5 @@ uniffi::build_foreign_language_testcases!( "tests/bindings/test_foreign_executor.py", - // Disable due to the Docker image being outdated for now. - // Please see https://github.com/mozilla/uniffi-rs/pull/1409#issuecomment-1437170423 - // - // "tests/bindings/test_foreign_executor.kts", - // "tests/bindings/test_foreign_executor.swift", + "tests/bindings/test_foreign_executor.kts", + "tests/bindings/test_foreign_executor.swift", ); diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 74685ba704..8072bfb35a 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -382,11 +382,11 @@ pub mod filters { pub fn future_callback_handler(result_type: &ResultType) -> Result { let return_component = match &result_type.return_type { - Some(return_type) => return_type.type_label(oracle()), + Some(return_type) => return_type.canonical_name(), None => "Void".into(), }; let throws_component = match &result_type.throws_type { - Some(throws_type) => format!("_{}", throws_type.type_label(oracle())), + Some(throws_type) => format!("_{}", throws_type.canonical_name()), None => "".into(), }; Ok(format!(