diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec91775de..257510aa8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,11 @@ - 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. +- The `NoPointer` singleton was renamed to `NoHandle` ### What's new? diff --git a/fixtures/coverall/tests/bindings/test_coverall.kts b/fixtures/coverall/tests/bindings/test_coverall.kts index f091591601..44f17b6273 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.kts +++ b/fixtures/coverall/tests/bindings/test_coverall.kts @@ -446,11 +446,11 @@ Coveralls("test_bytes").use { coveralls -> // Test fakes using open classes -class FakePatch(private val color: Color): Patch(NoPointer) { +class FakePatch(private val color: Color): Patch(NoHandle) { override fun `getColor`(): Color = color } -class FakeCoveralls(private val name: String) : Coveralls(NoPointer) { +class FakeCoveralls(private val name: String) : Coveralls(NoHandle) { private val repairs = mutableListOf() override fun `addPatch`(patch: Patch) { diff --git a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts index 38ee5a7abd..404c06a463 100644 --- a/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts +++ b/fixtures/foreign-executor/tests/bindings/test_foreign_executor.kts @@ -37,11 +37,3 @@ runBlocking { assert(result.delayMs <= 200U) tester.close() } - -// Test that we cleanup when dropping a ForeignExecutor handles -assert(FfiConverterForeignExecutor.handleCount() == 0) -val tester = ForeignExecutorTester(coroutineScope) -val tester2 = ForeignExecutorTester.newFromSequence(listOf(coroutineScope)) -tester.close() -tester2.close() -assert(FfiConverterForeignExecutor.handleCount() == 0) diff --git a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr index a8e7dc90a1..72a209e543 100644 --- a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr +++ b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr @@ -1,3 +1,31 @@ +error[E0277]: `(dyn Trait + 'static)` cannot be shared between threads safely + --> $OUT_DIR[uniffi_uitests]/trait.uniffi.rs + | + | #[::uniffi::export_for_udl] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Trait + 'static)` cannot be shared between threads safely + | + = help: the trait `Sync` is not implemented for `(dyn Trait + 'static)` +note: required by a bound in `HandleAlloc` + --> $WORKSPACE/uniffi_core/src/ffi_converter_traits.rs + | + | pub unsafe trait HandleAlloc: Send + Sync { + | ^^^^ required by this bound in `HandleAlloc` + = note: this error originates in the attribute macro `::uniffi::export_for_udl` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: `(dyn Trait + 'static)` cannot be sent between threads safely + --> $OUT_DIR[uniffi_uitests]/trait.uniffi.rs + | + | #[::uniffi::export_for_udl] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Trait + 'static)` cannot be sent between threads safely + | + = help: the trait `Send` is not implemented for `(dyn Trait + 'static)` +note: required by a bound in `HandleAlloc` + --> $WORKSPACE/uniffi_core/src/ffi_converter_traits.rs + | + | pub unsafe trait HandleAlloc: Send + Sync { + | ^^^^ required by this bound in `HandleAlloc` + = note: this error originates in the attribute macro `::uniffi::export_for_udl` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0277]: `(dyn Trait + 'static)` cannot be shared between threads safely --> $OUT_DIR[uniffi_uitests]/trait.uniffi.rs | @@ -26,6 +54,34 @@ note: required by a bound in `FfiConverterArc` | ^^^^ required by this bound in `FfiConverterArc` = note: this error originates in the attribute macro `::uniffi::export_for_udl` (in Nightly builds, run with -Z macro-backtrace for more info) +error[E0277]: `(dyn ProcMacroTrait + 'static)` cannot be shared between threads safely + --> tests/ui/interface_trait_not_sync_and_send.rs:11:1 + | +11 | #[uniffi::export] + | ^^^^^^^^^^^^^^^^^ `(dyn ProcMacroTrait + 'static)` cannot be shared between threads safely + | + = help: the trait `Sync` is not implemented for `(dyn ProcMacroTrait + 'static)` +note: required by a bound in `HandleAlloc` + --> $WORKSPACE/uniffi_core/src/ffi_converter_traits.rs + | + | pub unsafe trait HandleAlloc: Send + Sync { + | ^^^^ required by this bound in `HandleAlloc` + = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: `(dyn ProcMacroTrait + 'static)` cannot be sent between threads safely + --> tests/ui/interface_trait_not_sync_and_send.rs:11:1 + | +11 | #[uniffi::export] + | ^^^^^^^^^^^^^^^^^ `(dyn ProcMacroTrait + 'static)` cannot be sent between threads safely + | + = help: the trait `Send` is not implemented for `(dyn ProcMacroTrait + 'static)` +note: required by a bound in `HandleAlloc` + --> $WORKSPACE/uniffi_core/src/ffi_converter_traits.rs + | + | pub unsafe trait HandleAlloc: Send + Sync { + | ^^^^ required by this bound in `HandleAlloc` + = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0277]: `(dyn ProcMacroTrait + 'static)` cannot be shared between threads safely --> tests/ui/interface_trait_not_sync_and_send.rs:11:1 | diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 8504a0d9e5..c1b4f98995 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -333,12 +333,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 d2e6048188..dcf1bec631 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 20ae44b9a3..cfd7383556 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt @@ -1,5 +1,16 @@ {{- self.add_import("java.util.concurrent.atomic.AtomicLong") }} {{- self.add_import("java.util.concurrent.atomic.AtomicBoolean") }} + +// 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 @@ -100,7 +111,7 @@ abstract class FFIObject: Disposable, AutoCloseable { this.handle = null } - protected val handle: UniffiHandle? + internal val handle: UniffiHandle? private val wasDestroyed = AtomicBoolean(false) private val callCounter = AtomicLong(1) diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index 83d58f86e9..81ec179062 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -9,7 +9,7 @@ {%- call kt::docstring(obj, 0) %} open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { - constructor(handle: UniffiHandle): super(handle) + constructor(handleWrapper: UniffiHandleWrapper): super(handleWrapper.handle) /** * This constructor can be used to instantiate a fake object. @@ -25,7 +25,7 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { {%- when Some with (cons) %} {%- call kt::docstring(cons, 4) %} 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 %} @@ -106,7 +106,7 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { {%- match tm %} {%- when UniffiTrait::Display { fmt } %} override fun toString(): String = - callWithPointer { + callWithHandle { {%- call kt::to_ffi_call_with_prefix("it", fmt) %} }.let { {{ fmt.return_type().unwrap()|lift_fn }}(it) @@ -116,7 +116,7 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { override fun equals(other: Any?): Boolean { if (this === other) return true if (other !is {{ impl_class_name}}) return false - return callWithPointer { + return callWithHandle { {%- call kt::to_ffi_call_with_prefix("it", eq) %} }.let { {{ eq.return_type().unwrap()|lift_fn }}(it) @@ -124,7 +124,7 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { } {%- when UniffiTrait::Hash { hash } %} override fun hashCode(): Int = - callWithPointer { + callWithHandle { {%- call kt::to_ffi_call_with_prefix("it", hash) %} }.let { {{ hash.return_type().unwrap()|lift_fn }}(it).toInt() @@ -138,7 +138,7 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { {% for cons in obj.alternate_constructors() -%} {%- call kt::docstring(cons, 4) %} 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 %} @@ -155,20 +155,20 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_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 { {%- match obj.imp() %} {%- when ObjectImpl::Struct %} - return value.handle + 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 0f924e4d63..5c43941046 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt @@ -31,12 +31,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 5b50acb1f7..3854cd3a9f 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -357,11 +357,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 3c4f79ea7f..6829106f73 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -80,22 +80,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 @@ -104,5 +104,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 276ba868c3..f9a95500de 100644 --- a/uniffi_bindgen/src/bindings/python/templates/wrapper.py +++ b/uniffi_bindgen/src/bindings/python/templates/wrapper.py @@ -19,9 +19,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 @@ -35,8 +37,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 ee07c9c0e2..868bdd8ac7 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -530,10 +530,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(), } } @@ -541,8 +539,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), @@ -644,11 +641,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 f985ae3cd3..cc27580ae8 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift @@ -11,28 +11,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 31b2dadfc2..14754d27e2 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 b4ccffddb2..7bb2f3e44f 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -111,25 +111,25 @@ public class {{ impl_class_name }}: {%- when UniffiTrait::Display { fmt } %} public var description: String { return {% call swift::try(fmt) %} {{ fmt.return_type().unwrap()|lift_fn }}( - {% call swift::to_ffi_call_with_prefix("self.pointer", fmt) %} + {% call swift::to_ffi_call_with_prefix("self.handle", fmt) %} ) } {%- when UniffiTrait::Debug { fmt } %} public var debugDescription: String { return {% call swift::try(fmt) %} {{ fmt.return_type().unwrap()|lift_fn }}( - {% call swift::to_ffi_call_with_prefix("self.pointer", fmt) %} + {% call swift::to_ffi_call_with_prefix("self.handle", fmt) %} ) } {%- when UniffiTrait::Eq { eq, ne } %} public static func == (lhs: {{ impl_class_name }}, other: {{ impl_class_name }}) -> Bool { return {% call swift::try(eq) %} {{ eq.return_type().unwrap()|lift_fn }}( - {% call swift::to_ffi_call_with_prefix("lhs.pointer", eq) %} + {% call swift::to_ffi_call_with_prefix("lhs.handle", eq) %} ) } {%- when UniffiTrait::Hash { hash } %} public func hash(into hasher: inout Hasher) { let val = {% call swift::try(hash) %} {{ hash.return_type().unwrap()|lift_fn }}( - {% call swift::to_ffi_call_with_prefix("self.pointer", hash) %} + {% call swift::to_ffi_call_with_prefix("self.handle", hash) %} ) hasher.combine(val) } @@ -148,7 +148,7 @@ public class {{ impl_class_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 @@ -163,10 +163,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 fb86d62e37..7286137409 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift @@ -17,6 +17,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 0aad3a4272..6eef3a7e85 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 b4237d0203..9281147b27 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, }; @@ -162,6 +163,15 @@ impl ComponentInterface { self.types.namespace_docstring.as_deref() } + /// 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"); @@ -468,8 +478,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 2301417a0e..05d809dd8e 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -195,7 +195,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; @@ -435,7 +435,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 8ee2f46c35..65bea35f44 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/future.rs b/uniffi_core/src/ffi/rustfuture/future.rs index c759a26f0a..861751d3db 100644 --- a/uniffi_core/src/ffi/rustfuture/future.rs +++ b/uniffi_core/src/ffi/rustfuture/future.rs @@ -86,7 +86,7 @@ use std::{ }; use super::{RustFutureContinuationCallback, RustFuturePoll, Scheduler}; -use crate::{rust_call_with_out_status, FfiDefault, LowerReturn, RustCallStatus}; +use crate::{rust_call_with_out_status, FfiDefault, Handle, LowerReturn, RustCallStatus}; /// Wraps the actual future we're polling struct WrappedFuture @@ -229,7 +229,7 @@ where }) } - pub(super) fn poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()) { + pub(super) 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(); @@ -294,8 +294,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); @@ -308,7 +308,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) } diff --git a/uniffi_core/src/ffi/rustfuture/mod.rs b/uniffi_core/src/ffi/rustfuture/mod.rs index 879a45f30e..2ed7218b45 100644 --- a/uniffi_core/src/ffi/rustfuture/mod.rs +++ b/uniffi_core/src/ffi/rustfuture/mod.rs @@ -28,7 +28,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 === @@ -64,7 +64,7 @@ where pub unsafe fn rust_future_poll( handle: Handle, callback: RustFutureContinuationCallback, - data: *const (), + data: Handle, ) where dyn RustFutureFfi: HandleAlloc, { @@ -135,4 +135,3 @@ 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>); diff --git a/uniffi_core/src/ffi/rustfuture/scheduler.rs b/uniffi_core/src/ffi/rustfuture/scheduler.rs index aae5a0c1cf..2de19c5d1a 100644 --- a/uniffi_core/src/ffi/rustfuture/scheduler.rs +++ b/uniffi_core/src/ffi/rustfuture/scheduler.rs @@ -4,7 +4,7 @@ use std::mem; -use super::{RustFutureContinuationCallback, RustFuturePoll}; +use super::{Handle, RustFutureContinuationCallback, RustFuturePoll}; /// Schedules a [crate::RustFuture] by managing the continuation data /// @@ -30,7 +30,7 @@ pub(super) enum Scheduler { /// continuation being called with `RustFuturePoll::Ready`. Cancelled, /// Continuation set, the next time `wake()` is called is called, we should invoke it. - Set(RustFutureContinuationCallback, *const ()), + Set(RustFutureContinuationCallback, Handle), } impl Scheduler { @@ -40,7 +40,7 @@ impl Scheduler { /// 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. - pub(super) fn store(&mut self, callback: RustFutureContinuationCallback, data: *const ()) { + pub(super) fn store(&mut self, callback: RustFutureContinuationCallback, data: Handle) { match self { Self::Empty => *self = Self::Set(callback, data), Self::Set(old_callback, old_data) => { @@ -87,10 +87,3 @@ impl Scheduler { matches!(self, Self::Cancelled) } } - -// The `*const ()` data pointer references an object on the foreign side. -// This object must be `Sync` in Rust terminology -- it must be safe for us to pass the pointer to the continuation callback from any thread. -// If the foreign side upholds their side of the contract, then `Scheduler` is Send + Sync. - -unsafe impl Send for Scheduler {} -unsafe impl Sync for Scheduler {} diff --git a/uniffi_core/src/ffi/rustfuture/tests.rs b/uniffi_core/src/ffi/rustfuture/tests.rs index 1f68085562..83f74831a3 100644 --- a/uniffi_core/src/ffi/rustfuture/tests.rs +++ b/uniffi_core/src/ffi/rustfuture/tests.rs @@ -67,13 +67,14 @@ fn channel() -> (Sender, Arc>) { /// 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); }