Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow calling from more than one language #2

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,4 @@ impl CodeType for CallbackInterfaceCodeType {
fn canonical_name(&self) -> String {
format!("Type{}", self.id)
}

fn initialization_fn(&self) -> Option<String> {
Some(format!("uniffiCallbackInterface{}.register", self.id))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ internal object {{ trait_impl }} {

// Registers the foreign callback with the Rust side.
// This method is generated for each callback interface.
internal fun register(lib: UniffiLib) {
internal fun register(lib: UniffiLib): Int =
lib.{{ ffi_init_callback.name() }}(vtable)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,26 @@ internal const val UNIFFI_CALLBACK_UNEXPECTED_ERROR = 2
/**
* @suppress
*/
public abstract class FfiConverterCallbackInterface<CallbackInterface: Any>: FfiConverter<CallbackInterface, Long> {
public abstract class FfiConverterCallbackInterface<CallbackInterface: Any>(
internal val langIndex: Int
): FfiConverterRustBuffer<CallbackInterface> {
internal val handleMap = UniffiHandleMap<CallbackInterface>()

internal fun drop(handle: Long) {
handleMap.remove(handle)
}

override fun lift(value: Long): CallbackInterface {
return handleMap.get(value)
override fun read(buf: ByteBuffer): CallbackInterface {
val handle = buf.getLong()
assert(buf.getInt() == this.langIndex) { "Callback interface has been called in the wrong language" }
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't true in all cases: we should be able to call from one language to another via Rust.

Copy link

@bendk bendk Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think there's 2 options here:

  • Make it so the Rust scaffolding functions work with foreign handles as well. If Kotlin calls the scaffolding function with a handle from React-native, then Rust will forward the call using the React-native vtable
  • Make everything go through vtables.
    • Don't export the Rust scaffolding functions directly for a trait interface.
    • Instead, export a scaffolding function that inputs a language index and returns the vtable for that language.
    • Foreign code uses that to get a vtable whenever they see a handle with a language index they don't recognize. Once it has a vtable, it uses that to make the call. This works the exactly same for Rust and other foreign languages.

The second option seems better for performance and I kind of like not special-casing Rust. I might be a pain to implement though.

return handleMap.get(handle)
}

override fun read(buf: ByteBuffer) = lift(buf.getLong())

override fun lower(value: CallbackInterface) = handleMap.insert(value)

override fun allocationSize(value: CallbackInterface) = 8UL
override fun allocationSize(value: CallbackInterface) = 12UL

override fun write(value: CallbackInterface, buf: ByteBuffer) {
buf.putLong(lower(value))
val handle = handleMap.insert(value)
buf.putLong(handle)
buf.putInt(langIndex)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
{% include "Interface.kt" %}
{% include "CallbackInterfaceImpl.kt" %}

{%- let trait_impl=format!("uniffiCallbackInterface{}", name) %}
/**
* The ffiConverter which transforms the Callbacks in to handles to pass to Rust.
*
* @suppress
*/
public object {{ ffi_converter_name }}: FfiConverterCallbackInterface<{{ interface_name }}>()
public object {{ ffi_converter_name }}: FfiConverterCallbackInterface<{{ interface_name }}>(
langIndex = {{ trait_impl }}.register(UniffiLib.INSTANCE)
)
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,10 @@ public interface FfiConverter<KotlinType, FfiType> {
// FfiType. It's used by the callback interface code. Callback interface
// returns are always serialized into a `RustBuffer` regardless of their
// normal FFI type.
fun lowerIntoRustBuffer(value: KotlinType): RustBuffer.ByValue {
val rbuf = RustBuffer.alloc(allocationSize(value))
try {
val bbuf = rbuf.data!!.getByteBuffer(0, rbuf.capacity).also {
it.order(ByteOrder.BIG_ENDIAN)
}
fun lowerIntoRustBuffer(value: KotlinType): RustBuffer.ByValue =
RustBuffer.write(allocationSize(value)) { bbuf ->
write(value, bbuf)
rbuf.writeField("len", bbuf.position().toLong())
return rbuf
} catch (e: Throwable) {
RustBuffer.free(rbuf)
throw e
}
}

// Lift a value from a `RustBuffer`.
//
Expand Down
2 changes: 2 additions & 0 deletions uniffi_bindgen/src/bindings/kotlin/templates/HandleMap.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ internal class UniffiHandleMap<T: Any> {
fun remove(handle: Long): T {
return map.remove(handle) ?: throw InternalException("UniffiHandleMap: Invalid handle")
}

fun has(handle: Long): Boolean = map.contains(handle)
}
64 changes: 57 additions & 7 deletions uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ open class {{ impl_class_name }}: Disposable, AutoCloseable, {{ interface_name }
this.cleanable = UniffiLib.CLEANER.register(this, UniffiCleanAction(pointer))
}

constructor(rbuf: RustBuffer) : this(
rbuf.asByteBuffer()!!.let { buf ->
val pointer = buf.getLong()
buf.getInt()
Pointer(pointer)
}
)

/**
* This constructor can be used to instantiate a fake object. Only used for tests. Any
* attempt to actually use an object constructed this way will fail as there is no
Expand Down Expand Up @@ -171,7 +179,11 @@ open class {{ impl_class_name }}: Disposable, AutoCloseable, {{ interface_name }
this.destroy()
}

{% if obj.has_callback_interface() -%}
internal inline fun <R> callWithPointer(block: (ptr: RustBuffer.ByValue) -> R): R {
{%- else %}
internal inline fun <R> callWithPointer(block: (ptr: Pointer) -> R): R {
{%- endif %}
// 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 @@ -185,7 +197,11 @@ open class {{ impl_class_name }}: Disposable, AutoCloseable, {{ interface_name }
} while (! this.callCounter.compareAndSet(c, c + 1L))
// Now we can safely do the method call without the pointer being freed concurrently.
try {
{% if obj.has_callback_interface() -%}
return block({{ ffi_converter_name }}.lowerPointer(this.uniffiClonePointer()))
{%- else %}
return block(this.uniffiClonePointer())
{%- endif %}
} finally {
// This decrement always matches the increment we performed above.
if (this.callCounter.decrementAndGet() == 0L) {
Expand Down Expand Up @@ -267,20 +283,52 @@ open class {{ impl_class_name }}: Disposable, AutoCloseable, {{ interface_name }
{% include "CallbackInterfaceImpl.kt" %}
{%- endif %}

{%- let trait_impl=format!("uniffiCallbackInterface{}", name) %}

{%- if obj.has_callback_interface() %}
/**
* @suppress
*/
public object {{ ffi_converter_name }}: FfiConverter<{{ type_name }}, Pointer> {
{%- if obj.has_callback_interface() %}
public object {{ ffi_converter_name }}: FfiConverterRustBuffer<{{ type_name }}> {
internal val handleMap = UniffiHandleMap<{{ type_name }}>()
{%- endif %}
internal val langIndex: Int = {{ trait_impl }}.register(UniffiLib.INSTANCE)

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.
val handle = buf.getLong()
val langIndex = buf.getInt()
if (handleMap.has(handle)) {
return handleMap.get(handle)
} else {
return {{ impl_class_name }}(Pointer(handle))
}
Comment on lines +301 to +305
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this right yet: we should be able to detect here if the trait was implemented in our language, Rust or another foreign language.

}

override fun allocationSize(value: {{ type_name }}) = 12UL

fun lowerPointer(pointer: Pointer): RustBuffer.ByValue =
RustBuffer.write(12UL) { bbuf ->
bbuf.putLong(Pointer.nativeValue(pointer))
bbuf.putInt(langIndex)
Copy link

@bendk bendk Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we have to handle it with the current handle system. I'm hoping to implement something like mozilla#1823, which would allow us to pack both the pointer and the langIndex into a 64-bit handle.

This means creating some sort of tagged pointer. The simplest version of that would be using the lower bits for the langIndex. We know that Arc stores a word ref count, so on a 32-bit system it's aligned to at least 4 bytes. That gives us 2 bits to use, which could support 3 foreign languages plus Rust, do you think that would be enough?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 languages at the same time sounds like it should be enough.

Just for the sake of not boxing ourselves into a corner: I should imagine if we want more than 3 then we'd need to move to 64bits for everything, which also sounds do-able, if absolutely necessary.

}

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.
val handle = handleMap.insert(value)
buf.putLong(handle)
buf.putInt(this.langIndex)
}
}

{%- else %}
/**
* @suppress
*/
public object {{ ffi_converter_name }}: FfiConverter<{{ type_name }}, Pointer> {
override fun lower(value: {{ type_name }}): Pointer {
{%- if obj.has_callback_interface() %}
return Pointer(handleMap.insert(value))
{%- else %}
return value.uniffiClonePointer()
{%- endif %}
}

override fun lift(value: Pointer): {{ type_name }} {
Expand All @@ -301,3 +349,5 @@ public object {{ ffi_converter_name }}: FfiConverter<{{ type_name }}, Pointer> {
buf.putLong(Pointer.nativeValue(lower(value)))
}
}

{%- endif %}
15 changes: 15 additions & 0 deletions uniffi_bindgen/src/bindings/kotlin/templates/RustBufferTemplate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ open class RustBuffer : Structure() {
internal fun free(buf: RustBuffer.ByValue) = uniffiRustCall() { status ->
UniffiLib.INSTANCE.{{ ci.ffi_rustbuffer_free().name() }}(buf, status)
}

fun write(size: ULong, writer: (ByteBuffer) -> Unit): RustBuffer.ByValue {
val rbuf = RustBuffer.alloc(size)
try {
val bbuf = rbuf.data!!.getByteBuffer(0, rbuf.capacity).also {
it.order(ByteOrder.BIG_ENDIAN)
}
writer(bbuf)
rbuf.writeField("len", bbuf.position().toLong())
return rbuf
} catch (e: Throwable) {
RustBuffer.free(rbuf)
throw e
}
}
}

@Suppress("TooGenericExceptionThrown")
Expand Down
5 changes: 3 additions & 2 deletions uniffi_bindgen/src/interface/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,11 @@ 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 { imp, .. } if imp.has_callback_interface() => FfiType::RustBuffer(None),
// Objects are pointers to an Arc<>
Type::Object { name, .. } => FfiType::RustArcPtr(name.to_owned()),
// Callback interfaces are passed as opaque integer handles.
Type::CallbackInterface { .. } => FfiType::UInt64,
Type::CallbackInterface { .. } => FfiType::RustBuffer(None),
// Other types are serialized into a bytebuffer and deserialized on the other side.
Type::Enum { .. }
| Type::Record { .. }
Expand Down Expand Up @@ -221,7 +222,7 @@ impl FfiFunction {
name: "vtable".to_string(),
type_: FfiType::Struct(vtable_name).reference(),
}],
return_type: None,
return_type: Some(FfiType::Int32),
has_rust_call_status_arg: false,
..Self::default()
}
Expand Down
33 changes: 21 additions & 12 deletions uniffi_core/src/ffi/foreigncallbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,38 @@
//! code loads the exported library. For each callback type, we also define a "cell" type for
//! storing the callback.

use std::{
ptr::{null_mut, NonNull},
sync::atomic::{AtomicPtr, Ordering},
};
use std::{ptr::NonNull, sync::Mutex};

// Cell type that stores any NonNull<T>
#[doc(hidden)]
pub struct UniffiForeignPointerCell<T>(AtomicPtr<T>);
pub struct UniffiForeignPointerCell<T> {
pointers: Mutex<Vec<*mut T>>,
}
Comment on lines +15 to +17
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure this could be a better name. Is it still a Cell given it's holding multiple vtables?

Would this also be better as a Cow<T>?


impl<T> UniffiForeignPointerCell<T> {
pub const fn new() -> Self {
Self(AtomicPtr::new(null_mut()))
Self {
pointers: Mutex::new(Vec::new()),
}
}

pub fn set(&self, callback: NonNull<T>) {
self.0.store(callback.as_ptr(), Ordering::Relaxed);
pub fn set(&self, callback: NonNull<T>) -> usize {
let mut pointers = self.pointers.lock().unwrap();
let index = pointers.len();
pointers.push(callback.as_ptr());
index
}

pub fn get(&self) -> &T {
pub fn get(&self, index: usize) -> &T {
let pointers = self.pointers.lock().unwrap();
if index >= pointers.len() {
panic!("Foreign pointer used before being set. This is likely a uniffi bug.")
}
unsafe {
NonNull::new(self.0.load(Ordering::Relaxed))
.expect("Foreign pointer not set. This is likely a uniffi bug.")
.as_mut()
let ptr = pointers[index];
NonNull::new(ptr)
.expect("Foreign pointer not set. This is likely a uniffi bug.")
.as_ref()
}
}
}
Expand Down
34 changes: 18 additions & 16 deletions uniffi_macros/src/export/callback_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,19 @@ pub(super) fn trait_impl(
static #vtable_cell: ::uniffi::UniffiForeignPointerCell::<#vtable_type> = ::uniffi::UniffiForeignPointerCell::<#vtable_type>::new();

#[no_mangle]
extern "C" fn #init_ident(vtable: ::std::ptr::NonNull<#vtable_type>) {
#vtable_cell.set(vtable);
extern "C" fn #init_ident(vtable: ::std::ptr::NonNull<#vtable_type>) -> i32 {
#vtable_cell.set(vtable) as i32
}

#[derive(Debug)]
struct #trait_impl_ident {
handle: u64,
lang_index: usize,
}

impl #trait_impl_ident {
fn new(handle: u64) -> Self {
Self { handle }
fn new(handle: u64, lang_index: usize) -> Self {
Self { handle, lang_index }
}
}

Expand All @@ -112,7 +113,7 @@ pub(super) fn trait_impl(

impl ::std::ops::Drop for #trait_impl_ident {
fn drop(&mut self) {
let vtable = #vtable_cell.get();
let vtable = #vtable_cell.get(self.lang_index);
(vtable.uniffi_free)(self.handle);
}
}
Expand Down Expand Up @@ -141,22 +142,23 @@ pub fn ffi_converter_callback_interface_impl(
Ok(p) => p,
Err(e) => return e.into_compile_error(),
};
let try_lift_self = ffiops::try_lift(quote! { Self });

let try_lift = ffiops::try_lift_from_rust_buffer(quote! { Self });
quote! {
#[doc(hidden)]
#[automatically_derived]
unsafe #lift_impl_spec {
type FfiType = u64;

fn try_lift(v: Self::FfiType) -> ::uniffi::deps::anyhow::Result<Self> {
::std::result::Result::Ok(::std::boxed::Box::new(<#trait_impl_ident>::new(v)))
}
type FfiType = ::uniffi::rustbuffer::RustBuffer;

fn try_read(buf: &mut &[u8]) -> ::uniffi::deps::anyhow::Result<Self> {
use ::uniffi::deps::bytes::Buf;
::uniffi::check_remaining(buf, 8)?;
#try_lift_self(buf.get_u64())
::uniffi::check_remaining(buf, 12)?;
let handle = buf.get_u64();
let lang_index = ::std::convert::TryFrom::try_from(buf.get_i32())?;
::std::result::Result::Ok(::std::boxed::Box::new(<#trait_impl_ident>::new(handle, lang_index)))
}

fn try_lift(buf: Self::FfiType) -> ::uniffi::deps::anyhow::Result<Self> {
#try_lift(buf)
}
}

Expand Down Expand Up @@ -221,7 +223,7 @@ fn gen_method_impl(sig: &FnSignature, vtable_cell: &Ident) -> syn::Result<TokenS
if !is_async {
Ok(quote! {
fn #ident(#self_param, #(#params),*) -> #return_ty {
let vtable = #vtable_cell.get();
let vtable = #vtable_cell.get(self.lang_index);
let mut uniffi_call_status: ::uniffi::RustCallStatus = ::std::default::Default::default();
let mut uniffi_return_value: #lift_return_type = ::uniffi::FfiDefault::ffi_default();
(vtable.#ident)(self.handle, #(#lower_exprs,)* &mut uniffi_return_value, &mut uniffi_call_status);
Expand All @@ -231,7 +233,7 @@ fn gen_method_impl(sig: &FnSignature, vtable_cell: &Ident) -> syn::Result<TokenS
} else {
Ok(quote! {
async fn #ident(#self_param, #(#params),*) -> #return_ty {
let vtable = #vtable_cell.get();
let vtable = #vtable_cell.get(self.lang_index);
::uniffi::foreign_async_call::<_, #return_ty, crate::UniFfiTag>(move |uniffi_future_callback, uniffi_future_callback_data| {
let mut uniffi_foreign_future: ::uniffi::ForeignFuture = ::uniffi::FfiDefault::ffi_default();
(vtable.#ident)(self.handle, #(#lower_exprs,)* uniffi_future_callback, uniffi_future_callback_data, &mut uniffi_foreign_future);
Expand Down
Loading