[discuss] A proposal for passing around object instances

[rfk]

Riffing on some conversations today with @mhammond, here are some partly-formed thoughts on how we might work towards enabling object instances to be passed as arguments and returned from functions/methods, resolving #40 and #197.

Broadly, we have two big complexities to grapple with here - the way that handlemaps make lifting/lowering codegen for object instances difficult, and the management of lifetimes and object identity across the ffi boundary. I think we may be able to resolve both of these with some judicious use of Arc.

Step 1: Remove Handlemaps

First, we replace our current use of handlemaps with Arc. I expect this will be more performant, but really I'm interested in simplifying codegen in the bindings.

For a threadsafe interface T, we put it in an Arc<T> just like we do today. But instead of putting the Arc<T> into a handlemap to turn it into an integer handle, we use Arc::into_raw to turn it directly into a pointer, and pass the pointer over the ffi to the foreign-language code. When the foreign-language code wants to call a method on the instance, it passes the pointer back to the Rust code and we use Arc::from_raw to reconstitute the Arc, and can use it like we currently do with the one in the handlemap. (We have to be careful about not dropping the Arc after reconstituting it, but this seems tractable).

For a non-threadsafe interface T, we first make it synchronous by putting it in a Mutex<T>, then putting that inside an Arc<Mutex<T>>. We can then treat it the same way as for threadsafe instances above, converting to and from a raw pointer to pass over the FFI.

I need to dig more into the memory-safety and thread-safety requirements of the above, but it seems like it should be tractable.

The aim of this is to remove the use of handlemap closures in order to get object references in Rust. Rather than generating code like:

HANDLE_MAP.call_with_result(obj_handle, |obj| -> obj.method())

We'd generate code like:

let obj = unsafe { Arc::from_raw(obj_ptr) } // TODO: some manually-drop stuff in here somewhere probably
let obj = obj.clone()  // get our own copy of the Arc, to make sure it stays alive
let mut obj = obj.lock() // if we're using a Mutex, we can lock it here 
obj.method()

This resolves one of the big issues in #40 - if we pass multiple object instances as arguments over the FFI, we don't need to generate any nested closures to read items out of the handlemap, we can just generate straight-line code for each one.

For non-threadsafe interfaces, this doesn't solve the potential for deadlocking on the hidden Mutexes, but TBH, I think the right way to resolve that is to push all the locking down into Rust code where it's visible, and use threadsafe interfaces instead.

Step 2: Allow objects in argument position, with annotations.

Given the above, there are three ways that a Rust function exposed via uniffi could accept an object instance as an argument:

• By "reference", implied to live for the duration of the call: &ObjectName.
• For non-threadsafe types, by "mutable reference", with implied lifetime as above: &mut ObjectName.
• As an owned copy of the arc: Arc<ObjectName> or Arc<Mutex<ObjectName>>.

We'd need to represent each of these differently in the UDL so that we know how to codegen them, perhaps expanding our use of attributes like so:

• method([ByRef] ObjectName argname)
• method([ByMut] ObjectName argname)
• method([ByArc] ObjectName argname)

The names are not great, but you get the idea.

Note that Rust's lifetime/ownership system helps us avoid doing anything silly here. If you accept a [ByRef] ObjectName, you can't mutate it and you can't stash a reference to it anywhere that would outlive the function call. We don't attempt to provide any cleverness in handling lifetimes at the FFI boundary, and the only way for Rust code to accept a T that it can stash internally for later use, it to take its own reference-count-already-incremented owned copy of an Arc<T>.

I expect most of the complexity here would be in deciding on how to expose this to consumers.

Note that the Rust code at this stage cannot return existing object instances back to the foreign-language code, so we don't get #197 yet, and also we don't have the instance-identity-mapping problem yet.

Step 3: Allow objects in return position, with a reverse identity map.

Since we don't attempt to provide any cleverness in handling lifetimes at the FFI boundary, I don't think we can allow uniffied functions to return references to objects. However, we could correctly handle two cases:

