Skip to content

Commit

Permalink
Adding FfiType::Handle and use it for Rust objects
Browse files Browse the repository at this point in the history
  • Loading branch information
bendk committed Nov 22, 2023
1 parent 49e6d77 commit b06a1f8
Show file tree
Hide file tree
Showing 27 changed files with 251 additions and 286 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

- 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.

### What's new?

Expand Down
3 changes: 1 addition & 2 deletions uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,15 +327,14 @@ impl KotlinCodeOracle {
FfiType::Int64 | FfiType::UInt64 => "Long".to_string(),
FfiType::Float32 => "Float".to_string(),
FfiType::Float64 => "Double".to_string(),
FfiType::RustArcPtr(_) => "Pointer".to_string(),
FfiType::Handle => "UniffiHandle".to_string(),
FfiType::RustBuffer(maybe_suffix) => {
format!("RustBuffer{}", maybe_suffix.as_deref().unwrap_or_default())
}
FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(),
FfiType::ForeignCallback => "ForeignCallback".to_string(),
FfiType::ForeignExecutorHandle => "USize".to_string(),
FfiType::ForeignExecutorCallback => "UniFfiForeignExecutorCallback".to_string(),
FfiType::RustFutureHandle => "Pointer".to_string(),
FfiType::RustFutureContinuationCallback => {
"UniFffiRustFutureContinuationCallbackType".to_string()
}
Expand Down
8 changes: 4 additions & 4 deletions uniffi_bindgen/src/bindings/kotlin/templates/Async.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ internal object uniffiRustFutureContinuationCallback: UniFffiRustFutureContinuat
}

