Foreign-implemented trait interfaces

This adds support for the foreign bindings side to implement Rust traits
and for both sides to pass `Arc<dyn [Trait]>` objects back and forth.

I thought this would be a good time to redo the callback interface code.
The trait interface code uses a foreign-supplied vtable rather than a
single callback and method indexes like the old code used.

Renamed `CALL_PANIC` to `CALL_UNEXPECTED_ERROR` in the foreign bindings
templates.  This matches the name in the Rust code and makes more sense
for foreign trait implementations.

-------------------- TODO -----------------------------

This currently requires "wrapping" the object every time it's passed
across the FFI.  If the foreign code receives a trait object, then
passes it back to Rust.  Rust now has a handle to the foreign impl and
that foreign impl just calls back into Rust.  I think mozilla#1730 could help
solve this.

I think there should be better tests for reference counts, but I'm
waiting until we address the previous issue to implement them.

Python requires a hack to return RustBuffer from callbacks
(RubiconCtypesPatch.py) because of an old ctypes bug
(https://bugs.python.org/issue5710).  The options I can see are:
  - Go back to the callback interface style, where we input a RustBuffer
    and write the output to it.
  - Pass the return type as an out pointer.  This requires some special
    handling for void returns though.
  - Implement mozilla#1779, which changes RustBuffer to a simple `*mut u8` and
    then Python could return it.  This is my preference, I don't like
    the idea of making the ABI worse just because of Python.

TODO: If I make the Python tests fail, I see `RuntimeWarning: memory
leak in callback function` in the console.  I believe this is because of
the rubicon hack, but I'm not sure.  We should double check this before
merging.

Kotlin is not implemented yet.  I think when we do implement it, we're
going to want to face mozilla#1394 and make a decision there.  I don't think
it's reasonable to have consumers have to call a `close()` method on
these interfaces.  I also don't know how we would deal with this in
Rust.  ISTM that the same logic that says Kotlin objects need to be
manually closed dictates that if you use a Kotlin object in Rust, there
should be a `close()` method or something analogous to a to
`AutoCloseable`.

The Ruby coverall tests are now failing because they don't implement
trait interfaces.  We need to figure out a way for them to pass the test
suite.

Remove all the debugging printouts
Document this

fixtures/coverall/src/lib.rs

@@ -20,6 +20,12 @@ pub enum CoverallError {
  TooManyHoles,
 }
 
+impl From<uniffi::UnexpectedUniFFICallbackError> for CoverallError {
+ fn from(_e: uniffi::UnexpectedUniFFICallbackError) -> Self {
+ panic!("Saw UnexpectedUniFFICallbackError when a CoverallError was expected")
+ }
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum CoverallFlatError {
  #[error("Too many variants: {num}")]
@@ -90,6 +96,12 @@ pub enum ComplexError {
  UnknownError,
 }
 
+impl From<uniffi::UnexpectedUniFFICallbackError> for ComplexError {
+ fn from(_e: uniffi::UnexpectedUniFFICallbackError) -> Self {
+ Self::UnknownError
+ }
+}
+
 #[derive(Debug, thiserror::Error, uniffi::Error)]
 pub enum ComplexMacroError {
  #[error("OsError: {code} ({extended_code})")]

fixtures/coverall/tests/bindings/test_coverall.py

@@ -278,11 +278,69 @@ def test_bytes(self):
  coveralls = Coveralls("test_bytes")
  self.assertEqual(coveralls.reverse(b"123"), b"321")
 
+class PyGetters:
+ def get_bool(self, v, arg2):
+ print(f"get_bool {v} {arg2}", v ^ arg2)
+ return v ^ arg2
+
+ def get_string(self, v, arg2):
+ if v == "too-many-holes":
+ raise CoverallError.TooManyHoles
+ elif v == "unexpected-error":
+ raise RuntimeError("unexpected error")
+ elif arg2:
+ return v.upper()
+ else:
+ return v
+
+ def get_option(self, v, arg2):
+ if v == "os-error":
+ raise ComplexError.OsError(100, 200)
+ elif v == "unknown-error":
+ raise ComplexError.UnknownError
+ elif arg2:
+ if v:
+ return v.upper()
+ else:
+ return None
+ else:
+ return v
+
+ def get_list(self, v, arg2):
+ if arg2:
+ return v
+ else:
+ return []
+
+ def get_nothing(self, _v):
+ return None
+
+class PyNode:
+ def __init__(self):
+ self.parent = None
+
+ def name(self):
+ return "node-py"
+
+ def set_parent(self, parent):
+ self.parent = parent
+
+ def get_parent(self):
+ return self.parent
+
+ def strong_count(self):
+ return 0 # TODO
+
 class TraitsTest(unittest.TestCase):
  # Test traits implemented in Rust
- def test_rust_getters(self):
- test_getters(make_rust_getters())
- self.check_getters_from_python(make_rust_getters())
+ # def test_rust_getters(self):
+ # test_getters(None)
+ # self.check_getters_from_python(make_rust_getters())
+
+ # Test traits implemented in Rust
+ def test_python_getters(self):
+ test_getters(PyGetters())
+ #self.check_getters_from_python(PyGetters())
 
  def check_getters_from_python(self, getters):
  self.assertEqual(getters.get_bool(True, True), False);
@@ -317,6 +375,7 @@ def check_getters_from_python(self, getters):
  getters.get_string("unexpected-error", True)
 
  def test_path(self):
+ # Get traits creates 2 objects that implement the trait
  traits = get_traits()
  self.assertEqual(traits[0].name(), "node-1")
  # Note: strong counts are 1 more than you might expect, because the strong_count() method
@@ -326,11 +385,33 @@ def test_path(self):
  self.assertEqual(traits[1].name(), "node-2")
  self.assertEqual(traits[1].strong_count(), 2)
 
+ # Let's try connecting them together
  traits[0].set_parent(traits[1])
+ # Note: this doesn't increase the Rust strong count, since we wrap the Rust impl with a
+ # python impl before passing it to `set_parent()`
+ self.assertEqual(traits[1].strong_count(), 2)
  self.assertEqual(ancestor_names(traits[0]), ["node-2"])
  self.assertEqual(ancestor_names(traits[1]), [])
- self.assertEqual(traits[1].strong_count(), 3)
  self.assertEqual(traits[0].get_parent().name(), "node-2")
+
+ # Throw in a Python implementation of the trait
+ # The ancestry chain now goes traits[0] -> traits[1] -> py_node
+ py_node = PyNode()
+ traits[1].set_parent(py_node)
+ self.assertEqual(ancestor_names(traits[0]), ["node-2", "node-py"])
+ self.assertEqual(ancestor_names(traits[1]), ["node-py"])
+ self.assertEqual(ancestor_names(py_node), [])
+
+ # Rotating things.
+ # The ancestry chain now goes py_node -> traits[0] -> traits[1]
+ traits[1].set_parent(None)
+ py_node.set_parent(traits[0])
+ self.assertEqual(ancestor_names(py_node), ["node-1", "node-2"])
+ self.assertEqual(ancestor_names(traits[0]), ["node-2"])
+ self.assertEqual(ancestor_names(traits[1]), [])
+
+ # Make sure we don't crash when undoing it all
+ py_node.set_parent(None)
  traits[0].set_parent(None)
 
 if __name__=='__main__':

fixtures/coverall/tests/bindings/test_coverall.swift

@@ -319,10 +319,12 @@ do {
  assert(traits[1].name() == "node-2")
  assert(traits[1].strongCount() == 2)
 
+ // Note: this doesn't increase the Rust strong count, since we wrap the Rust impl with a
+ // python impl before passing it to `set_parent()`
  traits[0].setParent(parent: traits[1])
  assert(ancestorNames(node: traits[0]) == ["node-2"])
  assert(ancestorNames(node: traits[1]) == [])
- assert(traits[1].strongCount() == 3)
+ assert(traits[1].strongCount() == 2)
  assert(traits[0].getParent()!.name() == "node-2")
  traits[0].setParent(parent: nil)
 }

uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs

@@ -313,6 +313,7 @@ impl KotlinCodeOracle {
  "UniFffiRustFutureContinuationCallbackType".to_string()
  }
  FfiType::RustFutureContinuationData => "USize".to_string(),
+ FfiType::VTable(_name) => unimplemented!(),
  }
  }
 }

uniffi_bindgen/src/bindings/python/gen_python/mod.rs

@@ -314,6 +314,33 @@ impl PythonCodeOracle {
  FfiType::RustFutureHandle => "ctypes.c_void_p".to_string(),
  FfiType::RustFutureContinuationCallback => "_UNIFFI_FUTURE_CONTINUATION_T".to_string(),
  FfiType::RustFutureContinuationData => "ctypes.c_size_t".to_string(),
+ // VTables are passed as a pointer to the struct. Use a c_void_p since we want this to
+ // work before the struct is defined.
+ FfiType::VTable(_) => format!("ctypes.c_void_p"),
+ }
+ }
+
+ fn return_type_default(callable: impl Callable) -> String {
+ match callable.return_type() {
+ None => "None".to_owned(),
+ Some(t) => match t.clone().into() {
+ FfiType::Int8
+ | FfiType::UInt8
+ | FfiType::Int16
+ | FfiType::UInt16
+ | FfiType::Int32
+ | FfiType::UInt32
+ | FfiType::Int64
+ | FfiType::UInt64 => "0".to_owned(),
+ | FfiType::Float32
+ | FfiType::Float64 => "0.0".to_owned(),
+ FfiType::RustArcPtr(_) => "None".to_owned(),
+ FfiType::RustBuffer(maybe_suffix) => match maybe_suffix {
+ Some(suffix) => format!("_UniffiRustBuffer{suffix}.default()"),
+ None => "_UniffiRustBuffer.default()".to_owned(),
+ },
+ _ => panic!("Don't know how to return {t:?}"),
+ },
  }
  }
 }