• Functions that return an owned object: func() -> T.
• Functions that return an owned shared reference: func() -> Arc<T>.

In the first case, we know that the return value is the unique existing reference to the object. We can handle this in the same way we handle values returned from a constructor - by putting it in an Arc<T>, turning it into a pointer, and handing that back to the foreign-language code to create a new object wrapper around it. (This is the same safety argument I was trying to make over in #384 (comment), which perhaps is easier to form a cohesive mental model of in this more general setting).

The second case is more interesting. The Arc might contain a T that has never been seen before for the foreign-language bindings, or it might be a clone of an existing Arc that the foreign language bindings already hold a reference to. After turning this Arc into a pointer and passing it back to the foreign-language code, I think we need some sort of reverse identity map that - check if the foreign-language code already has an Object wrapper for this pointer, return the existing Object if so, or create a new one if not. (N.B. you can have many clones of an Arc<T> but when you turn them into raw pointers, they all turn into the same pointer, being the address at which the shared T is allocated on the heap).

Using Arcs, we can implement passing of object instances back and forth across the FFI while preserving object identity. Suppose we have something like this proposed interface for nimbus:

interface NimbusClient {
  constructor(...);
   [... other methods here ...]
};

namespace nimbus {

    void set_singleton([ByArc] NimbusClient client);
    [ByArc] NimbusClient get_singleton()

};

The foreign-language code could do something like this:

c1 = NimbusClient()
set_singleton(c1)
c2 = get_singleton()
assert c1 === c2

In the Rust implementation, we'd have functions like:

pub fn set_singleton(client: Arc<NimbusClient>) {
  // we own this arc, we can stash it in global Cell or something.
  GLOBAL_SINGLETONE.set(client);
};

pub fn get_singleton() -> Arc<NimbusClient> {
 // We can return as many clones of this as we want.
  GLOBAL_SINGLETON.get().clone() 
}

And UniFFI would generate the necessary plumbing to connect them together.

That's a lot, but I think this would work, and can be tackled in a few incremental pieces. Curious to hear what others think, and especially if you can poke any holes in the hand-waving above. /cc @mhammond @jhugman in particular based on previous discussions, but all input welcome.

┆Issue is synchronized with this Jira Task

[rfk]

I think we need some sort of reverse identity map that - check if the foreign-language code already
has an Object wrapper for this pointer, return the existing Object if so, or create a new one if not

A note to self, if this reverse identity map is going to conceptually map multiple Arc<T> on the Rust side to a single object on the foreign-language side, it's going to need to some smarts around object deallocation. When the foreign-language object is freed, it will need to decrement the refcount once for each Arc<T> that has been "merged in" to it. That could get thorny fast, but again, I think insisting that the foreign-language code only consumes owned Arc<T>s will help us keep things lined up correctly.

[rfk]

Cross-referencing #244, because @thomcc had some excellent notes on the history of handlemaps and on things we'll need to do if we want to move to raw pointers (including the important requirement that T: Send for any type we want to use like this).

[mhammond]

This sounds great and makes alot of sense. Ryan and I chatted a little and where I landed was:

My initial reaction was concern about dropping the safety features of handle-maps, particularly the panic handling and "handle poisoning". However, as #244 mentions, this is less of a problem for generated code. I've a vague concern that not all languages will prevent "accidental misuse" (eg, managing to take a copy of the raw pointer without the generated code knowing about it would be catastrophic) but Ryan reminded me that all our target languages already have libraries which wrap pointers and manage to do so safely. Panic safely can still be generated. So this sounds good to me.

Ryan also admitted that ideally he'd like to see non-threadsafe support dropped, so all locking primitives end up in explicit, hand-written rust code - most likely via mutexes. The pros of this include:

• Less chance of footguns by the mutexes being "hidden", so consumers not having the correct mental model, and thus avoiding our "nimbus theading issue". TBH I'm not completely sold on this - I suspect we would have still had that bug even with locks in visible rust - that bug was more a "rite of passage" for a new cross-language team.
• A smaller and easier to rationalize about uniffi, and less opportunities for requests of supporting various hybrid models (eg, "I'd like some things to be threadsafe and some not") - we make this be entirely the problem of the hand-written code rather than uniffi.

The cons are:

• Simple use-cases become more difficult. I don't think this should be a goal though - if you are making the decision to implement a component in rust that you want to use over an FFI from other languages, you can't declare your use-case is "simple". Just have a mutex and acquire it everywhere.
• Temptation to do complex locking where simple locking would do - aka, "if I need to do my own locking, I might as well make it was efficient as possible". I am slightly sympathetic to that.
• Breaking existing code. But I don't think Mozilla has any existing uniffi code that would be broken - it all starts out simple, then next thing you know we are having this discussion. There are no visible external users yet, and we haven't made any stability promises.

On balance, I'm inclined to support dropping support for the "non thread-safe" case, and if we can agree to that, we should just do it (ie, instead of it being an asprirational "eventually we'd like to", we should just do it now). We can always add stuff later.

Re the identity map: I'd be inclined to put this in the "nice to have" rather than making it an absolute requirement - primarily because it doesn't seem like it would be a breaking change - ie, if 2 objects that should, but not not, compare correctly with === today, then fixing that in the future probably wouldn't break that code. I'm interested to know @jhugman's take on this.

I guess my final thought is that this is still somewhat abstract, and that we should try and work on the canonical demo now - an actual UDL and document the semantics. OTOH, it's not clear this will be hugely valuable because some of the edge-cases or things we haven't considered might remain hidden. However, it's likely to give us something more concrete to hang our opinions on, which can't hurt. I said I'd have a poke at this, but I'm not sure I'll get far this week.

[jhugman]

This looks very promising/exciting!

Off the bat, I will third the decision to drop support for non-[Threadsafe] objects.

I'm a little bit confused around the safety of passing ByRef:

• By "reference", implied to live for the duration of the call: &ObjectName.
• For non-threadsafe types, by "mutable reference", with implied lifetime as above: &mut ObjectName.
• As an owned copy of the arc: Arc<ObjectName> or Arc<Mutex<ObjectName>>.

We'd need to represent each of these differently in the UDL so that we know how to codegen them, perhaps expanding our use of attributes like so:

• method([ByRef] ObjectName argname)
• method([ByMut] ObjectName argname)
• method([ByArc] ObjectName argname)

If we are leaving the locking to the hand-written Rust crate, i.e. [Threadsafe] by default, does that mean that the foreign language twin of the object can cause the Rust object to be dropped from another thread?

I'm also not sure what the default would be: what does void method(ObjectName argname) translate to? IIUC, it seems to me that passing any UDL interfaces objects should default to Arc<T>.

If that's right, then we can reduce the UDL surface to

• void method(ObjectName arg);
• void method([ByRef] arg);

So far, so good.

Getting rid of handlemaps seems sensible, but if I have a constructor (or a factory method):

interface RObject {
    constructor();
}

i.e. a Rust fn that returns a T; previously the handle map would own the T, so we could maintain the fiction that the object had crossed the FFI and was now inaccessible from Rust.

Now with no handle maps, who holding the T while it's on the foreign language side?

Conceptually, I'm arriving at a place where we have only constructors handing ownership to the foreign languages side, and if Rust needs it back, it will need to be done via an Arc<T>, or by an a foreign language deinit/destroy.

The Reverse Identity Map

I have a few thoughts on this; I like @mhammond 's YAGNI noises here; however, we should try and take steps to aggressively document this. e.g. forcing an [Copy] attribute for return interface objects in the UDL.

interface Thing {
     [Name=from_json]
     constructor(string json_string);
}

namespace things {
    void set_singleton(Thing thing);
    [Copy] Thing get_singleton();
}

Alternative spellings for [Copy]: [Twin], [Incomparable].

I think I share the unease about the handle/identity map; without further annotation of the objects being passed in or returned out related to their use, I think it'll be hard to make a good general solution.

Without thinking too hard about this, I can imagine an annotation to guide uniffi on which side of the FFI the object (or its twin) will spend most of its time.

namespace things {
    void set_singleton([Longlived] Thing thing);
    [Shortlived] Thing get_singleton();

    [Longlived] Thing configure_quickly([Shortlived] Thing thing);
}

[rfk]

If we are leaving the locking to the hand-written Rust crate, i.e. [Threadsafe] by default, does that mean that the
foreign language twin of the object can cause the Rust object to be dropped from another thread?

For example, if one foreign-language thread is doing a method call while another thread destroys the object? I think ideally the foreign-language bindings would not allow this, perhaps because they can only destroy the object in the foreign-language destructor. But you're right, we'll have to have a story about how to prevent this

As long as the foreign-languge code doesn't destroy the object while a method-call is in flight, we should be OK. I reckon we could guard against this with some foreign-language side locking around the cloning of the arc if we need to.

what does void method(ObjectName argname) translate to?

We also have the option of rejecting that syntax, and insisting on an explicit annotation.

previously the handle map would own the T, so we could maintain the fiction that the object had crossed the
FFI and was now inaccessible from Rust.
Now with no handle maps, who holding the T while it's on the foreign language side?

Conceptually, calling Arc::into_raw will "disappear" that arc from Rust's ownership system without dropping it. We can pass the resulting raw pointer to the foreign-language code and declare that the foreign-language code now "owns" that arc, and is responsible for dropping it when it's done ( by re-materializing it into Rust's ownership system, and then dropping it).

Concretely, putting the T into an Arc means that the T and its reference count lives behind a pointer on the heap; everyone who owns a copy of the Arc is responsible for decrementing the reference count by one when they drop their copy. We can use the above to "disappear" one increment of the refcount to the foreign-language code, letting it keep the allocation alive for as long as the foreign-language "twin" is alive.

Conceptually, I'm arriving at a place where we have only constructors handing ownership to the foreign languages side,
and if Rust needs it back, it will need to be done via an Arc, or by an a foreign language deinit/destroy.

This sounds right to me, yes.

however, we should try and take steps to aggressively document this. e.g. forcing an [Copy] attribute
for return interface objects in the UDL.

TBH this doesn't sound that useful to me at first glance, but then again, I spend a lot more time on the other side of the boundary.

It's not clear to me what the consumer would do with this information - assuming we don't support the reverse handlemap at all, then every single object instance returned from the UDL is going to be a fresh object, and the annotation doesn't tell you what it's a copy of. Maybe we need a more concrete consumer example here to help make the discussion more concrete?

[jhugman]

however, we should try and take steps to aggressively document this. e.g. forcing an [Copy] attribute
for return interface objects in the UDL.

TBH this doesn't sound that useful to me at first glance, but then again, I spend a lot more time on the other side of the boundary.

It's not clear to me what the consumer would do with this information

I could've been clearer: this would be in the context where we're don't provide any bookkeeping of the foreign language objects that have been passed into rust, and then out again, i.e. reverse identity maps.

Without the identity map

As @mhammond points out; these objects would be equal (==) but not the same objects (===). I don't think this is a problem that's worth the difficult bookkeeping for such a marginal case (it's only for Rust objects constructed in Foreign Land, then passed into Rust, then returned back to Foreign Land).

My [Copy] attribute (or some other spelling) would force documentation in to the UDL. In this example, spelling it NotTheSame would make this sort of method deliberately— and rightly— dissonant; and should make the API designer re-consider the method.

[NotTheSame] Thing identity(Thing thing);

With the identity map.

The other attributes (Longlived and Shortlived) are off-the-cuff hints to the identity map implementation, because I can't think of a scheme that won't leak memory without it knowing the intent of how the API is going to be used.

[rfk]

This work shipped in v0.12.0, via #462.