From 93f7981775a94d8845df2f8ce65b2f27bdbf0e08 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Mon, 29 Jan 2024 18:01:03 -0500 Subject: [PATCH] Consolidate/Simplify the foreign bindings handlemap code I'm trying to get #1823 merged now that 0.26.0 is out, but I'll break it up a bit to make it easier. The first step is consolidating the foreign handle map code. Foreign now languages define 1 handle map and use it for all object types. I tried to simplify the handle map implementations, the had a lot of features that we weren't using at all and seemed to be only half-implemented to me, like the right map and counter map. Handles are always `u64` values. This allows us to remove some code like the `USize` handling on Kotlin and the `FfiType::RustFutureContinuationData` variant. --- CHANGELOG.md | 1 + .../src/bindings/kotlin/gen_kotlin/mod.rs | 1 - .../src/bindings/kotlin/templates/Async.kt | 8 +- .../templates/CallbackInterfaceRuntime.kt | 38 +-------- .../bindings/kotlin/templates/HandleMap.kt | 30 +++++++ .../src/bindings/kotlin/templates/Helpers.kt | 81 ------------------- .../kotlin/templates/ObjectTemplate.kt | 3 +- .../src/bindings/kotlin/templates/wrapper.kt | 2 + .../src/bindings/python/gen_python/mod.rs | 1 - .../src/bindings/python/templates/Async.py | 6 +- .../templates/CallbackInterfaceRuntime.py | 37 +-------- .../bindings/python/templates/HandleMap.py | 31 +++++++ .../templates/NamespaceLibraryTemplate.py | 2 +- .../python/templates/ObjectTemplate.py | 2 +- .../python/templates/PointerManager.py | 68 ---------------- .../python/templates/RustBufferTemplate.py | 3 - .../src/bindings/python/templates/wrapper.py | 4 +- .../src/bindings/ruby/gen_ruby/mod.rs | 2 +- .../src/bindings/swift/gen_swift/mod.rs | 8 +- .../src/bindings/swift/templates/Async.swift | 38 +++------ .../templates/CallbackInterfaceImpl.swift | 12 ++- .../templates/CallbackInterfaceRuntime.swift | 57 ------------- .../templates/CallbackInterfaceTemplate.swift | 15 ++-- .../bindings/swift/templates/HandleMap.swift | 35 ++++++++ .../bindings/swift/templates/Helpers.swift | 8 ++ .../swift/templates/ObjectTemplate.swift | 2 +- .../bindings/swift/templates/wrapper.swift | 1 + uniffi_bindgen/src/interface/ffi.rs | 1 - uniffi_bindgen/src/interface/mod.rs | 4 +- uniffi_core/src/ffi/rustfuture/future.rs | 6 +- uniffi_core/src/ffi/rustfuture/mod.rs | 4 +- uniffi_core/src/ffi/rustfuture/scheduler.rs | 4 +- uniffi_core/src/ffi/rustfuture/tests.rs | 6 +- uniffi_macros/src/setup_scaffolding.rs | 2 +- uniffi_meta/src/lib.rs | 2 +- 35 files changed, 170 insertions(+), 355 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 67d06a4b85..0c1aa08578 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - The callback interface code was reworked to use vtables rather than a single callback method. See https://github.com/mozilla/uniffi-rs/pull/1818 for details and how the other bindings were updated. +- Removed `FfiType::RustFutureContinuationData`. `RustFutureContinuation` callbacks now take a `u64` handle. ## v0.26.0 (backend crates: v0.26.0) - (_2024-01-23_) diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 86c08aae6c..be16218aaf 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -379,7 +379,6 @@ impl KotlinCodeOracle { FfiType::Struct(name) => self.ffi_struct_name(name), FfiType::Reference(inner) => self.ffi_type_label_by_reference(inner), FfiType::VoidPointer | FfiType::RustFutureHandle => "Pointer".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 141a9681de..33fdff69b5 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.toByte() internal const val UNIFFI_RUST_FUTURE_POLL_MAYBE_READY = 1.toByte() -internal val uniffiContinuationHandleMap = UniFfiHandleMap>() +internal val uniffiContinuationHandleMap = UniffiHandleMap>() // FFI type for Rust future continuations internal object uniffiRustFutureContinuationCallback: UniffiRustFutureContinuationCallback { - override fun callback(data: USize, pollResult: Byte) { - uniffiContinuationHandleMap.remove(data)?.resume(pollResult) + override fun callback(data: Long, pollResult: Byte) { + uniffiContinuationHandleMap.remove(data).resume(pollResult) } } internal suspend fun uniffiRustCallAsync( rustFuture: Pointer, - pollFunc: (Pointer, UniffiRustFutureContinuationCallback, USize) -> Unit, + pollFunc: (Pointer, UniffiRustFutureContinuationCallback, Long) -> Unit, completeFunc: (Pointer, UniffiRustCallStatus) -> F, freeFunc: (Pointer) -> Unit, liftFunc: (F) -> T, diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt index e7b7dc4725..6bd6c93fb9 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/CallbackInterfaceRuntime.kt @@ -1,37 +1,3 @@ -{{- self.add_import("java.util.concurrent.atomic.AtomicLong") }} -{{- self.add_import("java.util.concurrent.locks.ReentrantLock") }} -{{- self.add_import("kotlin.concurrent.withLock") }} - -internal typealias UniffiHandle = 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): UniffiHandle = - lock.withLock { - currentHandle.getAndAdd(stride) - .also { handle -> - leftMap[handle] = obj - } - } - - fun get(handle: UniffiHandle) = lock.withLock { - leftMap[handle] ?: throw InternalException("No callback in handlemap; this is a Uniffi bug") - } - - fun delete(handle: UniffiHandle) { - this.remove(handle) - } - - fun remove(handle: UniffiHandle): T? = - lock.withLock { - leftMap.remove(handle) - } -} - // 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. internal const val IDX_CALLBACK_FREE = 0 @@ -40,8 +6,8 @@ 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() +public abstract class FfiConverterCallbackInterface: FfiConverter { + internal val handleMap = UniffiHandleMap() internal fun drop(handle: UniffiHandle) { handleMap.remove(handle) 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..a06c4468ba --- /dev/null +++ b/uniffi_bindgen/src/bindings/kotlin/templates/HandleMap.kt @@ -0,0 +1,30 @@ +// Handle from a UniffiHandleMap +internal typealias UniffiHandle = Long + +// Map handles to objects +// +// This is used pass an opaque 64-bit handle representing a foreign object to the Rust code. +internal class UniffiHandleMap { + private val map = ConcurrentHashMap() + private val counter = java.util.concurrent.atomic.AtomicLong(0) + + val size: Int + get() = map.size + + // Insert a new object into the handle map and get a handle for it + fun insert(obj: T): UniffiHandle { + val handle = counter.getAndAdd(1) + map.put(handle, obj) + return handle + } + + // Get an object from the handle map + fun get(handle: UniffiHandle): T { + return map.get(handle) ?: throw InternalException("UniffiHandleMap.get: Invalid handle") + } + + // Remove an entry from the handlemap and get the Kotlin object back + fun remove(handle: UniffiHandle): T { + return map.remove(handle) ?: throw InternalException("UniffiHandleMap: Invalid handle") + } +} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt index d82a6a7407..b9e55ff821 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt @@ -77,51 +77,6 @@ private inline fun uniffiRustCall(callback: (UniffiRustCallStatus) -> U): U return uniffiRustCallWithError(UniffiNullRustCallStatusErrorHandler, 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) - } - } - } -} - internal inline fun uniffiTraitInterfaceCall( callStatus: UniffiRustCallStatus, makeCall: () -> T, @@ -153,39 +108,3 @@ internal inline fun uniffiTraitInterfaceCallWithError( } } } - -// 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) - } -} diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index 8492b832c7..d91c0eab31 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -95,7 +95,6 @@ // [1] https://stackoverflow.com/questions/24376768/can-java-finalize-an-object-when-it-is-still-in-scope/24380219 // -{{ self.add_import("java.util.concurrent.atomic.AtomicLong") }} {{ self.add_import("java.util.concurrent.atomic.AtomicBoolean") }} {%- if self.include_once_check("interface-support") %} {%- include "ObjectCleanerHelper.kt" %} @@ -312,7 +311,7 @@ open class {{ impl_class_name }}: Disposable, AutoCloseable, {{ interface_name } public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, Pointer> { {%- if obj.has_callback_interface() %} - internal val handleMap = ConcurrentHandleMap<{{ interface_name }}>() + internal val handleMap = UniffiHandleMap<{{ type_name }}>() {%- endif %} override fun lower(value: {{ type_name }}): Pointer { diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt index 0f924e4d63..2cdc72a5e2 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/wrapper.kt @@ -30,6 +30,7 @@ import java.nio.ByteBuffer import java.nio.ByteOrder import java.nio.CharBuffer import java.nio.charset.CodingErrorAction +import java.util.concurrent.atomic.AtomicLong import java.util.concurrent.ConcurrentHashMap {%- for req in self.imports() %} @@ -39,6 +40,7 @@ import java.util.concurrent.ConcurrentHashMap {% include "RustBufferTemplate.kt" %} {% include "FfiConverterTemplate.kt" %} {% include "Helpers.kt" %} +{% include "HandleMap.kt" %} // Contains loading, initialization code, // and the FFI Function declarations in a com.sun.jna.Library. diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index 699cf22b2f..d25314659e 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -372,7 +372,6 @@ impl PythonCodeOracle { // Pointer to an `asyncio.EventLoop` instance FfiType::Reference(inner) => format!("ctypes.POINTER({})", self.ffi_type_label(inner)), FfiType::VoidPointer | FfiType::RustFutureHandle => "ctypes.c_void_p".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 aae87ab98b..4a230112ea 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() +_UniffiContinuationHandleMap = _UniffiHandleMap() # Continuation callback for async functions # lift the return value or error and resolve the future, causing the async function to resume. @UNIFFI_RUST_FUTURE_CONTINUATION_CALLBACK def _uniffi_continuation_callback(future_ptr, poll_code): - (eventloop, future) = _UniffiContinuationPointerManager.release_pointer(future_ptr) + (eventloop, future) = _UniffiContinuationHandleMap.remove(future_ptr) eventloop.call_soon_threadsafe(_uniffi_set_future_result, future, poll_code) def _uniffi_set_future_result(future, poll_code): @@ -26,7 +26,7 @@ async def _uniffi_rust_call_async(rust_future, ffi_poll, ffi_complete, ffi_free, ffi_poll( rust_future, _uniffi_continuation_callback, - _UniffiContinuationPointerManager.new_pointer((eventloop, future)), + _UniffiContinuationHandleMap.insert((eventloop, future)), ) poll_code = await future if poll_code == _UNIFFI_RUST_FUTURE_POLL_READY: diff --git a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py index 1337d9685f..d802c90fde 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): 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..30472a067d --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/templates/HandleMap.py @@ -0,0 +1,31 @@ +class _UniffiHandleMap: + """ + A map where inserting, getting and removing data is synchronized with a lock. + """ + + def __init__(self): + # type Handle = int + self._map = {} # type: Dict[Handle, Any] + self._lock = threading.Lock() + self._counter = itertools.count() + + def insert(self, obj): + with self._lock: + handle = next(self._counter) + self._map[handle] = obj + return handle + + def get(self, handle): + try: + with self._lock: + return self._map[handle] + except KeyError: + raise InternalError("UniffiHandleMap.get: Invalid handle") + + def remove(self, handle): + try: + with self._lock: + return self._map.pop(handle) + except KeyError: + raise InternalError("UniffiHandleMap.remove: Invalid handle") + diff --git a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py index 3845d8a8b9..9af1bd12f2 100644 --- a/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py @@ -9,7 +9,7 @@ def _uniffi_future_callback_t(return_type): """ Factory function to create callback function types for async functions """ - return ctypes.CFUNCTYPE(None, ctypes.c_size_t, return_type, _UniffiRustCallStatus) + return ctypes.CFUNCTYPE(None, ctypes.c_uint64, return_type, _UniffiRustCallStatus) def _uniffi_load_indirect(): """ diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index ddcb9246bf..212e1f7a6c 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -84,7 +84,7 @@ def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}: class {{ ffi_converter_name }}: {%- if obj.has_callback_interface() %} - _handle_map = ConcurrentHandleMap() + _handle_map = _UniffiHandleMap() {%- endif %} @staticmethod 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..564f94fc3d 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. diff --git a/uniffi_bindgen/src/bindings/python/templates/wrapper.py b/uniffi_bindgen/src/bindings/python/templates/wrapper.py index 276ba868c3..67647c68ad 100644 --- a/uniffi_bindgen/src/bindings/python/templates/wrapper.py +++ b/uniffi_bindgen/src/bindings/python/templates/wrapper.py @@ -22,6 +22,8 @@ import struct import contextlib import datetime +import threading +import itertools import typing {%- if ci.has_async_fns() %} import asyncio @@ -36,7 +38,7 @@ {% include "RustBufferTemplate.py" %} {% include "Helpers.py" %} -{% include "PointerManager.py" %} +{% include "HandleMap.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 8e30ee0980..a2d6d41247 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -162,7 +162,7 @@ mod filters { FfiType::Struct(_) => { unimplemented!("Structs are not implemented") } - FfiType::RustFutureHandle | FfiType::RustFutureContinuationData => { + FfiType::RustFutureHandle => { 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 14d1c01a0d..6f1464efe7 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -537,9 +537,7 @@ impl SwiftCodeOracle { FfiType::Reference(inner) => { format!("UnsafeMutablePointer<{}>", self.ffi_type_label(inner)) } - FfiType::VoidPointer - | FfiType::RustFutureHandle - | FfiType::RustFutureContinuationData => "UnsafeMutableRawPointer".into(), + FfiType::VoidPointer | FfiType::RustFutureHandle => "UnsafeMutableRawPointer".into(), } } @@ -640,9 +638,7 @@ pub mod filters { } FfiType::Struct(name) => SwiftCodeOracle.ffi_struct_name(name), FfiType::Reference(inner) => format!("{}* _Nonnull", header_ffi_type_name(inner)?), - FfiType::VoidPointer - | FfiType::RustFutureHandle - | FfiType::RustFutureContinuationData => "void* _Nonnull".into(), + FfiType::VoidPointer | FfiType::RustFutureHandle => "void* _Nonnull".into(), }) } diff --git a/uniffi_bindgen/src/bindings/swift/templates/Async.swift b/uniffi_bindgen/src/bindings/swift/templates/Async.swift index 578ff4ddf4..b91e264fe8 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 let uniffiContinuationHandleMap = UniffiHandleMap>() + fileprivate func uniffiRustCallAsync( rustFutureFunc: () -> UnsafeMutableRawPointer, - pollFunc: (UnsafeMutableRawPointer, @escaping UniffiRustFutureContinuationCallback, UnsafeMutableRawPointer) -> (), + pollFunc: (UnsafeMutableRawPointer, @escaping UniffiRustFutureContinuationCallback, UInt64) -> (), completeFunc: (UnsafeMutableRawPointer, UnsafeMutablePointer) -> F, freeFunc: (UnsafeMutableRawPointer) -> (), liftFunc: (F) throws -> T, @@ -19,7 +21,11 @@ fileprivate func uniffiRustCallAsync( var pollResult: Int8; repeat { pollResult = await withUnsafeContinuation { - pollFunc(rustFuture, uniffiFutureContinuationCallback, ContinuationHolder($0).toOpaque()) + pollFunc( + rustFuture, + uniffiFutureContinuationCallback, + uniffiContinuationHandleMap.insert(obj: $0) + ) } } while pollResult != UNIFFI_RUST_FUTURE_POLL_READY @@ -31,28 +37,10 @@ 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) { + if let continuation = try? uniffiContinuationHandleMap.remove(handle: handle) { + continuation.resume(returning: pollResult) + } else { + print("uniffiFutureContinuationCallback invalid handle") } } diff --git a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift index 91d1d854e2..2db6729c30 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceImpl.swift @@ -16,9 +16,12 @@ fileprivate struct {{ trait_impl }} { uniffiCallStatus: UnsafeMutablePointer {%- endif %} ) in - guard let uniffiObj = {{ ffi_converter_name }}.handleMap.get(handle: uniffiHandle) else { + let uniffiObj: {{ type_name }} + do { + try uniffiObj = {{ ffi_converter_name }}.handleMap.get(handle: uniffiHandle) + } catch { uniffiCallStatus.pointee.code = CALL_UNEXPECTED_ERROR - uniffiCallStatus.pointee.errorBuf = {{ Type::String.borrow()|lower_fn }}("No callback in handlemap; this is a Uniffi bug") + uniffiCallStatus.pointee.errorBuf = {{ Type::String.borrow()|lower_fn }}("Callback handle map error: \(error)") return } let makeCall = { {% if meth.throws() %}try {% endif %}uniffiObj.{{ meth.name()|fn_name }}( @@ -51,7 +54,10 @@ fileprivate struct {{ trait_impl }} { }, {%- endfor %} uniffiFree: { (uniffiHandle: UInt64) -> () in - {{ ffi_converter_name }}.handleMap.delete(handle: uniffiHandle) + let result = try? {{ ffi_converter_name }}.handleMap.remove(handle: uniffiHandle) + if result == nil { + print("Uniffi callback interface {{ name }}: handle missing in uniffiFree") + } } ) } 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 8cdd735b9a..1827dc19bf 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/CallbackInterfaceTemplate.swift @@ -13,27 +13,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 = UniffiHandle - 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: UniffiHandle) throws -> SwiftType { + try 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: UniffiHandle = try readInt(&buf) return try lift(handle) } - public static func lower(_ v: SwiftType) -> UniFFICallbackHandle { + public static func lower(_ v: SwiftType) -> UniffiHandle { return handleMap.insert(obj: v) } 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..c636ca6be6 --- /dev/null +++ b/uniffi_bindgen/src/bindings/swift/templates/HandleMap.swift @@ -0,0 +1,35 @@ +fileprivate typealias UniffiHandle = UInt64 +fileprivate class UniffiHandleMap { + private var map: [UniffiHandle: T] = [:] + private let lock = NSLock() + private var currentHandle: UniffiHandle = 1 + + func insert(obj: T) -> UniffiHandle { + lock.withLock { + let handle = currentHandle + currentHandle += 1 + map[handle] = obj + return handle + } + } + + func get(handle: UniffiHandle) throws -> T { + try lock.withLock { + guard let obj = map[handle] else { + throw UniffiInternalError.unexpectedStaleHandle + } + return obj + } + } + + @discardableResult + func remove(handle: UniffiHandle) throws -> T { + try lock.withLock { + guard let obj = map.removeValue(forKey: handle) else { + throw UniffiInternalError.unexpectedStaleHandle + } + return obj + } + } +} + diff --git a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift index d233d4b762..cfddf7b313 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Helpers.swift @@ -26,6 +26,14 @@ fileprivate enum UniffiInternalError: LocalizedError { } } +fileprivate extension NSLock { + func withLock(f: () throws -> T) rethrows -> T { + self.lock() + defer { self.unlock() } + return try f() + } +} + fileprivate let CALL_SUCCESS: Int8 = 0 fileprivate let CALL_ERROR: Int8 = 1 fileprivate let CALL_UNEXPECTED_ERROR: Int8 = 2 diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index fc5d47516e..b54ea27945 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -154,7 +154,7 @@ public class {{ impl_class_name }}: public struct {{ ffi_converter_name }}: FfiConverter { {%- if obj.has_callback_interface() %} - fileprivate static var handleMap = UniFFICallbackHandleMap<{{ type_name }}>() + fileprivate static var handleMap = UniffiHandleMap<{{ type_name }}>() {%- endif %} typealias FfiType = UnsafeMutableRawPointer diff --git a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift index fb86d62e37..b4591482e1 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/wrapper.swift @@ -18,6 +18,7 @@ import {{ config.ffi_module_name() }} {% include "RustBufferTemplate.swift" %} {% include "Helpers.swift" %} +{% include "HandleMap.swift" %} // Public interface members begin here. {{ type_helper_code }} diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index 7259b5bf32..44b09d3fea 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -55,7 +55,6 @@ pub enum FfiType { Struct(String), /// Pointer to a Rust future RustFutureHandle, - RustFutureContinuationData, /// Pointer to an FfiType. Reference(Box), /// Opaque pointer diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 823d3672cc..375f7c0ea2 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -230,7 +230,7 @@ impl ComponentInterface { FfiCallbackFunction { name: "RustFutureContinuationCallback".to_owned(), arguments: vec![ - FfiArgument::new("data", FfiType::RustFutureContinuationData), + FfiArgument::new("data", FfiType::UInt64), FfiArgument::new("poll_result", FfiType::Int8), ], return_type: None, @@ -522,7 +522,7 @@ impl ComponentInterface { }, FfiArgument { name: "callback_data".to_owned(), - type_: FfiType::RustFutureContinuationData, + type_: FfiType::UInt64, }, ], return_type: None, diff --git a/uniffi_core/src/ffi/rustfuture/future.rs b/uniffi_core/src/ffi/rustfuture/future.rs index b104b20a32..a42729bf36 100644 --- a/uniffi_core/src/ffi/rustfuture/future.rs +++ b/uniffi_core/src/ffi/rustfuture/future.rs @@ -223,7 +223,7 @@ where }) } - pub(super) fn poll(self: Arc, callback: RustFutureContinuationCallback, data: *const ()) { + pub(super) fn poll(self: Arc, callback: RustFutureContinuationCallback, data: u64) { let ready = self.is_cancelled() || { let mut locked = self.future.lock().unwrap(); let waker: std::task::Waker = Arc::clone(&self).into(); @@ -289,7 +289,7 @@ where /// 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 ()); + fn ffi_poll(self: Arc, callback: RustFutureContinuationCallback, data: u64); fn ffi_cancel(&self); fn ffi_complete(&self, call_status: &mut RustCallStatus) -> ReturnType; fn ffi_free(self: Arc); @@ -302,7 +302,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: u64) { self.poll(callback, data) } diff --git a/uniffi_core/src/ffi/rustfuture/mod.rs b/uniffi_core/src/ffi/rustfuture/mod.rs index 4aaf013fd5..3aec88a2a8 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: u64, RustFuturePoll); /// Opaque handle for a Rust future that's stored by the foreign language code #[repr(transparent)] @@ -75,7 +75,7 @@ where pub unsafe fn rust_future_poll( handle: RustFutureHandle, callback: RustFutureContinuationCallback, - data: *const (), + data: u64, ) { let future = &*(handle.0 as *mut Arc>); future.clone().ffi_poll(callback, data) diff --git a/uniffi_core/src/ffi/rustfuture/scheduler.rs b/uniffi_core/src/ffi/rustfuture/scheduler.rs index aae5a0c1cf..629ee0c109 100644 --- a/uniffi_core/src/ffi/rustfuture/scheduler.rs +++ b/uniffi_core/src/ffi/rustfuture/scheduler.rs @@ -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, u64), } 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: u64) { match self { Self::Empty => *self = Self::Set(callback, data), Self::Set(old_callback, old_data) => { diff --git a/uniffi_core/src/ffi/rustfuture/tests.rs b/uniffi_core/src/ffi/rustfuture/tests.rs index 1f68085562..886ee27c71 100644 --- a/uniffi_core/src/ffi/rustfuture/tests.rs +++ b/uniffi_core/src/ffi/rustfuture/tests.rs @@ -67,12 +67,12 @@ 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 = Arc::into_raw(cell.clone()) as u64; + rust_future.clone().ffi_poll(poll_continuation, handle); cell } -extern "C" fn poll_continuation(data: *const (), code: RustFuturePoll) { +extern "C" fn poll_continuation(data: u64, code: RustFuturePoll) { let cell = unsafe { Arc::from_raw(data as *const OnceCell) }; cell.set(code).expect("Error setting OnceCell"); } diff --git a/uniffi_macros/src/setup_scaffolding.rs b/uniffi_macros/src/setup_scaffolding.rs index 1d0c368504..58bb1ee483 100644 --- a/uniffi_macros/src/setup_scaffolding.rs +++ b/uniffi_macros/src/setup_scaffolding.rs @@ -166,7 +166,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::RustFutureHandle, callback: ::uniffi::RustFutureContinuationCallback, data: *const ()) { + pub unsafe extern "C" fn #ffi_rust_future_poll(handle: ::uniffi::RustFutureHandle, callback: ::uniffi::RustFutureContinuationCallback, data: u64) { ::uniffi::ffi::rust_future_poll::<#return_type>(handle, callback, data); } diff --git a/uniffi_meta/src/lib.rs b/uniffi_meta/src/lib.rs index eb4cdca572..c7d8b3868a 100644 --- a/uniffi_meta/src/lib.rs +++ b/uniffi_meta/src/lib.rs @@ -23,7 +23,7 @@ mod metadata; // `docs/uniffi-versioning.md` for details. // // Once we get to 1.0, then we'll need to update the scheme to something like 100 + major_version -pub const UNIFFI_CONTRACT_VERSION: u32 = 25; +pub const UNIFFI_CONTRACT_VERSION: u32 = 26; /// Similar to std::hash::Hash. ///