@@ -412,6 +439,26 @@ pub mod filters {
  Ok(type_.into())
  }
 
+ pub fn return_type_default(callable: impl Callable) -> Result<String, askama::Error> {
+ Ok(PythonCodeOracle::return_type_default(callable))
+ }
+
+ pub fn ctypes_ffi_prototype(ffi_func: &FfiFunction) -> Result<String, askama::Error> {
+ Ok(format!(
+ "ctypes.CFUNCTYPE({}, {}, ctypes.POINTER(_UniffiRustCallStatus))",
+ match &ffi_func.return_type() {
+ Some(v) => PythonCodeOracle::ffi_type_label(v),
+ None => "None".to_owned(),
+ },
+ ffi_func
+ .arguments()
+ .into_iter()
+ .map(|a| PythonCodeOracle::ffi_type_label(&a.type_()))
+ .collect::<Vec<_>>()
+ .join(", ")
+ ))
+ }
+
  /// Get the Python syntax for representing a given low-level `FfiType`.
  pub fn ffi_type_name(type_: &FfiType) -> Result<String, askama::Error> {
  Ok(PythonCodeOracle::ffi_type_label(type_))

uniffi_bindgen/src/bindings/python/templates/Helpers.py

@@ -16,18 +16,22 @@ class _UniffiRustCallStatus(ctypes.Structure):
  # These match the values from the uniffi::rustcalls module
  CALL_SUCCESS = 0
  CALL_ERROR = 1
- CALL_PANIC = 2
+ CALL_UNEXPECTED_ERROR = 2
 
  def __str__(self):
  if self.code == _UniffiRustCallStatus.CALL_SUCCESS:
  return "_UniffiRustCallStatus(CALL_SUCCESS)"
  elif self.code == _UniffiRustCallStatus.CALL_ERROR:
  return "_UniffiRustCallStatus(CALL_ERROR)"
- elif self.code == _UniffiRustCallStatus.CALL_PANIC:
- return "_UniffiRustCallStatus(CALL_PANIC)"
+ elif self.code == _UniffiRustCallStatus.CALL_UNEXPECTED_ERROR:
+ return "_UniffiRustCallStatus(CALL_UNEXPECTED_ERROR)"
  else:
  return "_UniffiRustCallStatus(<invalid code>)"
 
+ @classmethod
+ def default(cls):
+ cls(_UniffiRustCallStatus.CALL_SUCCESS, _UniffiRustBuffer.default())
+
 def _rust_call(fn, *args):
  # Call a rust function
  return _rust_call_with_error(None, fn, *args)
@@ -53,7 +57,7 @@ def _uniffi_check_call_status(error_ffi_converter, call_status):
  raise InternalError("_rust_call_with_error: CALL_ERROR, but error_ffi_converter is None")
  else:
  raise error_ffi_converter.lift(call_status.error_buf)
- elif call_status.code == _UniffiRustCallStatus.CALL_PANIC:
+ elif call_status.code == _UniffiRustCallStatus.CALL_UNEXPECTED_ERROR:
  # When the rust code sees a panic, it tries to construct a _UniffiRustBuffer
  # with the message. But if that code panics, then it just sends back
  # an empty buffer.
@@ -66,6 +70,30 @@ def _uniffi_check_call_status(error_ffi_converter, call_status):
  raise InternalError("Invalid _UniffiRustCallStatus code: {}".format(
  call_status.code))
 
+def _uniffi_trait_interface_call(call_status, make_call, lower_fn, default_return):
+ try:
+ return lower_fn(make_call())
+ except Exception as e:
+ import traceback
+ print(f"Unexpected error in _uniffi_trait_interface_call: {e}")
+ call_status.code = _UniffiRustCallStatus.CALL_UNEXPECTED_ERROR
+ call_status.error_buf = {{ Type::String.borrow()|lower_fn }}(str(e))
+ return default_return
+
+def _uniffi_trait_interface_call_with_error(call_status, make_call, lower_fn, default_return, error_type, lower_error):
+ try:
+ try:
+ return lower_fn(make_call())
+ except error_type as e:
+ call_status.code = _UniffiRustCallStatus.CALL_ERROR
+ call_status.error_buf = lower_error(e)
+ return default_return
+ except Exception as e:
+ print(f"Unexpected error in _uniffi_trait_interface_call_with_error: {e}")
+ call_status.code = _UniffiRustCallStatus.CALL_UNEXPECTED_ERROR
+ call_status.error_buf = {{ Type::String.borrow()|lower_fn }}(str(e))
+ return default_return
+
 # A function pointer for a callback as defined by UniFFI.
 # Rust definition `fn(handle: u64, method: u32, args: _UniffiRustBuffer, buf_ptr: *mut _UniffiRustBuffer) -> int`
 _UNIFFI_FOREIGN_CALLBACK_T = ctypes.CFUNCTYPE(ctypes.c_int, ctypes.c_ulonglong, ctypes.c_ulong, ctypes.POINTER(ctypes.c_char), ctypes.c_int, ctypes.POINTER(_UniffiRustBuffer))

uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py

@@ -63,7 +63,8 @@ def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}:
 {% endmatch %}
 {% endfor %}
 
