From 9c5ad98582175e6a1ff97d5396c3005e8637958f Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Tue, 31 Oct 2023 10:42:05 -0400 Subject: [PATCH] Use Handle for passing foreign objects to Rust Consolidated the various handle maps into a single implementation for each language. This handle map works basically the same as all the others, but it's API is based on the `HandleAlloc` trait. Handles have a couple properties: * All foreign handles are odd, which allows us to distinguish between Rust and foreign handles. * For handles store a map ID that can detect when a handle is used with the wrong map. Made all languages always use the handle maps for passing objects. No more trying to leak pointers from to foreign objects. Started updating the ForeignExecutor code to use handles, but this is still a WIP while the ForeignExecutor type is in it's limbo state. --- CHANGELOG.md | 5 +- .../tests/bindings/test_foreign_executor.kts | 8 -- .../src/bindings/kotlin/gen_kotlin/mod.rs | 2 - .../src/bindings/kotlin/templates/Async.kt | 10 +-- .../kotlin/templates/CallbackInterfaceImpl.kt | 4 +- .../templates/CallbackInterfaceRuntime.kt | 45 ++-------- .../templates/ForeignExecutorTemplate.kt | 29 +++---- .../bindings/kotlin/templates/HandleMap.kt | 53 ++++++++++++ .../src/bindings/kotlin/templates/Helpers.kt | 84 +------------------ .../kotlin/templates/ObjectRuntime.kt | 10 +++ .../kotlin/templates/ObjectTemplate.kt | 16 ++-- .../src/bindings/kotlin/templates/wrapper.kt | 3 + .../src/bindings/python/gen_python/mod.rs | 3 - .../src/bindings/python/templates/Async.py | 8 +- .../python/templates/CallbackInterfaceImpl.py | 2 +- .../templates/CallbackInterfaceRuntime.py | 39 +-------- .../templates/ForeignExecutorTemplate.py | 16 ++-- .../bindings/python/templates/HandleMap.py | 64 ++++++++++++++ .../src/bindings/python/templates/Helpers.py | 2 +- .../templates/NamespaceLibraryTemplate.py | 2 +- .../python/templates/ObjectTemplate.py | 10 +-- .../python/templates/PointerManager.py | 68 --------------- .../python/templates/RustBufferTemplate.py | 6 -- .../src/bindings/python/templates/wrapper.py | 4 +- .../src/bindings/ruby/gen_ruby/mod.rs | 5 +- .../src/bindings/swift/gen_swift/mod.rs | 7 +- .../src/bindings/swift/templates/Async.swift | 32 ++----- .../swift/templates/BridgingHeaderTemplate.h | 2 +- .../templates/CallbackInterfaceImpl.swift | 9 +- .../templates/CallbackInterfaceRuntime.swift | 57 ------------- .../templates/CallbackInterfaceTemplate.swift | 18 ++-- .../bindings/swift/templates/HandleMap.swift | 75 +++++++++++++++++ .../bindings/swift/templates/Helpers.swift | 8 ++ .../swift/templates/ObjectTemplate.swift | 7 +- .../bindings/swift/templates/wrapper.swift | 1 + uniffi_bindgen/src/interface/ffi.rs | 22 +++-- uniffi_bindgen/src/interface/mod.rs | 16 +++- uniffi_bindgen/src/interface/object.rs | 4 +- .../templates/CallbackInterfaceTemplate.rs | 4 +- uniffi_core/src/ffi/callbackinterface.rs | 4 +- uniffi_core/src/ffi/ffidefault.rs | 12 --- uniffi_core/src/ffi/foreigncallbacks.rs | 6 +- uniffi_core/src/ffi/foreignexecutor.rs | 38 +++------ uniffi_core/src/ffi/handle.rs | 7 ++ uniffi_core/src/ffi/rustfuture.rs | 34 ++++---- uniffi_core/src/ffi_converter_impls.rs | 20 +---- .../src/export/callback_interface.rs | 10 ++- uniffi_macros/src/setup_scaffolding.rs | 5 +- 48 files changed, 375 insertions(+), 521 deletions(-) create mode 100644 uniffi_bindgen/src/bindings/kotlin/templates/HandleMap.kt create mode 100644 uniffi_bindgen/src/bindings/python/templates/HandleMap.py delete mode 100644 uniffi_bindgen/src/bindings/python/templates/PointerManager.py create mode 100644 uniffi_bindgen/src/bindings/swift/templates/HandleMap.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 86a6e255ed..33c2ab9ff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,10 @@ - The `rust_future_continuation_callback_set` FFI function was removed. `rust_future_poll` now inputs the callback pointer. External bindings authors will need to update their code. - The object handle FFI has changed. External bindings generators will need to update their code to - use the new handle system. + use the new handle system: + * A single `FfiType::Handle` is used for all object handles. + * `FfiType::Handle` is always a 64-bit int. + * Foreign handles must always set the lowest bit of that int. ## v0.25.0 (backend crates: v0.25.0) - (_2023-10-18_) diff --git a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts index 38ee5a7abd..404c06a463 100644 --- a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts +++ b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts @@ -37,11 +37,3 @@ runBlocking { assert(result.delayMs <= 200U) tester.close() } - -// Test that we cleanup when dropping a ForeignExecutor handles -assert(FfiConverterForeignExecutor.handleCount() == 0) -val tester = ForeignExecutorTester(coroutineScope) -val tester2 = ForeignExecutorTester.newFromSequence(listOf(coroutineScope)) -tester.close() -tester2.close() -assert(FfiConverterForeignExecutor.handleCount() == 0) diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index edcfb6e810..7cd8867d14 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -298,12 +298,10 @@ impl KotlinCodeOracle { } FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(), FfiType::ForeignCallback => "ForeignCallback".to_string(), - FfiType::ForeignExecutorHandle => "USize".to_string(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback".to_string(), FfiType::RustFutureContinuationCallback => { "UniFffiRustFutureContinuationCallbackType".to_string() } - FfiType::RustFutureContinuationData => "USize".to_string(), } } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt index 5c5a2ed42a..f2ee0ba0d5 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Async.kt @@ -3,18 +3,18 @@ internal const val UNIFFI_RUST_FUTURE_POLL_READY = 0.toShort() internal const val UNIFFI_RUST_FUTURE_POLL_MAYBE_READY = 1.toShort() -internal val uniffiContinuationHandleMap = UniFfiHandleMap>() +internal val uniffiContinuationHandleMap = UniffiHandleMap>() // FFI type for Rust future continuations internal object uniffiRustFutureContinuationCallback: UniFffiRustFutureContinuationCallbackType { - override fun callback(continuationHandle: USize, pollResult: Short) { - uniffiContinuationHandleMap.remove(continuationHandle)?.resume(pollResult) + override fun callback(continuationHandle: UniffiHandle, pollResult: Short) { + uniffiContinuationHandleMap.consumeHandle(continuationHandle).resume(pollResult) } } internal suspend fun uniffiRustCallAsync( rustFuture: UniffiHandle, - pollFunc: (UniffiHandle, UniFffiRustFutureContinuationCallbackType, USize) -> Unit, + pollFunc: (UniffiHandle, UniFffiRustFutureContinuationCallbackType, UniffiHandle) -> Unit, completeFunc: (UniffiHandle, RustCallStatus) -> F, freeFunc: (UniffiHandle) -> Unit, liftFunc: (F) -> T, @@ -26,7 +26,7 @@ internal suspend fun uniffiRustCallAsync( pollFunc( rustFuture, uniffiRustFutureContinuationCallback, - uniffiContinuationHandleMap.insert(continuation) + uniffiContinuationHandleMap.newHandle(continuation) ) } } while (pollResult != UNIFFI_RUST_FUTURE_POLL_READY); diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt index f1c58ee971..9c0f1be53d 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceImpl.kt @@ -3,11 +3,11 @@ // Implement the foreign callback handler for {{ interface_name }} internal class {{ callback_handler_class }} : ForeignCallback { @Suppress("TooGenericExceptionCaught") - override fun invoke(handle: Handle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int { + override fun invoke(handle: UniffiHandle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int { val cb = {{ ffi_converter_name }}.handleMap.get(handle) return when (method) { IDX_CALLBACK_FREE -> { - {{ ffi_converter_name }}.handleMap.remove(handle) + {{ ffi_converter_name }}.handleMap.consumeHandle(handle) // Successful return // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt index d0e0686322..fc96b224ec 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt @@ -1,39 +1,8 @@ {{- self.add_import("java.util.concurrent.atomic.AtomicLong") }} {{- self.add_import("java.util.concurrent.locks.ReentrantLock") }} -{{- self.add_import("kotlin.concurrent.withLock") }} - -internal typealias Handle = Long -internal class ConcurrentHandleMap( - private val leftMap: MutableMap = mutableMapOf(), -) { - private val lock = java.util.concurrent.locks.ReentrantLock() - private val currentHandle = AtomicLong(0L) - private val stride = 1L - - fun insert(obj: T): Handle = - lock.withLock { - currentHandle.getAndAdd(stride) - .also { handle -> - leftMap[handle] = obj - } - } - - fun get(handle: Handle) = lock.withLock { - leftMap[handle] ?: throw InternalException("No callback in handlemap; this is a Uniffi bug") - } - - fun delete(handle: Handle) { - this.remove(handle) - } - - fun remove(handle: Handle): T? = - lock.withLock { - leftMap.remove(handle) - } -} interface ForeignCallback : com.sun.jna.Callback { - public fun invoke(handle: Handle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int + public fun invoke(handle: UniffiHandle, method: Int, argsData: Pointer, argsLen: Int, outBuf: RustBufferByReference): Int } // Magic number for the Rust proxy to call using the same mechanism as every other method, @@ -44,20 +13,16 @@ internal const val UNIFFI_CALLBACK_SUCCESS = 0 internal const val UNIFFI_CALLBACK_ERROR = 1 internal const val UNIFFI_CALLBACK_UNEXPECTED_ERROR = 2 -public abstract class FfiConverterCallbackInterface: FfiConverter { - internal val handleMap = ConcurrentHandleMap() - - internal fun drop(handle: Handle) { - handleMap.remove(handle) - } +public abstract class FfiConverterCallbackInterface: FfiConverter { + internal val handleMap = UniffiHandleMap() - override fun lift(value: Handle): CallbackInterface { + override fun lift(value: UniffiHandle): CallbackInterface { return handleMap.get(value) } override fun read(buf: ByteBuffer) = lift(buf.getLong()) - override fun lower(value: CallbackInterface) = handleMap.insert(value) + override fun lower(value: CallbackInterface) = handleMap.newHandle(value) override fun allocationSize(value: CallbackInterface) = 8 diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt index 3544b2f9e6..90c84b9fc9 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ForeignExecutorTemplate.kt @@ -16,7 +16,7 @@ internal interface UniFfiRustTaskCallback : com.sun.jna.Callback { } internal object UniFfiForeignExecutorCallback : com.sun.jna.Callback { - fun callback(handle: USize, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) : Byte { + fun callback(handle: UniffiHandle, delayMs: Int, rustTask: UniFfiRustTaskCallback?, rustTaskData: Pointer?) : Byte { if (rustTask == null) { FfiConverterForeignExecutor.drop(handle) return UNIFFI_FOREIGN_EXECUTOR_CALLBACK_SUCCESS @@ -42,11 +42,11 @@ internal object UniFfiForeignExecutorCallback : com.sun.jna.Callback { } } -public object FfiConverterForeignExecutor: FfiConverter { - internal val handleMap = UniFfiHandleMap() +public object FfiConverterForeignExecutor: FfiConverter { + internal val handleMap = UniffiHandleMap() - internal fun drop(handle: USize) { - handleMap.remove(handle) + internal fun drop(handle: UniffiHandle) { + handleMap.consumeHandle(handle) } internal fun register(lib: _UniFFILib) { @@ -58,26 +58,21 @@ public object FfiConverterForeignExecutor: FfiConverter { {% endmatch %} } - // Number of live handles, exposed so we can test the memory management - public fun handleCount() : Int { - return handleMap.size - } - - override fun allocationSize(value: CoroutineScope) = USize.size + override fun allocationSize(value: CoroutineScope) = 8 - override fun lift(value: USize): CoroutineScope { - return handleMap.get(value) ?: throw RuntimeException("unknown handle in FfiConverterForeignExecutor.lift") + override fun lift(value: UniffiHandle): CoroutineScope { + return handleMap.get(value) } override fun read(buf: ByteBuffer): CoroutineScope { - return lift(USize.readFromBuffer(buf)) + return lift(buf.getLong()) } - override fun lower(value: CoroutineScope): USize { - return handleMap.insert(value) + override fun lower(value: CoroutineScope): UniffiHandle { + return handleMap.newHandle(value) } override fun write(value: CoroutineScope, buf: ByteBuffer) { - lower(value).writeToBuffer(buf) + buf.putLong(lower(value)) } } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/HandleMap.kt b/uniffi_bindgen/src/bindings/kotlin/templates/HandleMap.kt new file mode 100644 index 0000000000..427d01f7ca --- /dev/null +++ b/uniffi_bindgen/src/bindings/kotlin/templates/HandleMap.kt @@ -0,0 +1,53 @@ +internal class UniffiHandleMap { + private val lock = ReentrantReadWriteLock() + private var mapId: Long = UniffiHandleMap.nextMapId() + private val map: MutableMap = mutableMapOf() + // Note: Foreign handles are always odd + private var keyCounter = 1L + + private fun nextKey(): Long = keyCounter.also { + keyCounter = (keyCounter + 2L).and(0xFFFF_FFFF_FFFFL) + } + + private fun makeHandle(key: Long): UniffiHandle = key.or(mapId) + + private fun key(handle: UniffiHandle): Long { + if (handle.and(0x7FFF_0000_0000_0000L) != mapId) { + throw InternalException("Handle map ID mismatch") + } + return handle.and(0xFFFF_FFFF_FFFFL) + } + + fun newHandle(obj: T): UniffiHandle = lock.writeLock().withLock { + val key = nextKey() + map[key] = obj + makeHandle(key) + } + + fun get(handle: UniffiHandle) = lock.readLock().withLock { + map[key(handle)] ?: throw InternalException("Missing key in handlemap: was the handle used after being freed?") + } + + fun cloneHandle(handle: UniffiHandle): UniffiHandle = lock.writeLock().withLock { + val obj = map[key(handle)] ?: throw InternalException("Missing key in handlemap: was the handle used after being freed?") + val clone = nextKey() + map[clone] = obj + makeHandle(clone) + } + + fun consumeHandle(handle: UniffiHandle): T = lock.writeLock().withLock { + map.remove(key(handle)) ?: throw InternalException("Missing key in handlemap: was the handle used after being freed?") + } + + companion object { + // Generate map IDs that are likely to be unique + private var mapIdCounter: Long = {{ ci.namespace_hash() }}.and(0x7FFF) + + // Map ID, shifted into the top 16 bits + internal fun nextMapId(): Long = mapIdCounter.shl(48).also { + // On Kotlin, map ids are only 15 bits to get around signed/unsigned issues + mapIdCounter = ((mapIdCounter + 1).and(0x7FFF)) + } + } +} + diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt index 2cf93f5cc2..e91c8e4119 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt @@ -78,89 +78,7 @@ private inline fun rustCall(callback: (RustCallStatus) -> U): U { return rustCallWithError(NullCallStatusErrorHandler, callback); } -// IntegerType that matches Rust's `usize` / C's `size_t` -public class USize(value: Long = 0) : IntegerType(Native.SIZE_T_SIZE, value, true) { - // This is needed to fill in the gaps of IntegerType's implementation of Number for Kotlin. - override fun toByte() = toInt().toByte() - // Needed until https://youtrack.jetbrains.com/issue/KT-47902 is fixed. - @Deprecated("`toInt().toChar()` is deprecated") - override fun toChar() = toInt().toChar() - override fun toShort() = toInt().toShort() - - fun writeToBuffer(buf: ByteBuffer) { - // Make sure we always write usize integers using native byte-order, since they may be - // casted to pointer values - buf.order(ByteOrder.nativeOrder()) - try { - when (Native.SIZE_T_SIZE) { - 4 -> buf.putInt(toInt()) - 8 -> buf.putLong(toLong()) - else -> throw RuntimeException("Invalid SIZE_T_SIZE: ${Native.SIZE_T_SIZE}") - } - } finally { - buf.order(ByteOrder.BIG_ENDIAN) - } - } - - companion object { - val size: Int - get() = Native.SIZE_T_SIZE - - fun readFromBuffer(buf: ByteBuffer) : USize { - // Make sure we always read usize integers using native byte-order, since they may be - // casted from pointer values - buf.order(ByteOrder.nativeOrder()) - try { - return when (Native.SIZE_T_SIZE) { - 4 -> USize(buf.getInt().toLong()) - 8 -> USize(buf.getLong()) - else -> throw RuntimeException("Invalid SIZE_T_SIZE: ${Native.SIZE_T_SIZE}") - } - } finally { - buf.order(ByteOrder.BIG_ENDIAN) - } - } - } -} - - -// Map handles to objects -// -// This is used when the Rust code expects an opaque pointer to represent some foreign object. -// Normally we would pass a pointer to the object, but JNA doesn't support getting a pointer from an -// object reference , nor does it support leaking a reference to Rust. -// -// Instead, this class maps USize values to objects so that we can pass a pointer-sized type to -// Rust when it needs an opaque pointer. -// -// TODO: refactor callbacks to use this class -internal class UniFfiHandleMap { - private val map = ConcurrentHashMap() - // Use AtomicInteger for our counter, since we may be on a 32-bit system. 4 billion possible - // values seems like enough. If somehow we generate 4 billion handles, then this will wrap - // around back to zero and we can assume the first handle generated will have been dropped by - // then. - private val counter = java.util.concurrent.atomic.AtomicInteger(0) - - val size: Int - get() = map.size - - fun insert(obj: T): USize { - val handle = USize(counter.getAndAdd(1).toLong()) - map.put(handle, obj) - return handle - } - - fun get(handle: USize): T? { - return map.get(handle) - } - - fun remove(handle: USize): T? { - return map.remove(handle) - } -} - // FFI type for Rust future continuations internal interface UniFffiRustFutureContinuationCallbackType : com.sun.jna.Callback { - fun callback(continuationHandle: USize, pollResult: Short); + fun callback(continuationHandle: Long, pollResult: Short); } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt index b9ede7b453..48f7e9ec77 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt @@ -30,6 +30,16 @@ inline fun T.use(block: (T) -> R) = } } +// Wraps `UniffiHandle` to pass to object constructors. +// +// This class exists because `UniffiHandle` is a typealias to `Long`. If the object constructor +// inputs `UniffiHandle` directly and the user defines a primary constructor than inputs a single +// `Long` or `ULong` input, then we get JVM signature conflicts. To avoid this, we pass this type +// in instead. +// +// Let's try to remove this when we update the code based on ADR-0008. +data class UniffiHandleWrapper(val handle: UniffiHandle) + // The base class for all UniFFI Object types. // // This class provides core operations for working with the Rust handle to the live Rust struct on diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index ddb7b999aa..38b3d970c6 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -5,14 +5,12 @@ {% include "Interface.kt" %} -class {{ impl_class_name }}( - handle: UniffiHandle -) : FFIObject(handle), {{ interface_name }}{ - +class {{ impl_class_name }} internal constructor(handleWrapper: UniffiHandleWrapper) + : FFIObject(handleWrapper.handle), {{ interface_name }} { {%- match obj.primary_constructor() %} {%- when Some with (cons) %} constructor({% call kt::arg_list_decl(cons) -%}) : - this({% call kt::to_ffi_call(cons) %}) + this(UniffiHandleWrapper({% call kt::to_ffi_call(cons) %})) {%- when None %} {%- endmatch %} @@ -88,7 +86,7 @@ class {{ impl_class_name }}( companion object { {% for cons in obj.alternate_constructors() -%} fun {{ cons.name()|fn_name }}({% call kt::arg_list_decl(cons) %}): {{ impl_class_name }} = - {{ impl_class_name }}({% call kt::to_ffi_call(cons) %}) + {{ impl_class_name }}(UniffiHandleWrapper({% call kt::to_ffi_call(cons) %})) {% endfor %} } {% else %} @@ -105,7 +103,7 @@ class {{ impl_class_name }}( public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, UniffiHandle> { {%- if obj.is_trait_interface() %} - internal val handleMap = ConcurrentHandleMap<{{ interface_name }}>() + internal val handleMap = UniffiHandleMap<{{ type_name }}>() {%- endif %} override fun lower(value: {{ type_name }}): UniffiHandle { @@ -113,12 +111,12 @@ public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, Uniffi {%- when ObjectImpl::Struct %} return value.handle {%- when ObjectImpl::Trait %} - return UniffiHandle(handleMap.insert(value)) + return handleMap.newHandle(value) {%- endmatch %} } override fun lift(value: UniffiHandle): {{ type_name }} { - return {{ impl_class_name }}(value) + return {{ impl_class_name }}(UniffiHandleWrapper(value)) } override fun read(buf: ByteBuffer): {{ type_name }} { diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt index 9ee4229018..806777b139 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt @@ -29,12 +29,15 @@ import java.nio.ByteOrder import java.nio.CharBuffer import java.nio.charset.CodingErrorAction import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.withLock {%- for req in self.imports() %} {{ req.render() }} {%- endfor %} {% include "RustBufferTemplate.kt" %} +{% include "HandleMap.kt" %} {% include "FfiConverterTemplate.kt" %} {% include "Helpers.kt" %} diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index fd496daca6..5e385e457b 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -318,11 +318,8 @@ impl PythonCodeOracle { }, FfiType::ForeignBytes => "_UniffiForeignBytes".to_string(), FfiType::ForeignCallback => "_UNIFFI_FOREIGN_CALLBACK_T".to_string(), - // Pointer to an `asyncio.EventLoop` instance - FfiType::ForeignExecutorHandle => "ctypes.c_size_t".to_string(), FfiType::ForeignExecutorCallback => "_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T".to_string(), FfiType::RustFutureContinuationCallback => "_UNIFFI_FUTURE_CONTINUATION_T".to_string(), - FfiType::RustFutureContinuationData => "ctypes.c_size_t".to_string(), } } diff --git a/uniffi_bindgen/src/bindings/python/templates/Async.py b/uniffi_bindgen/src/bindings/python/templates/Async.py index 51bc31b595..f75bfcd821 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Async.py +++ b/uniffi_bindgen/src/bindings/python/templates/Async.py @@ -3,13 +3,13 @@ _UNIFFI_RUST_FUTURE_POLL_MAYBE_READY = 1 # Stores futures for _uniffi_continuation_callback -_UniffiContinuationPointerManager = _UniffiPointerManager() +_UNIFFI_CONTINUATION_HANDLE_MAP = UniffiHandleMap() # Continuation callback for async functions # lift the return value or error and resolve the future, causing the async function to resume. @_UNIFFI_FUTURE_CONTINUATION_T -def _uniffi_continuation_callback(future_ptr, poll_code): - (eventloop, future) = _UniffiContinuationPointerManager.release_pointer(future_ptr) +def _uniffi_continuation_callback(handle, poll_code): + (eventloop, future) = _UNIFFI_CONTINUATION_HANDLE_MAP.consume_handle(handle) eventloop.call_soon_threadsafe(_uniffi_set_future_result, future, poll_code) def _uniffi_set_future_result(future, poll_code): @@ -26,7 +26,7 @@ async def _uniffi_rust_call_async(rust_future, ffi_poll, ffi_complete, ffi_free, ffi_poll( rust_future, _uniffi_continuation_callback, - _UniffiContinuationPointerManager.new_pointer((eventloop, future)), + _UNIFFI_CONTINUATION_HANDLE_MAP.new_handle((eventloop, future)), ) poll_code = await future if poll_code == _UNIFFI_RUST_FUTURE_POLL_READY: diff --git a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py index f9a4768a5c..83f483dc2a 100644 --- a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py +++ b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceImpl.py @@ -51,7 +51,7 @@ def makeCallAndHandleReturn(): cb = {{ ffi_converter_name }}._handle_map.get(handle) if method == IDX_CALLBACK_FREE: - {{ ffi_converter_name }}._handle_map.remove(handle) + {{ ffi_converter_name }}._handle_map.consume_handle(handle) # Successfull return # See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` diff --git a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py index 1b7346ba4c..79d0d3b0ea 100644 --- a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py +++ b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py @@ -1,38 +1,3 @@ -import threading - -class ConcurrentHandleMap: - """ - A map where inserting, getting and removing data is synchronized with a lock. - """ - - def __init__(self): - # type Handle = int - self._left_map = {} # type: Dict[Handle, Any] - - self._lock = threading.Lock() - self._current_handle = 0 - self._stride = 1 - - def insert(self, obj): - with self._lock: - handle = self._current_handle - self._current_handle += self._stride - self._left_map[handle] = obj - return handle - - def get(self, handle): - with self._lock: - obj = self._left_map.get(handle) - if not obj: - raise InternalError("No callback in handlemap; this is a uniffi bug") - return obj - - def remove(self, handle): - with self._lock: - if handle in self._left_map: - obj = self._left_map.pop(handle) - return obj - # Magic number for the Rust proxy to call using the same mechanism as every other method, # to free the callback once it's dropped by Rust. IDX_CALLBACK_FREE = 0 @@ -42,7 +7,7 @@ def remove(self, handle): _UNIFFI_CALLBACK_UNEXPECTED_ERROR = 2 class UniffiCallbackInterfaceFfiConverter: - _handle_map = ConcurrentHandleMap() + _handle_map = UniffiHandleMap() @classmethod def lift(cls, handle): @@ -55,7 +20,7 @@ def read(cls, buf): @classmethod def lower(cls, cb): - handle = cls._handle_map.insert(cb) + handle = cls._handle_map.new_handle(cb) return handle @classmethod diff --git a/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py index 6a6932fed0..c4a28a03cf 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py @@ -9,33 +9,33 @@ _UNIFFI_FOREIGN_EXECUTOR_CALLBACK_ERROR = 2 class {{ ffi_converter_name }}: - _pointer_manager = _UniffiPointerManager() + _handle_map = UniffiHandleMap() @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) + return cls._handle_map.new_handle(eventloop) @classmethod def write(cls, eventloop, buf): - buf.write_c_size_t(cls.lower(eventloop)) + buf.write_u64(cls.lower(eventloop)) @classmethod def read(cls, buf): - return cls.lift(buf.read_c_size_t()) + return cls.lift(buf.read_u64()) @classmethod def lift(cls, value): - return cls._pointer_manager.lookup(value) + return cls._handle_map.get(value) @_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T -def _uniffi_executor_callback(eventloop_address, delay, task_ptr, task_data): +def _uniffi_executor_callback(handle, delay, task_ptr, task_data): if task_ptr is None: - {{ ffi_converter_name }}._pointer_manager.release_pointer(eventloop_address) + {{ ffi_converter_name }}._handle_map.consume_handle(handle) return _UNIFFI_FOREIGN_EXECUTOR_CALLBACK_SUCCESS else: - eventloop = {{ ffi_converter_name }}._pointer_manager.lookup(eventloop_address) + eventloop = {{ ffi_converter_name }}._handle_map.get(handle) if eventloop.is_closed(): return _UNIFFI_FOREIGN_EXECUTOR_CALLBACK_CANCELED diff --git a/uniffi_bindgen/src/bindings/python/templates/HandleMap.py b/uniffi_bindgen/src/bindings/python/templates/HandleMap.py new file mode 100644 index 0000000000..182e1d4ffc --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/templates/HandleMap.py @@ -0,0 +1,64 @@ +UniffiHandle = typing.NewType('UniffiHandle', int) + +# TODO: it would be nice to make this a generic class, however let's wait until python 3.11 is the +# minimum version, so we can do that without having to add a `TypeVar` to the top-level namespace. +class UniffiHandleMap: + """ + Manage handles for objects that are passed across the FFI + + See the `uniffi_core::HandleAlloc` trait for the semantics of each method + """ + + # Generates ids that are likely to be unique for each map + map_id_counter = itertools.count({{ ci.namespace_hash() }}) + + def __init__(self): + self._lock = threading.Lock() + self._map = {} + # Map ID, shifted into the top 16 bits + self._map_id = (next(UniffiHandleMap.map_id_counter) & 0xFFFF) << 48 + # Note: Foreign handles are always odd + self._key_counter = 1 + + def _next_key(self) -> int: + key = self._key_counter + self._key_counter = (self._key_counter + 2) & 0xFFFF_FFFF_FFFF + return key + + def _make_handle(self, key: int) -> int: + return key | self._map_id + + def _key(self, handle: int) -> int: + if (handle & 0xFFFF_0000_0000_0000) != self._map_id: + raise InternalError("Handle map ID mismatch") + return handle & 0xFFFF_FFFF_FFFF + + def new_handle(self, obj: object) -> int: + with self._lock: + key = self._next_key() + self._map[key] = obj + return self._make_handle(key) + + def clone_handle(self, handle: int) -> int: + try: + with self._lock: + obj = self._map[self._key(handle)] + key = self._next_key() + self._map[key] = obj + return self._make_handle(key) + except KeyError: + raise InternalError("handlemap key error: was the handle used after being freed?") + + def get(self, handle: int) -> object: + try: + with self._lock: + return self._map[self._key(handle)] + except KeyError: + raise InternalError("handlemap key error: was the handle used after being freed?") + + def consume_handle(self, handle: int) -> object: + try: + with self._lock: + return self._map.pop(self._key(handle)) + except KeyError: + raise InternalError("handlemap key error: was the handle used after being freed?") diff --git a/uniffi_bindgen/src/bindings/python/templates/Helpers.py b/uniffi_bindgen/src/bindings/python/templates/Helpers.py index dca962f176..0569528377 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Helpers.py +++ b/uniffi_bindgen/src/bindings/python/templates/Helpers.py @@ -71,5 +71,5 @@ def _uniffi_check_call_status(error_ffi_converter, call_status): _UNIFFI_FOREIGN_CALLBACK_T = ctypes.CFUNCTYPE(ctypes.c_int, ctypes.c_ulonglong, ctypes.c_ulong, ctypes.POINTER(ctypes.c_char), ctypes.c_int, ctypes.POINTER(_UniffiRustBuffer)) # UniFFI future continuation -_UNIFFI_FUTURE_CONTINUATION_T = ctypes.CFUNCTYPE(None, ctypes.c_size_t, ctypes.c_int8) +_UNIFFI_FUTURE_CONTINUATION_T = ctypes.CFUNCTYPE(None, ctypes.c_uint64, ctypes.c_int8) diff --git a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py index fac6cd5564..bfcc28be81 100644 --- a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py @@ -14,7 +14,7 @@ However, when task is NULL this indicates that Rust has dropped the ForeignExecutor and we should decrease the EventLoop refcount. """ -_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T = ctypes.CFUNCTYPE(ctypes.c_int8, ctypes.c_size_t, ctypes.c_uint32, ctypes.c_void_p, ctypes.c_void_p) +_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T = ctypes.CFUNCTYPE(ctypes.c_int8, ctypes.c_uint64, ctypes.c_uint32, ctypes.c_void_p, ctypes.c_void_p) """ Function pointer for a Rust task, which a callback function that takes a opaque pointer diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index 8cce1500db..98f1c8ac56 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -76,22 +76,22 @@ def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}: class {{ ffi_converter_name }}: {%- if obj.is_trait_interface() %} - _handle_map = ConcurrentHandleMap() + _handle_map = UniffiHandleMap() {%- endif %} @staticmethod - def lift(value: int): + def lift(value: UniffiHandle): return {{ impl_name }}._make_instance_(value) @staticmethod - def lower(value: {{ protocol_name }}): + def lower(value: {{ type_name }}): {%- match obj.imp() %} {%- when ObjectImpl::Struct %} if not isinstance(value, {{ impl_name }}): raise TypeError("Expected {{ impl_name }} instance, {} found".format(type(value).__name__)) return value._uniffi_handle {%- when ObjectImpl::Trait %} - return {{ ffi_converter_name }}._handle_map.insert(value) + return {{ ffi_converter_name }}._handle_map.new_handle(value) {%- endmatch %} @classmethod @@ -100,5 +100,5 @@ def read(cls, buf: _UniffiRustBuffer): return cls.lift(ptr) @classmethod - def write(cls, value: {{ protocol_name }}, buf: _UniffiRustBuffer): + def write(cls, value: {{ type_name }}, buf: _UniffiRustBuffer): buf.write_u64(cls.lower(value)) diff --git a/uniffi_bindgen/src/bindings/python/templates/PointerManager.py b/uniffi_bindgen/src/bindings/python/templates/PointerManager.py deleted file mode 100644 index 23aa28eab4..0000000000 --- a/uniffi_bindgen/src/bindings/python/templates/PointerManager.py +++ /dev/null @@ -1,68 +0,0 @@ -class _UniffiPointerManagerCPython: - """ - Manage giving out pointers to Python objects on CPython - - This class is used to generate opaque pointers that reference Python objects to pass to Rust. - It assumes a CPython platform. See _UniffiPointerManagerGeneral for the alternative. - """ - - def new_pointer(self, obj): - """ - Get a pointer for an object as a ctypes.c_size_t instance - - Each call to new_pointer() must be balanced with exactly one call to release_pointer() - - This returns a ctypes.c_size_t. This is always the same size as a pointer and can be - interchanged with pointers for FFI function arguments and return values. - """ - # IncRef the object since we're going to pass a pointer to Rust - ctypes.pythonapi.Py_IncRef(ctypes.py_object(obj)) - # id() is the object address on CPython - # (https://docs.python.org/3/library/functions.html#id) - return id(obj) - - def release_pointer(self, address): - py_obj = ctypes.cast(address, ctypes.py_object) - obj = py_obj.value - ctypes.pythonapi.Py_DecRef(py_obj) - return obj - - def lookup(self, address): - return ctypes.cast(address, ctypes.py_object).value - -class _UniffiPointerManagerGeneral: - """ - Manage giving out pointers to Python objects on non-CPython platforms - - This has the same API as _UniffiPointerManagerCPython, but doesn't assume we're running on - CPython and is slightly slower. - - Instead of using real pointers, it maps integer values to objects and returns the keys as - c_size_t values. - """ - - def __init__(self): - self._map = {} - self._lock = threading.Lock() - self._current_handle = 0 - - def new_pointer(self, obj): - with self._lock: - handle = self._current_handle - self._current_handle += 1 - self._map[handle] = obj - return handle - - def release_pointer(self, handle): - with self._lock: - return self._map.pop(handle) - - def lookup(self, handle): - with self._lock: - return self._map[handle] - -# Pick an pointer manager implementation based on the platform -if platform.python_implementation() == 'CPython': - _UniffiPointerManager = _UniffiPointerManagerCPython # type: ignore -else: - _UniffiPointerManager = _UniffiPointerManagerGeneral # type: ignore diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py index c317a632fc..35efeb2ab9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py @@ -137,9 +137,6 @@ def read_float(self): def read_double(self): return self._unpack_from(8, ">d") - def read_c_size_t(self): - return self._unpack_from(ctypes.sizeof(ctypes.c_size_t) , "@N") - class _UniffiRustBufferBuilder: """ Helper for structured writing of bytes into a _UniffiRustBuffer. @@ -206,6 +203,3 @@ def write_float(self, v): def write_double(self, v): self._pack_into(8, ">d", v) - - def write_c_size_t(self, v): - self._pack_into(ctypes.sizeof(ctypes.c_size_t) , "@N", v) diff --git a/uniffi_bindgen/src/bindings/python/templates/wrapper.py b/uniffi_bindgen/src/bindings/python/templates/wrapper.py index 24c3290ff7..024663ae5b 100644 --- a/uniffi_bindgen/src/bindings/python/templates/wrapper.py +++ b/uniffi_bindgen/src/bindings/python/templates/wrapper.py @@ -17,9 +17,11 @@ import sys import ctypes import enum +import itertools import struct import contextlib import datetime +import threading import typing {%- if ci.has_async_fns() %} import asyncio @@ -33,8 +35,8 @@ _DEFAULT = object() {% include "RustBufferTemplate.py" %} +{% include "HandleMap.py" %} {% include "Helpers.py" %} -{% include "PointerManager.py" %} {% include "RustBufferHelper.py" %} # Contains loading, initialization code, and the FFI Function declarations. diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs index f9807618f9..41f8a7506e 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -159,10 +159,7 @@ mod filters { FfiType::ForeignExecutorCallback => { unimplemented!("Foreign executors are not implemented") } - FfiType::ForeignExecutorHandle => { - unimplemented!("Foreign executors are not implemented") - } - FfiType::RustFutureContinuationCallback | FfiType::RustFutureContinuationData => { + FfiType::RustFutureContinuationCallback => { 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 fde9022cdf..87b3651728 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -464,10 +464,8 @@ impl SwiftCodeOracle { FfiType::RustBuffer(_) => "RustBuffer".into(), FfiType::ForeignBytes => "ForeignBytes".into(), FfiType::ForeignCallback => "ForeignCallback".into(), - FfiType::ForeignExecutorHandle => "Int".into(), FfiType::ForeignExecutorCallback => "ForeignExecutorCallback".into(), FfiType::RustFutureContinuationCallback => "UniFfiRustFutureContinuation".into(), - FfiType::RustFutureContinuationData => "UnsafeMutableRawPointer".into(), } } @@ -475,8 +473,7 @@ impl SwiftCodeOracle { match ffi_type { FfiType::ForeignCallback | FfiType::ForeignExecutorCallback - | FfiType::RustFutureContinuationCallback - | FfiType::RustFutureContinuationData => { + | FfiType::RustFutureContinuationCallback => { format!("{} _Nonnull", self.ffi_type_label_raw(ffi_type)) } _ => self.ffi_type_label_raw(ffi_type), @@ -578,11 +575,9 @@ pub mod filters { FfiType::ForeignBytes => "ForeignBytes".into(), FfiType::ForeignCallback => "ForeignCallback _Nonnull".into(), FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback _Nonnull".into(), - FfiType::ForeignExecutorHandle => "size_t".into(), FfiType::RustFutureContinuationCallback => { "UniFfiRustFutureContinuation _Nonnull".into() } - FfiType::RustFutureContinuationData => "void* _Nonnull".into(), }) } diff --git a/uniffi_bindgen/src/bindings/swift/templates/Async.swift b/uniffi_bindgen/src/bindings/swift/templates/Async.swift index 668b38e79b..f7446ca5e0 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Async.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Async.swift @@ -1,9 +1,11 @@ private let UNIFFI_RUST_FUTURE_POLL_READY: Int8 = 0 private let UNIFFI_RUST_FUTURE_POLL_MAYBE_READY: Int8 = 1 +fileprivate var UNIFFI_CONTINUATION_HANDLE_MAP = UniffiHandleMap>() + fileprivate func uniffiRustCallAsync( rustFutureFunc: () -> UInt64, - pollFunc: (UInt64, @escaping UniFfiRustFutureContinuation, UnsafeMutableRawPointer) -> (), + pollFunc: (UInt64, @escaping UniFfiRustFutureContinuation, UInt64) -> (), completeFunc: (UInt64, UnsafeMutablePointer) -> F, freeFunc: (UInt64) -> (), liftFunc: (F) throws -> T, @@ -19,7 +21,7 @@ fileprivate func uniffiRustCallAsync( var pollResult: Int8; repeat { pollResult = await withUnsafeContinuation { - pollFunc(rustFuture, uniffiFutureContinuationCallback, ContinuationHolder($0).toOpaque()) + pollFunc(rustFuture, uniffiFutureContinuationCallback, UNIFFI_CONTINUATION_HANDLE_MAP.newHandle(obj: $0)) } } while pollResult != UNIFFI_RUST_FUTURE_POLL_READY @@ -31,28 +33,6 @@ fileprivate func uniffiRustCallAsync( // Callback handlers for an async calls. These are invoked by Rust when the future is ready. They // lift the return value or error and resume the suspended function. -fileprivate func uniffiFutureContinuationCallback(ptr: UnsafeMutableRawPointer, pollResult: Int8) { - ContinuationHolder.fromOpaque(ptr).resume(pollResult) -} - -// Wraps UnsafeContinuation in a class so that we can use reference counting when passing it across -// the FFI -fileprivate class ContinuationHolder { - let continuation: UnsafeContinuation - - init(_ continuation: UnsafeContinuation) { - self.continuation = continuation - } - - func resume(_ pollResult: Int8) { - self.continuation.resume(returning: pollResult) - } - - func toOpaque() -> UnsafeMutableRawPointer { - return Unmanaged.passRetained(self).toOpaque() - } - - static func fromOpaque(_ ptr: UnsafeRawPointer) -> ContinuationHolder { - return Unmanaged.fromOpaque(ptr).takeRetainedValue() - } +fileprivate func uniffiFutureContinuationCallback(handle: UInt64, pollResult: Int8) { + UNIFFI_CONTINUATION_HANDLE_MAP.consumeHandle(handle: handle).resume(returning: pollResult) } diff --git a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h index 87698e359f..587eab591b 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h +++ b/uniffi_bindgen/src/bindings/swift/templates/BridgingHeaderTemplate.h @@ -60,7 +60,7 @@ typedef struct RustCallStatus { #endif // def UNIFFI_SHARED_H // Continuation callback for UniFFI Futures -typedef void (*UniFfiRustFutureContinuation)(void * _Nonnull, int8_t); +typedef void (*UniFfiRustFutureContinuation)(uint64_t, int8_t); // Scaffolding functions {%- for func in ci.iter_ffi_function_definitions() %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift index 157da46128..3ec718af73 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift @@ -3,7 +3,7 @@ // Declaration and FfiConverters for {{ type_name }} Callback Interface fileprivate let {{ callback_handler }} : ForeignCallback = - { (handle: UniFFICallbackHandle, method: Int32, argsData: UnsafePointer, argsLen: Int32, out_buf: UnsafeMutablePointer) -> Int32 in + { (handle: UInt64, method: Int32, argsData: UnsafePointer, argsLen: Int32, out_buf: UnsafeMutablePointer) -> Int32 in {% for meth in methods.iter() -%} {%- let method_name = format!("invoke_{}", meth.name())|fn_name %} @@ -55,18 +55,15 @@ fileprivate let {{ callback_handler }} : ForeignCallback = switch method { case IDX_CALLBACK_FREE: - {{ ffi_converter_name }}.handleMap.remove(handle: handle) + let _ = {{ ffi_converter_name }}.handleMap.consumeHandle(handle: handle) // Sucessful return // See docs of ForeignCallback in `uniffi_core/src/ffi/foreigncallbacks.rs` return UNIFFI_CALLBACK_SUCCESS {% for meth in methods.iter() -%} {% let method_name = format!("invoke_{}", meth.name())|fn_name -%} case {{ loop.index }}: - guard let cb = {{ ffi_converter_name }}.handleMap.get(handle: handle) else { - out_buf.pointee = {{ Type::String.borrow()|lower_fn }}("No callback in handlemap; this is a Uniffi bug") - return UNIFFI_CALLBACK_UNEXPECTED_ERROR - } do { + let cb = {{ ffi_converter_name }}.handleMap.get(handle: handle) return try {{ method_name }}(cb, argsData, argsLen, out_buf) } catch let error { out_buf.pointee = {{ Type::String.borrow()|lower_fn }}(String(describing: error)) diff --git a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift index d03b7ccb3f..5863c2ad41 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceRuntime.swift @@ -1,60 +1,3 @@ -fileprivate extension NSLock { - func withLock(f: () throws -> T) rethrows -> T { - self.lock() - defer { self.unlock() } - return try f() - } -} - -fileprivate typealias UniFFICallbackHandle = UInt64 -fileprivate class UniFFICallbackHandleMap { - private var leftMap: [UniFFICallbackHandle: T] = [:] - private var counter: [UniFFICallbackHandle: UInt64] = [:] - private var rightMap: [ObjectIdentifier: UniFFICallbackHandle] = [:] - - private let lock = NSLock() - private var currentHandle: UniFFICallbackHandle = 1 - private let stride: UniFFICallbackHandle = 1 - - func insert(obj: T) -> UniFFICallbackHandle { - lock.withLock { - let id = ObjectIdentifier(obj as AnyObject) - let handle = rightMap[id] ?? { - currentHandle += stride - let handle = currentHandle - leftMap[handle] = obj - rightMap[id] = handle - return handle - }() - counter[handle] = (counter[handle] ?? 0) + 1 - return handle - } - } - - func get(handle: UniFFICallbackHandle) -> T? { - lock.withLock { - leftMap[handle] - } - } - - func delete(handle: UniFFICallbackHandle) { - remove(handle: handle) - } - - @discardableResult - func remove(handle: UniFFICallbackHandle) -> T? { - lock.withLock { - defer { counter[handle] = (counter[handle] ?? 1) - 1 } - guard counter[handle] == 1 else { return leftMap[handle] } - let obj = leftMap.removeValue(forKey: handle) - if let obj = obj { - rightMap.removeValue(forKey: ObjectIdentifier(obj as AnyObject)) - } - return obj - } - } -} - // Magic number for the Rust proxy to call using the same mechanism as every other method, // to free the callback once it's dropped by Rust. private let IDX_CALLBACK_FREE: Int32 = 0 diff --git a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift index f9e75b2a3f..6a0da0369f 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift @@ -10,28 +10,24 @@ // FfiConverter protocol for callback interfaces fileprivate struct {{ ffi_converter_name }} { - fileprivate static var handleMap = UniFFICallbackHandleMap<{{ type_name }}>() + fileprivate static var handleMap = UniffiHandleMap<{{ type_name }}>() } extension {{ ffi_converter_name }} : FfiConverter { typealias SwiftType = {{ type_name }} - // We can use Handle as the FfiType because it's a typealias to UInt64 - typealias FfiType = UniFFICallbackHandle + typealias FfiType = UInt64 - public static func lift(_ handle: UniFFICallbackHandle) throws -> SwiftType { - guard let callback = handleMap.get(handle: handle) else { - throw UniffiInternalError.unexpectedStaleHandle - } - return callback + public static func lift(_ handle: UInt64) throws -> SwiftType { + return handleMap.get(handle: handle) } public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> SwiftType { - let handle: UniFFICallbackHandle = try readInt(&buf) + let handle: UInt64 = try readInt(&buf) return try lift(handle) } - public static func lower(_ v: SwiftType) -> UniFFICallbackHandle { - return handleMap.insert(obj: v) + public static func lower(_ v: SwiftType) -> UInt64 { + return handleMap.newHandle(obj: v) } public static func write(_ v: SwiftType, into buf: inout [UInt8]) { diff --git a/uniffi_bindgen/src/bindings/swift/templates/HandleMap.swift b/uniffi_bindgen/src/bindings/swift/templates/HandleMap.swift new file mode 100644 index 0000000000..b294eddd7f --- /dev/null +++ b/uniffi_bindgen/src/bindings/swift/templates/HandleMap.swift @@ -0,0 +1,75 @@ +// Generate map IDs that are likely to be unique +fileprivate var uniffiMapIdCounter: UInt64 = {{ ci.namespace_hash() }} & 0xFFFF +fileprivate func uniffiNextMapId() -> UInt64 { + let mapId = uniffiMapIdCounter + uniffiMapIdCounter = (uniffiMapIdCounter + 1) & 0xFFFF + return mapId +} + +// Manage handles for objects that are passed across the FFI +// +// See the `uniffi_core::HandleAlloc` trait for the semantics of each method +fileprivate class UniffiHandleMap { + + // Map ID, shifted into the top 16 bits + private let mapId: UInt64 = uniffiNextMapId() << 48 + private let lock: NSLock = NSLock() + private var map: [UInt64: T] = [:] + // Note: foreign handles are always odd + private var keyCounter: UInt64 = 1 + + private func nextKey() -> UInt64 { + let key = keyCounter + keyCounter = (keyCounter + 2) & 0xFFFF_FFFF_FFFF + return key + } + + private func makeHandle(_ key: UInt64) -> UInt64 { + return key | mapId + } + + private func key(_ handle: UInt64) -> UInt64 { + if (handle & 0xFFFF_0000_0000_0000 != mapId) { + fatalError("Handle map ID mismatch") + } + return handle & 0xFFFF_FFFF_FFFF + } + + func newHandle(obj: T) -> UInt64 { + lock.withLock { + let key = nextKey() + map[key] = obj + return makeHandle(key) + } + } + + func get(handle: UInt64) -> T { + lock.withLock { + guard let obj = map[key(handle)] else { + fatalError("handlemap key error: was the handle used after being freed?") + } + return obj + } + } + + func cloneHandle(handle: UInt64) -> UInt64 { + lock.withLock { + guard let obj = map[key(handle)] else { + fatalError("handlemap key error: was the handle used after being freed?") + } + let key = nextKey() + map[key] = obj + return makeHandle(key) + } + } + + @discardableResult + func consumeHandle(handle: UInt64) -> T { + lock.withLock { + guard let obj = map.removeValue(forKey: key(handle)) else { + fatalError("handlemap key error: was the handle used after being freed?") + } + return obj + } + } +} diff --git a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift index acc37cdca3..bbe31b9424 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift @@ -99,3 +99,11 @@ private func uniffiCheckCallStatus( throw UniffiInternalError.unexpectedRustCallStatusCode } } + +fileprivate extension NSLock { + func withLock(f: () throws -> T) rethrows -> T { + self.lock() + defer { self.unlock() } + return try f() + } +} diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index 5f566bdcab..7be8e34d7f 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -99,7 +99,7 @@ public class {{ impl_class_name }}: {{ protocol_name }} { public struct {{ ffi_converter_name }}: FfiConverter { {%- if obj.is_trait_interface() %} - fileprivate static var handleMap = UniFFICallbackHandleMap<{{ type_name }}>() + fileprivate static var handleMap = UniffiHandleMap<{{ type_name }}>() {%- endif %} typealias FfiType = UInt64 @@ -114,10 +114,7 @@ public struct {{ ffi_converter_name }}: FfiConverter { {%- when ObjectImpl::Struct %} return value.handle {%- when ObjectImpl::Trait %} - guard let ptr = UnsafeMutableRawPointer(bitPattern: UInt(truncatingIfNeeded: handleMap.insert(obj: value))) else { - fatalError("Cast to UnsafeMutableRawPointer failed") - } - return ptr + return handleMap.newHandle(obj: value) {%- endmatch %} } diff --git a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift index c34d348efb..70c0990088 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift @@ -14,6 +14,7 @@ import {{ config.ffi_module_name() }} #endif {% include "RustBufferTemplate.swift" %} +{% include "HandleMap.swift" %} {% include "Helpers.swift" %} // Public interface members begin here. diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index bd0fafae80..de8db86334 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -46,14 +46,10 @@ pub enum FfiType { ForeignBytes, /// Pointer to a callback function that handles all callbacks on the foreign language side. ForeignCallback, - /// Pointer-sized opaque handle that represents a foreign executor. Foreign bindings can - /// either use an actual pointer or a usized integer. - ForeignExecutorHandle, /// Pointer to the callback function that's invoked to schedule calls with a ForeignExecutor ForeignExecutorCallback, /// Continuation function for a Rust future RustFutureContinuationCallback, - RustFutureContinuationData, // TODO: you can imagine a richer structural typesystem here, e.g. `Ref` or something. // We don't need that yet and it's possible we never will, so it isn't here for now. } @@ -85,10 +81,14 @@ impl From<&Type> for FfiType { // Byte strings are also always owned rust values. // We might add a separate type for borrowed byte strings in future as well. Type::Bytes => FfiType::RustBuffer(None), - Type::Object { .. } => FfiType::Handle, - // Callback interfaces are passed as opaque integer handles. - Type::CallbackInterface { .. } => FfiType::UInt64, - Type::ForeignExecutor => FfiType::ForeignExecutorHandle, + // Object types interfaces are passed as opaque handles. + Type::Object { .. } + | Type::CallbackInterface { .. } + | Type::ForeignExecutor + | Type::External { + kind: ExternalKind::Interface, + .. + } => FfiType::Handle, // Other types are serialized into a bytebuffer and deserialized on the other side. Type::Enum { .. } | Type::Record { .. } @@ -97,10 +97,8 @@ impl From<&Type> for FfiType { | Type::Map { .. } | Type::Timestamp | Type::Duration => FfiType::RustBuffer(None), - Type::External { - kind: ExternalKind::Interface, - .. - } => FfiType::Handle, + // External data classes are also serialized as a buffer, but need a module name to + // make imports work. Type::External { name, kind: ExternalKind::DataClass, diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 472dc68790..99420a1504 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -45,7 +45,8 @@ //! * Error messages and general developer experience leave a lot to be desired. use std::{ - collections::{btree_map::Entry, BTreeMap, BTreeSet, HashSet}, + collections::{btree_map::Entry, hash_map::DefaultHasher, BTreeMap, BTreeSet, HashSet}, + hash::{Hash, Hasher}, iter, }; @@ -153,6 +154,15 @@ impl ComponentInterface { &self.types.namespace.name } + /// Get the checksum value for our namespace. + /// + /// This is highly likely to be unique for each `ComponentInterface` + pub fn namespace_hash(&self) -> u32 { + let mut hasher = DefaultHasher::new(); + self.types.namespace.name.hash(&mut hasher); + hasher.finish() as u32 + } + pub fn uniffi_contract_version(&self) -> u32 { // This is set by the scripts in the version-mismatch fixture let force_version = std::env::var("UNIFFI_FORCE_CONTRACT_VERSION"); @@ -450,8 +460,8 @@ impl ComponentInterface { type_: FfiType::RustFutureContinuationCallback, }, FfiArgument { - name: "callback_data".to_owned(), - type_: FfiType::RustFutureContinuationData, + name: "continuation_data".to_owned(), + type_: FfiType::Handle, }, ], return_type: None, diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index f38ea8cfbc..290e412598 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -189,7 +189,7 @@ impl Object { pub fn derive_ffi_funcs(&mut self) -> Result<()> { assert!(!self.ffi_func_free.name().is_empty()); self.ffi_func_free.arguments = vec![FfiArgument { - name: "ptr".to_string(), + name: "handle".to_string(), type_: FfiType::Handle, }]; self.ffi_func_free.return_type = None; @@ -419,7 +419,7 @@ impl Method { // hence `arguments` and `full_arguments` are different. pub fn full_arguments(&self) -> Vec { vec![Argument { - name: "ptr".to_string(), + name: "handle".to_string(), // TODO: ideally we'd get this via `ci.resolve_type_expression` so that it // is contained in the proper `TypeUniverse`, but this works for now. type_: Type::Object { diff --git a/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs b/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs index 7be11554a4..1f4e03af47 100644 --- a/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs +++ b/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs @@ -29,11 +29,11 @@ pub extern "C" fn {{ cbi.ffi_init_callback().name() }}(callback: uniffi::Foreign #[doc(hidden)] #[derive(Debug)] struct {{ trait_impl }} { - handle: u64 + handle: ::uniffi::Handle } impl {{ trait_impl }} { - fn new(handle: u64) -> Self { + fn new(handle: ::uniffi::Handle) -> Self { Self { handle } } } diff --git a/uniffi_core/src/ffi/callbackinterface.rs b/uniffi_core/src/ffi/callbackinterface.rs index 7be66880bb..55bb4beb28 100644 --- a/uniffi_core/src/ffi/callbackinterface.rs +++ b/uniffi_core/src/ffi/callbackinterface.rs @@ -113,7 +113,7 @@ //! type and then returns to client code. //! -use crate::{ForeignCallback, ForeignCallbackCell, Lift, LiftReturn, RustBuffer}; +use crate::{ForeignCallback, ForeignCallbackCell, Handle, Lift, LiftReturn, RustBuffer}; use std::fmt; /// The method index used by the Drop trait to communicate to the foreign language side that Rust has finished with it, @@ -167,7 +167,7 @@ impl ForeignCallbackInternals { } /// Invoke a callback interface method on the foreign side and return the result - pub fn invoke_callback(&self, handle: u64, method: u32, args: RustBuffer) -> R + pub fn invoke_callback(&self, handle: Handle, method: u32, args: RustBuffer) -> R where R: LiftReturn, { diff --git a/uniffi_core/src/ffi/ffidefault.rs b/uniffi_core/src/ffi/ffidefault.rs index c5cd42efd5..be5f0dd796 100644 --- a/uniffi_core/src/ffi/ffidefault.rs +++ b/uniffi_core/src/ffi/ffidefault.rs @@ -45,24 +45,12 @@ impl FfiDefault for crate::Handle { } } -impl FfiDefault for *const std::ffi::c_void { - fn ffi_default() -> Self { - std::ptr::null() - } -} - impl FfiDefault for crate::RustBuffer { fn ffi_default() -> Self { unsafe { Self::from_raw_parts(std::ptr::null_mut(), 0, 0) } } } -impl FfiDefault for crate::ForeignExecutorHandle { - fn ffi_default() -> Self { - Self(std::ptr::null()) - } -} - impl FfiDefault for Option { fn ffi_default() -> Self { None diff --git a/uniffi_core/src/ffi/foreigncallbacks.rs b/uniffi_core/src/ffi/foreigncallbacks.rs index ac2463cd8e..1746489e30 100644 --- a/uniffi_core/src/ffi/foreigncallbacks.rs +++ b/uniffi_core/src/ffi/foreigncallbacks.rs @@ -10,7 +10,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; -use crate::{ForeignExecutorHandle, RustBuffer, RustTaskCallback}; +use crate::{Handle, RustBuffer, RustTaskCallback}; /// ForeignCallback is the Rust representation of a foreign language function. /// It is the basis for all callbacks interfaces. It is registered exactly once per callback interface, @@ -30,7 +30,7 @@ use crate::{ForeignExecutorHandle, RustBuffer, RustTaskCallback}; /// * Callbacks return one of the `CallbackResult` values /// Note: The output buffer might still contain 0 bytes of data. pub type ForeignCallback = unsafe extern "C" fn( - handle: u64, + handle: Handle, method: u32, args_data: *const u8, args_len: i32, @@ -50,7 +50,7 @@ pub type ForeignCallback = unsafe extern "C" fn( /// /// The callback should return one of the `ForeignExecutorCallbackResult` values. pub type ForeignExecutorCallback = extern "C" fn( - executor: ForeignExecutorHandle, + executor: Handle, delay: u32, task: Option, task_data: *const (), diff --git a/uniffi_core/src/ffi/foreignexecutor.rs b/uniffi_core/src/ffi/foreignexecutor.rs index 7b1cb9bd80..3f0f826644 100644 --- a/uniffi_core/src/ffi/foreignexecutor.rs +++ b/uniffi_core/src/ffi/foreignexecutor.rs @@ -6,20 +6,7 @@ use std::panic; -use crate::{ForeignExecutorCallback, ForeignExecutorCallbackCell}; - -/// Opaque handle for a foreign task executor. -/// -/// Foreign code can either use an actual pointer, or use an integer value casted to it. -#[repr(transparent)] -#[derive(Clone, Copy, Debug)] -pub struct ForeignExecutorHandle(pub(crate) *const ()); - -// Implement Send + Sync for `ForeignExecutor`. The foreign bindings code is responsible for -// making the `ForeignExecutorCallback` thread-safe. -unsafe impl Send for ForeignExecutorHandle {} - -unsafe impl Sync for ForeignExecutorHandle {} +use crate::{ForeignExecutorCallback, ForeignExecutorCallbackCell, Handle}; /// Result code returned by `ForeignExecutorCallback` #[repr(i8)] @@ -95,11 +82,11 @@ pub fn foreign_executor_callback_set(callback: ForeignExecutorCallback) { /// Schedule Rust calls using a foreign executor #[derive(Debug)] pub struct ForeignExecutor { - pub(crate) handle: ForeignExecutorHandle, + pub(crate) handle: Handle, } impl ForeignExecutor { - pub fn new(executor: ForeignExecutorHandle) -> Self { + pub fn new(executor: Handle) -> Self { Self { handle: executor } } @@ -162,11 +149,11 @@ impl ForeignExecutor { /// Low-level schedule interface /// /// When using this function, take care to ensure that the `ForeignExecutor` that holds the -/// `ForeignExecutorHandle` has not been dropped. +/// `Handle` has not been dropped. /// /// Returns true if the callback was successfully scheduled pub(crate) fn schedule_raw( - handle: ForeignExecutorHandle, + handle: Handle, delay: u32, callback: RustTaskCallback, data: *const (), @@ -219,6 +206,7 @@ pub use test::MockEventLoop; #[cfg(test)] mod test { use super::*; + use crate::HandleAlloc; use std::{ future::Future, pin::Pin, @@ -264,11 +252,9 @@ mod test { }) } - /// Create a new ForeignExecutorHandle - pub fn new_handle(self: &Arc) -> ForeignExecutorHandle { - // To keep the memory management simple, we simply leak an arc reference for this. We - // only create a handful of these in the tests so there's no need for proper cleanup. - ForeignExecutorHandle(Arc::into_raw(Arc::clone(self)) as *const ()) + /// Create a new Handle + pub fn new_handle(self: &Arc) -> Handle { + >::new_handle(Arc::clone(self)) } pub fn new_executor(self: &Arc) -> ForeignExecutor { @@ -314,13 +300,13 @@ mod test { // `ForeignExecutorCallback` that we install for testing extern "C" fn mock_executor_callback( - handle: ForeignExecutorHandle, + handle: Handle, delay: u32, task: Option, task_data: *const (), ) -> i8 { - let eventloop = handle.0 as *const MockEventLoop; - let mut inner = unsafe { (*eventloop).inner.lock().unwrap() }; + let eventloop = >::get_arc(handle); + let mut inner = eventloop.inner.lock().unwrap(); if inner.is_shutdown { ForeignExecutorCallbackResult::Cancelled as i8 } else { diff --git a/uniffi_core/src/ffi/handle.rs b/uniffi_core/src/ffi/handle.rs index c7fa6b93e0..5b5b4b63f4 100644 --- a/uniffi_core/src/ffi/handle.rs +++ b/uniffi_core/src/ffi/handle.rs @@ -14,6 +14,13 @@ /// * The lowest bit will always be set for foreign handles and never set for Rust ones (since the /// leaked pointer will be aligned). /// +/// Foreign handles for internal bindings languages look like this: +/// * Bits 0-48 are the map key. Keys are always odd, so the lowest bit is always set. +/// * Bits 48-64 are the map identifier, used to detect using a handle with the wrong map +/// +/// External bindings authors are free to use any form they want for their handles, as long the lowest bit is set. +/// In particular, they don't need to implement the map identifier. +/// /// Rust handles are mainly managed is through the [crate::HandleAlloc] trait. #[derive(Copy, Clone, Default, Debug, PartialEq, Eq)] #[repr(transparent)] diff --git a/uniffi_core/src/ffi/rustfuture.rs b/uniffi_core/src/ffi/rustfuture.rs index 070b23847c..3dd87d5e88 100644 --- a/uniffi_core/src/ffi/rustfuture.rs +++ b/uniffi_core/src/ffi/rustfuture.rs @@ -105,7 +105,7 @@ pub enum RustFuturePoll { /// /// The Rust side of things calls this when the foreign side should call [rust_future_poll] again /// to continue progress on the future. -pub type RustFutureContinuationCallback = extern "C" fn(callback_data: *const (), RustFuturePoll); +pub type RustFutureContinuationCallback = extern "C" fn(callback_data: Handle, RustFuturePoll); // === Public FFI API === @@ -141,7 +141,7 @@ where pub unsafe fn rust_future_poll( handle: Handle, callback: RustFutureContinuationCallback, - data: *const (), + data: Handle, ) where dyn RustFutureFfi: HandleAlloc, { @@ -212,7 +212,6 @@ derive_ffi_traits!(impl HandleAlloc for dyn RustFutureFfi); derive_ffi_traits!(impl HandleAlloc for dyn RustFutureFfi); derive_ffi_traits!(impl HandleAlloc for dyn RustFutureFfi); derive_ffi_traits!(impl HandleAlloc for dyn RustFutureFfi<()>); -derive_ffi_traits!(impl HandleAlloc for dyn RustFutureFfi<*const ::std::ffi::c_void>); /// Thread-safe storage for [RustFutureContinuationCallback] data /// @@ -230,8 +229,8 @@ enum ContinuationDataCell { /// The future has been cancelled, any future `store` calls should immediately result in the /// continuation being called with `RustFuturePoll::Ready`. Cancelled, - /// Continuation set, the next time `wake()` is called is called, we should invoke it. - Set(RustFutureContinuationCallback, *const ()), + /// Continuation set, the next time `wake()` is called, we should invoke it. + Set(RustFutureContinuationCallback, Handle), } impl ContinuationDataCell { @@ -241,7 +240,7 @@ impl ContinuationDataCell { /// Store new continuation data if we are in the `Empty` state. If we are in the `Waked` or /// `Cancelled` state, call the continuation immediately with the data. - fn store(&mut self, callback: RustFutureContinuationCallback, data: *const ()) { + fn store(&mut self, callback: RustFutureContinuationCallback, data: Handle) { match self { Self::Empty => *self = Self::Set(callback, data), Self::Set(old_callback, old_data) => { @@ -289,11 +288,6 @@ impl ContinuationDataCell { } } -// ContinuationDataCell is Send + Sync as long we handle the *const () pointer correctly - -unsafe impl Send for ContinuationDataCell {} -unsafe impl Sync for ContinuationDataCell {} - /// Wraps the actual future we're polling struct WrappedFuture where @@ -438,7 +432,7 @@ where }) } - fn poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()) { + fn poll(self: Arc, callback: RustFutureContinuationCallback, data: Handle) { let ready = self.is_cancelled() || { let mut locked = self.future.lock().unwrap(); let waker: std::task::Waker = Arc::clone(&self).into(); @@ -503,8 +497,8 @@ where /// x86-64 machine . By parametrizing on `T::ReturnType` we can instead monomorphize by hand and /// only create those functions for each of the 13 possible FFI return types. #[doc(hidden)] -pub trait RustFutureFfi { - fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()); +pub trait RustFutureFfi: Send + Sync { + fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: Handle); fn ffi_cancel(&self); fn ffi_complete(&self, call_status: &mut RustCallStatus) -> ReturnType; fn ffi_free(self: Arc); @@ -517,7 +511,7 @@ where T: LowerReturn + Send + 'static, UT: Send + 'static, { - fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()) { + fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: Handle) { self.poll(callback, data) } @@ -601,13 +595,15 @@ mod tests { /// Poll a Rust future and get an OnceCell that's set when the continuation is called fn poll(rust_future: &Arc>) -> Arc> { let cell = Arc::new(OnceCell::new()); - let cell_ptr = Arc::into_raw(cell.clone()) as *const (); - rust_future.clone().ffi_poll(poll_continuation, cell_ptr); + let handle = as HandleAlloc>::new_handle( + Arc::clone(&cell), + ); + rust_future.clone().ffi_poll(poll_continuation, handle); cell } - extern "C" fn poll_continuation(data: *const (), code: RustFuturePoll) { - let cell = unsafe { Arc::from_raw(data as *const OnceCell) }; + extern "C" fn poll_continuation(data: Handle, code: RustFuturePoll) { + let cell = as HandleAlloc>::get_arc(data); cell.set(code).expect("Error setting OnceCell"); } diff --git a/uniffi_core/src/ffi_converter_impls.rs b/uniffi_core/src/ffi_converter_impls.rs index af18f3873b..2460f87cb0 100644 --- a/uniffi_core/src/ffi_converter_impls.rs +++ b/uniffi_core/src/ffi_converter_impls.rs @@ -23,7 +23,7 @@ /// "UT" means an abitrary `UniFfiTag` type. use crate::{ check_remaining, derive_ffi_traits, ffi_converter_rust_buffer_lift_and_lower, metadata, - ConvertError, FfiConverter, ForeignExecutor, Lift, LiftReturn, Lower, LowerReturn, + ConvertError, FfiConverter, ForeignExecutor, Handle, Lift, LiftReturn, Lower, LowerReturn, MetadataBuffer, Result, RustBuffer, UnexpectedUniFFICallbackError, }; use anyhow::bail; @@ -411,7 +411,7 @@ where /// The foreign bindings may use an actual pointer to the executor object, or a usized integer /// handle. unsafe impl FfiConverter for ForeignExecutor { - type FfiType = crate::ForeignExecutorHandle; + type FfiType = Handle; // Passing these back to the foreign bindings is currently not supported fn lower(executor: Self) -> Self::FfiType { @@ -419,13 +419,7 @@ unsafe impl FfiConverter for ForeignExecutor { } fn write(executor: Self, buf: &mut Vec) { - // Use native endian when writing these values, so they can be casted to pointer values - match std::mem::size_of::() { - // Use native endian when reading these values, so they can be casted to pointer values - 4 => buf.put_u32_ne(executor.handle.0 as u32), - 8 => buf.put_u64_ne(executor.handle.0 as u64), - n => panic!("Invalid usize width: {n}"), - }; + buf.put_u64(executor.handle.as_raw()) } fn try_lift(executor: Self::FfiType) -> Result { @@ -433,13 +427,7 @@ unsafe impl FfiConverter for ForeignExecutor { } fn try_read(buf: &mut &[u8]) -> Result { - let usize_val = match std::mem::size_of::() { - // Use native endian when reading these values, so they can be casted to pointer values - 4 => buf.get_u32_ne() as usize, - 8 => buf.get_u64_ne() as usize, - n => panic!("Invalid usize width: {n}"), - }; - >::try_lift(crate::ForeignExecutorHandle(usize_val as *const ())) + >::try_lift(Handle::from_raw(buf.get_u64()).unwrap()) } const TYPE_ID_META: MetadataBuffer = diff --git a/uniffi_macros/src/export/callback_interface.rs b/uniffi_macros/src/export/callback_interface.rs index 85a8b0663a..e17cac48cc 100644 --- a/uniffi_macros/src/export/callback_interface.rs +++ b/uniffi_macros/src/export/callback_interface.rs @@ -45,11 +45,11 @@ pub(super) fn trait_impl( #[doc(hidden)] #[derive(Debug)] struct #trait_impl_ident { - handle: u64, + handle: ::uniffi::Handle, } impl #trait_impl_ident { - fn new(handle: u64) -> Self { + fn new(handle: ::uniffi::Handle) -> Self { Self { handle } } } @@ -109,7 +109,11 @@ pub fn ffi_converter_callback_interface_impl( type FfiType = u64; fn try_lift(v: Self::FfiType) -> ::uniffi::deps::anyhow::Result { - Ok(::std::boxed::Box::new(<#trait_impl_ident>::new(v))) + let handle = match ::uniffi::Handle::from_raw(v) { + Some(h) => h, + None => ::uniffi::deps::anyhow::bail!("{}::try_lift: null handle", #trait_name), + }; + Ok(::std::boxed::Box::new(<#trait_impl_ident>::new(handle))) } fn try_read(buf: &mut &[u8]) -> ::uniffi::deps::anyhow::Result { diff --git a/uniffi_macros/src/setup_scaffolding.rs b/uniffi_macros/src/setup_scaffolding.rs index ebcd1dfd82..fbce21a1e9 100644 --- a/uniffi_macros/src/setup_scaffolding.rs +++ b/uniffi_macros/src/setup_scaffolding.rs @@ -130,7 +130,7 @@ pub fn setup_scaffolding(namespace: String) -> Result { #[doc(hidden)] pub trait UniffiCustomTypeConverter { type Builtin; - fn into_custom(val: Self::Builtin) -> uniffi::Result where Self: Sized; + fn into_custom(val: Self::Builtin) -> ::uniffi::Result where Self: Sized; fn from_custom(obj: Self) -> Self::Builtin; } }) @@ -161,7 +161,6 @@ fn rust_future_scaffolding_fns(module_path: &str) -> TokenStream { (quote! { f32 }, "f32"), (quote! { f64 }, "f64"), (quote! { ::uniffi::Handle }, "handle"), - (quote! { *const ::std::ffi::c_void }, "pointer"), (quote! { ::uniffi::RustBuffer }, "rust_buffer"), (quote! { () }, "void"), ]; @@ -176,7 +175,7 @@ fn rust_future_scaffolding_fns(module_path: &str) -> TokenStream { #[allow(clippy::missing_safety_doc, missing_docs)] #[doc(hidden)] #[no_mangle] - pub unsafe extern "C" fn #ffi_rust_future_poll(handle: ::uniffi::Handle, callback: ::uniffi::RustFutureContinuationCallback, data: *const ()) { + pub unsafe extern "C" fn #ffi_rust_future_poll(handle: ::uniffi::Handle, callback: ::uniffi::RustFutureContinuationCallback, data: ::uniffi::Handle) { ::uniffi::ffi::rust_future_poll::<#return_type, crate::UniFfiTag>(handle, callback, data); }