internal suspend fun<T, F, E: Exception> uniffiRustCallAsync(
rustFuture: Pointer,
pollFunc: (Pointer, UniFffiRustFutureContinuationCallbackType, USize) -> Unit,
completeFunc: (Pointer, RustCallStatus) -> F,
freeFunc: (Pointer) -> Unit,
rustFuture: UniffiHandle,
pollFunc: (UniffiHandle, UniFffiRustFutureContinuationCallbackType, USize) -> Unit,
completeFunc: (UniffiHandle, RustCallStatus) -> F,
freeFunc: (UniffiHandle) -> Unit,
liftFunc: (F) -> T,
errorHandler: CallStatusErrorHandler<E>
): T {
Expand Down
5 changes: 5 additions & 0 deletions uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// Handles are defined as unsigned in Rust, but that's doesn't work well with JNA.
// We can pretend its signed since Rust handles are opaque values and Kotlin handles don't use all
// 64 bits.
typealias UniffiHandle = Long

// A handful of classes and functions to support the generated data structures.
// This would be a good candidate for isolating in its own ffi-support lib.
// Error runtime.
Expand Down
40 changes: 20 additions & 20 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@
{{- self.add_import("java.util.concurrent.atomic.AtomicBoolean") }}
// The base class for all UniFFI Object types.
//
// This class provides core operations for working with the Rust `Arc<T>` pointer to
// the live Rust struct on the other side of the FFI.
// This class provides core operations for working with the Rust handle to the live Rust struct on
// the other side of the FFI.
//
// There's some subtlety here, because we have to be careful not to operate on a Rust
// struct after it has been dropped, and because we must expose a public API for freeing
// the Kotlin wrapper object in lieu of reliable finalizers. The core requirements are:
//
// * Each `FFIObject` instance holds an opaque pointer to the underlying Rust struct.
// Method calls need to read this pointer from the object's state and pass it in to
// * Each `FFIObject` instance holds an opaque handle to the underlying Rust struct.
// Method calls need to read this handle from the object's state and pass it in to
// the Rust FFI.
//
// * When an `FFIObject` is no longer needed, its pointer should be passed to a
// * When an `FFIObject` is no longer needed, its handle should be passed to a
// special destructor function provided by the Rust FFI, which will drop the
// underlying Rust struct.
//
Expand All @@ -30,13 +30,13 @@
// the destructor has been called, and must never call the destructor more than once.
// Doing so may trigger memory unsafety.
//
// If we try to implement this with mutual exclusion on access to the pointer, there is the
// If we try to implement this with mutual exclusion on access to the handle, there is the
// possibility of a race between a method call and a concurrent call to `destroy`:
//
// * Thread A starts a method call, reads the value of the pointer, but is interrupted
// before it can pass the pointer over the FFI to Rust.
// * Thread A starts a method call, reads the value of the handle, but is interrupted
// before it can pass the handle over the FFI to Rust.
// * Thread B calls `destroy` and frees the underlying Rust struct.
// * Thread A resumes, passing the already-read pointer value to Rust and triggering
// * Thread A resumes, passing the already-read handle value to Rust and triggering
// a use-after-free.
//
// One possible solution would be to use a `ReadWriteLock`, with each method call taking
Expand Down Expand Up @@ -83,24 +83,24 @@
//
abstract class FFIObject: Disposable, AutoCloseable {

constructor(pointer: Pointer) {
this.pointer = pointer
constructor(handle: UniffiHandle) {
this.handle = handle
}

/**
* This constructor can be used to instantiate a fake object.
*
* **WARNING: Any object instantiated with this constructor cannot be passed to an actual Rust-backed object.**
* Since there isn't a backing [Pointer] the FFI lower functions will crash.
* @param noPointer Placeholder value so we can have a constructor separate from the default empty one that may be
* Since there isn't a backing [UniffiHandle] the FFI lower functions will crash.
* @param NoHandle Placeholder value so we can have a constructor separate from the default empty one that may be
* implemented for classes extending [FFIObject].
*/
@Suppress("UNUSED_PARAMETER")
constructor(noPointer: NoPointer) {
this.pointer = null
constructor(noHandle: NoHandle) {
this.handle = null
}

protected val pointer: Pointer?
protected val handle: UniffiHandle?

private val wasDestroyed = AtomicBoolean(false)
private val callCounter = AtomicLong(1)
Expand All @@ -125,7 +125,7 @@ abstract class FFIObject: Disposable, AutoCloseable {
this.destroy()
}

internal inline fun <R> callWithPointer(block: (ptr: Pointer) -> R): R {
internal inline fun <R> callWithHandle(block: (handle: UniffiHandle) -> R): R {
// Check and increment the call counter, to keep the object alive.
// This needs a compare-and-set retry loop in case of concurrent updates.
do {
Expand All @@ -137,9 +137,9 @@ abstract class FFIObject: Disposable, AutoCloseable {
throw IllegalStateException("${this.javaClass.simpleName} call counter would overflow")
}
} while (! this.callCounter.compareAndSet(c, c + 1L))
// Now we can safely do the method call without the pointer being freed concurrently.
// Now we can safely do the method call without the handle being freed concurrently.
try {
return block(this.pointer!!)
return block(this.handle!!)
} finally {
// This decrement always matches the increment we performed above.
if (this.callCounter.decrementAndGet() == 0L) {
Expand All @@ -150,4 +150,4 @@ abstract class FFIObject: Disposable, AutoCloseable {
}

/** Used to instantiate a [FFIObject] without an actual pointer, for fakes in tests, mostly. */
object NoPointer
object NoHandle
46 changes: 19 additions & 27 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@
{%- call kt::docstring(obj, 0) %}
open class {{ impl_class_name }} : FFIObject, {{ interface_name }} {

constructor(pointer: Pointer): super(pointer)
constructor(handle: UniffiHandle): super(handle)

/**
* This constructor can be used to instantiate a fake object.
*
* **WARNING: Any object instantiated with this constructor cannot be passed to an actual Rust-backed object.**
* Since there isn't a backing [Pointer] the FFI lower functions will crash.
* @param noPointer Placeholder value so we can have a constructor separate from the default empty one that may be
* Since there isn't a backing [UniffiHandle] the FFI lower functions will crash.
* @param noHandle Placeholder value so we can have a constructor separate from the default empty one that may be
* implemented for classes extending [FFIObject].
*/
constructor(noPointer: NoPointer): super(noPointer)
constructor(noHandle: NoHandle): super(noHandle)

{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}
Expand All @@ -38,9 +38,9 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} {
* Clients **must** call this method once done with the object, or cause a memory leak.
*/
override protected fun freeRustArcPtr() {
this.pointer?.let { ptr ->
this.handle?.let { handle ->
rustCall() { status ->
_UniFFILib.INSTANCE.{{ obj.ffi_object_free().name() }}(ptr, status)
_UniFFILib.INSTANCE.{{ obj.ffi_object_free().name() }}(handle, status)
}
}
}
Expand All @@ -58,9 +58,9 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} {
{%- call kt::arg_list_decl(meth) -%}
){% match meth.return_type() %}{% when Some with (return_type) %} : {{ return_type|type_name(ci) }}{% when None %}{%- endmatch %} {
return uniffiRustCallAsync(
callWithPointer { thisPtr ->
callWithHandle { uniffiHandle ->
_UniFFILib.INSTANCE.{{ meth.ffi_func().name() }}(
thisPtr,
uniffiHandle,
{% call kt::arg_list_lowered(meth) %}
)
},
Expand All @@ -86,20 +86,16 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} {
{%- else -%}
{%- match meth.return_type() -%}
{%- when Some with (return_type) -%}
override fun {{ meth.name()|fn_name }}(
{%- call kt::arg_list_protocol(meth) -%}
): {{ return_type|type_name(ci) }} =
callWithPointer {
override fun {{ meth.name()|fn_name }}({% call kt::arg_list_protocol(meth) %}): {{ return_type|type_name(ci) }} =
callWithHandle {
{%- call kt::to_ffi_call_with_prefix("it", meth) %}
}.let {
{{ return_type|lift_fn }}(it)
}

{%- when None -%}
override fun {{ meth.name()|fn_name }}(
{%- call kt::arg_list_protocol(meth) -%}
) =
callWithPointer {
override fun {{ meth.name()|fn_name }}({% call kt::arg_list_protocol(meth) %}) =
callWithHandle {
{%- call kt::to_ffi_call_with_prefix("it", meth) %}
}
{% endmatch %}
Expand Down Expand Up @@ -157,35 +153,31 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} {
{% include "CallbackInterfaceImpl.kt" %}
{%- endif %}

public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, Pointer> {
public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, UniffiHandle> {
{%- if obj.is_trait_interface() %}
internal val handleMap = ConcurrentHandleMap<{{ interface_name }}>()
{%- endif %}

override fun lower(value: {{ type_name }}): Pointer {
override fun lower(value: {{ type_name }}): UniffiHandle {
{%- match obj.imp() %}
{%- when ObjectImpl::Struct %}
return value.callWithPointer { it }
return value.handle
{%- when ObjectImpl::Trait %}
return Pointer(handleMap.insert(value))
return UniffiHandle(handleMap.insert(value))
{%- endmatch %}
}

override fun lift(value: Pointer): {{ type_name }} {
override fun lift(value: UniffiHandle): {{ type_name }} {
return {{ impl_class_name }}(value)
}

override fun read(buf: ByteBuffer): {{ type_name }} {
// The Rust code always writes pointers as 8 bytes, and will
// fail to compile if they don't fit.
return lift(Pointer(buf.getLong()))
return lift(buf.getLong())
}

override fun allocationSize(value: {{ type_name }}) = 8

override fun write(value: {{ type_name }}, buf: ByteBuffer) {
// The Rust code always expects pointers written as 8 bytes,
// and will fail to compile if they don't fit.
buf.putLong(Pointer.nativeValue(lower(value)))
buf.putLong(lower(value))
}
}
3 changes: 1 addition & 2 deletions uniffi_bindgen/src/bindings/python/gen_python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ impl PythonCodeOracle {
FfiType::UInt64 => "ctypes.c_uint64".to_string(),
FfiType::Float32 => "ctypes.c_float".to_string(),
FfiType::Float64 => "ctypes.c_double".to_string(),
FfiType::RustArcPtr(_) => "ctypes.c_void_p".to_string(),
FfiType::Handle => "ctypes.c_uint64".to_string(),
FfiType::RustBuffer(maybe_suffix) => match maybe_suffix {
Some(suffix) => format!("_UniffiRustBuffer{suffix}"),
None => "_UniffiRustBuffer".to_string(),
Expand All @@ -360,7 +360,6 @@ impl PythonCodeOracle {
// Pointer to an `asyncio.EventLoop` instance
FfiType::ForeignExecutorHandle => "ctypes.c_size_t".to_string(),
FfiType::ForeignExecutorCallback => "_UNIFFI_FOREIGN_EXECUTOR_CALLBACK_T".to_string(),
FfiType::RustFutureHandle => "ctypes.c_void_p".to_string(),
FfiType::RustFutureContinuationCallback => "_UNIFFI_FUTURE_CONTINUATION_T".to_string(),
FfiType::RustFutureContinuationData => "ctypes.c_size_t".to_string(),
}
Expand Down
29 changes: 13 additions & 16 deletions uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,30 @@

class {{ impl_name }}:
{%- call py::docstring(obj, 4) %}

_pointer: ctypes.c_void_p
_uniffi_handle: ctypes.c_int64

{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}
def __init__(self, {% call py::arg_list_decl(cons) -%}):
{%- call py::docstring(cons, 8) %}
{%- call py::setup_args_extra_indent(cons) %}
self._pointer = {% call py::to_ffi_call(cons) %}
self._uniffi_handle = {% call py::to_ffi_call(cons) %}
{%- when None %}
{%- endmatch %}

def __del__(self):
# In case of partial initialization of instances.
pointer = getattr(self, "_pointer", None)
if pointer is not None:
_rust_call(_UniffiLib.{{ obj.ffi_object_free().name() }}, pointer)
handle = getattr(self, "_uniffi_handle", None)
if handle is not None:
_rust_call(_UniffiLib.{{ obj.ffi_object_free().name() }}, handle)

# Used by alternative constructors or any methods which return this type.
@classmethod
def _make_instance_(cls, pointer):
def _make_instance_(cls, handle):
# Lightly yucky way to bypass the usual __init__ logic
# and just create a new instance with the required pointer.
# and just create a new instance with the required handle.
inst = cls.__new__(cls)
inst._pointer = pointer
inst._uniffi_handle = handle
return inst

{%- for cons in obj.alternate_constructors() %}
Expand All @@ -41,8 +40,8 @@ def {{ cons.name()|fn_name }}(cls, {% call py::arg_list_decl(cons) %}):
{%- call py::docstring(cons, 8) %}
{%- call py::setup_args_extra_indent(cons) %}
# Call the (fallible) function before creating any half-baked object instances.
pointer = {% call py::to_ffi_call(cons) %}
return cls._make_instance_(pointer)
uniffi_handle = {% call py::to_ffi_call(cons) %}
return cls._make_instance_(uniffi_handle)
{% endfor %}

{%- for meth in obj.methods() -%}
Expand All @@ -60,13 +59,13 @@ def __eq__(self, other: object) -> {{ eq.return_type().unwrap()|type_name }}:
if not isinstance(other, {{ type_name }}):
return NotImplemented

return {{ eq.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._pointer", eq) %})
return {{ eq.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._uniffi_handle", eq) %})

def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}:
if not isinstance(other, {{ type_name }}):
return NotImplemented

return {{ ne.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._pointer", ne) %})
return {{ ne.return_type().unwrap()|lift_fn }}({% call py::to_ffi_call_with_prefix("self._uniffi_handle", ne) %})
{%- when UniffiTrait::Hash { hash } %}
{%- call py::method_decl("__hash__", hash) %}
{% endmatch %}
Expand Down Expand Up @@ -94,16 +93,14 @@ def lower(value: {{ protocol_name }}):
{%- when ObjectImpl::Struct %}
if not isinstance(value, {{ impl_name }}):
raise TypeError("Expected {{ impl_name }} instance, {} found".format(type(value).__name__))
return value._pointer
return value._uniffi_handle
{%- when ObjectImpl::Trait %}
return {{ ffi_converter_name }}._handle_map.insert(value)
{%- endmatch %}

@classmethod
def read(cls, buf: _UniffiRustBuffer):
ptr = buf.read_u64()
if ptr == 0:
raise InternalError("Raw pointer value was null")
return cls.lift(ptr)

@classmethod
Expand Down
6 changes: 3 additions & 3 deletions uniffi_bindgen/src/bindings/python/templates/macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}):
{%- call setup_args_extra_indent(meth) %}
return _uniffi_rust_call_async(
_UniffiLib.{{ meth.ffi_func().name() }}(
self._pointer, {% call arg_list_lowered(meth) %}
self._uniffi_handle, {% call arg_list_lowered(meth) %}
),
_UniffiLib.{{ meth.ffi_rust_future_poll(ci) }},
_UniffiLib.{{ meth.ffi_rust_future_complete(ci) }},
Expand Down Expand Up @@ -148,15 +148,15 @@ def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}) -> "{{ return_typ
{%- call docstring(meth, 8) %}
{%- call setup_args_extra_indent(meth) %}
return {{ return_type|lift_fn }}(
{% call to_ffi_call_with_prefix("self._pointer", meth) %}
{% call to_ffi_call_with_prefix("self._uniffi_handle", meth) %}
)

{%- when None %}

def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}):
{%- call docstring(meth, 8) %}
{%- call setup_args_extra_indent(meth) %}
{% call to_ffi_call_with_prefix("self._pointer", meth) %}
{% call to_ffi_call_with_prefix("self._uniffi_handle", meth) %}
{% endmatch %}
{% endif %}

Expand Down
Loading

0 comments on commit b06a1f8

Please sign in to comment.