-
+{%- match imp %}
+{%- when ObjectImpl::Struct %}
 class {{ ffi_converter_name }}:
  @classmethod
  def read(cls, buf):
@@ -72,16 +73,46 @@ def read(cls, buf):
  raise InternalError("Raw pointer value was null")
  return cls.lift(ptr)
 
+ @staticmethod
+ def lift(value):
+ return {{ type_name }}._make_instance_(value)
+
  @classmethod
  def write(cls, value, buf):
  if not isinstance(value, {{ type_name }}):
  raise TypeError("Expected {{ type_name }} instance, {} found".format(type(value).__name__))
  buf.write_u64(cls.lower(value))
 
+ @staticmethod
+ def lower(value):
+ return value._pointer
+{%- when ObjectImpl::Trait %}
+{%- let trait_impl="UniffiTraitImpl{name}"|format %}
+{%- include "TraitObjectImpl.py" %}
+class {{ ffi_converter_name }}:
+ # Lift and read Rust-backed trait objects
  @staticmethod
  def lift(value):
- return {{ type_name }}._make_instance_(value)
+ obj = {{ type_name }}._make_instance_(value)
+ print(f"lift: {obj} {value:x}", flush=False)
+ return obj
+
+ @classmethod
+ def read(cls, buf):
+ ptr = buf.read_u64()
+ if ptr == 0:
+ raise InternalError("Raw pointer value was null")
+ return cls.lift(ptr)
 
