Skip to content

Commit

Permalink
Use Handle for passing Rust objects across the FFI
Browse files Browse the repository at this point in the history
* Added the `SlabAlloc` FFI trait.  This is used to manage handles for
  `Arc<T>` instances, including `Arc<dyn Trait>`
* Also use handles for trait interfaces.  This still needs some
  updates on the foreign side before it's working.
* Renamed a bunch of stuff and replaced a lot of comment text
* Added the `const_random` crate to generate random slab IDs.  This
  should be a pretty lightweight dpeendency.
* Bumped `CONTRACT_VERSION` since this is a change to the FFI
  • Loading branch information
bendk committed Oct 26, 2023
1 parent d7f25b9 commit 9d62c74
Show file tree
Hide file tree
Showing 32 changed files with 356 additions and 241 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@

[All changes in [[UnreleasedUniFFIVersion]]](https://github.com/mozilla/uniffi-rs/compare/v0.25.0...HEAD).

### What's changed?

- The object handle FFI has changed. External bindings generators will need to update their code to
use the new slab/handle system.

## v0.25.0 (backend crates: v0.25.0) - (_2023-10-18_)

[All changes in v0.25.0](https://github.com/mozilla/uniffi-rs/compare/v0.24.3...v0.25.0).
Expand Down
44 changes: 44 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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 @@ -298,15 +298,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 @@ -17,10 +17,10 @@ internal object uniffiRustFutureContinuationCallback: UniFffiRustFutureContinuat
}

internal suspend fun<T, F, E: Exception> uniffiRustCallAsync(
rustFuture: Pointer,
pollFunc: (Pointer, USize) -> Unit,
completeFunc: (Pointer, RustCallStatus) -> F,
freeFunc: (Pointer) -> Unit,
rustFuture: UniffiHandle,
pollFunc: (UniffiHandle, USize) -> Unit,
completeFunc: (UniffiHandle, RustCallStatus) -> F,
freeFunc: (UniffiHandle) -> Unit,
liftFunc: (F) -> T,
errorHandler: CallStatusErrorHandler<E>
): T {
Expand Down
2 changes: 2 additions & 0 deletions uniffi_bindgen/src/bindings/kotlin/templates/Helpers.kt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
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
26 changes: 13 additions & 13 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectRuntime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ inline fun <T : Disposable?, R> T.use(block: (T) -> R) =

// 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 @@ -60,13 +60,13 @@ inline fun <T : Disposable?, R> T.use(block: (T) -> R) =
// 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 @@ -112,7 +112,7 @@ inline fun <T : Disposable?, R> T.use(block: (T) -> R) =
// [1] https://stackoverflow.com/questions/24376768/can-java-finalize-an-object-when-it-is-still-in-scope/24380219
//
abstract class FFIObject(
protected val pointer: Pointer
internal val handle: UniffiHandle
): Disposable, AutoCloseable {

private val wasDestroyed = AtomicBoolean(false)
Expand All @@ -138,7 +138,7 @@ abstract class FFIObject(
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 @@ -150,9 +150,9 @@ abstract class FFIObject(
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 Down
32 changes: 14 additions & 18 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
{% include "Interface.kt" %}

class {{ impl_class_name }}(
pointer: Pointer
) : FFIObject(pointer), {{ interface_name }}{
handle: UniffiHandle
) : FFIObject(handle), {{ interface_name }}{

{%- match obj.primary_constructor() %}
{%- when Some with (cons) %}
Expand All @@ -26,7 +26,7 @@ class {{ impl_class_name }}(
*/
override protected fun freeRustArcPtr() {
rustCall() { status ->
_UniFFILib.INSTANCE.{{ obj.ffi_object_free().name() }}(this.pointer, status)
_UniFFILib.INSTANCE.{{ obj.ffi_object_free().name() }}(this.handle, status)
}
}

Expand All @@ -40,9 +40,9 @@ class {{ impl_class_name }}(
@Suppress("ASSIGNED_BUT_NEVER_ACCESSED_VARIABLE")
override suspend fun {{ meth.name()|fn_name }}({%- call kt::arg_list_decl(meth) -%}){% match meth.return_type() %}{% when Some with (return_type) %} : {{ return_type|type_name }}{% when None %}{%- endmatch %} {
return uniffiRustCallAsync(
callWithPointer { thisPtr ->
callWithHandle { uniffiHandle ->
_UniFFILib.INSTANCE.{{ meth.ffi_func().name() }}(
thisPtr,
uniffiHandle,
{% call kt::arg_list_lowered(meth) %}
)
},
Expand All @@ -69,15 +69,15 @@ class {{ impl_class_name }}(
{%- 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 }} =
callWithPointer {
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 {
callWithHandle {
{%- call kt::to_ffi_call_with_prefix("it", meth) %}
}
{% endmatch %}
Expand All @@ -103,35 +103,31 @@ class {{ impl_class_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 @@ -311,7 +311,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_int64".to_string(),
FfiType::RustBuffer(maybe_suffix) => match maybe_suffix {
Some(suffix) => format!("_UniffiRustBuffer{suffix}"),
None => "_UniffiRustBuffer".to_string(),
Expand All @@ -321,7 +321,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
28 changes: 13 additions & 15 deletions uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,29 @@
{% include "Protocol.py" %}

class {{ impl_name }}:
_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::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 @@ -36,8 +36,8 @@ def _make_instance_(cls, pointer):
def {{ cons.name()|fn_name }}(cls, {% call py::arg_list_decl(cons) %}):
{%- 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 @@ -55,13 +55,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 @@ -89,16 +89,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
Loading

0 comments on commit 9d62c74

Please sign in to comment.