+ # Lower and write Python-backed trait objects
  @staticmethod
  def lower(value):
- return value._pointer
+ ptr = {{ trait_impl }}._UniffiPointerManager.new_pointer(value)
+ print(f"lower: {value} {ptr:x}", flush=False)
+ return ptr
+
+ @classmethod
+ def write(cls, value, buf):
+ buf.write_u64(cls.lower(value))
+
+{%- endmatch %}

uniffi_bindgen/src/bindings/python/templates/PointerManager.py

@@ -44,7 +44,7 @@ class _UniffiPointerManagerGeneral:
  def __init__(self):
  self._map = {}
  self._lock = threading.Lock()
- self._current_handle = 0
+ self._current_handle = 1
 
  def new_pointer(self, obj):
  with self._lock:
@@ -62,7 +62,10 @@ def lookup(self, handle):
  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
+# Unfortunately, the _UniffiPointerManagerCPython is not compatible with the rubicon hack, so we
+# always use the slower _UniffiPointerManagerGeneral
+# if platform.python_implementation() == 'CPython':
+# _UniffiPointerManager = _UniffiPointerManagerCPython # type: ignore
+# else:
+# _UniffiPointerManager = _UniffiPointerManagerGeneral # type: ignore
+_UniffiPointerManager = _UniffiPointerManagerGeneral # type: ignore