`HandleAlloc::clone_handle` and `consume_handle` should be marked unsafe

[mgeisler]

The clone_handle and consume_handle methods in the HandleAlloc trait should be unsafe:

https://github.com/mozilla/uniffi-rs/blob/cd38ccea8236df7d93aff336c325a3a8e524af5d/uniffi_core/src/ffi_converter_traits.rs#L629-L637

The problem is that you can create a Handle with any u64 value you want in safe Rust:

let h = Handle::from_raw(42);
h.clone_handle(); // calls the unsafe Arc::increment_strong_count on 42

I discussed this with @badboy at RustFest and there is a chance that none of the generated bindings code ever calls it like that. If so, then it should be possible to add unsafe to the trait methods and propagate this upwards.

[badboy]

One way this is called is through this:

https://github.com/mozilla/uniffi-rs/blob/cd38ccea8236df7d93aff336c325a3a8e524af5d/uniffi_core/src/ffi_converter_traits.rs#L459

which gets called from e.g. this: https://github.com/mozilla/uniffi-rs/blob/main/uniffi_core/src/ffi/rustfuture/mod.rs#L75 which ultimately ends up being a FFI function. So the handle should indeed only be coming from generated code, so if the other uses are correct, the Handle received in clone_handle should be correct. Maybe we should still have that safety documentation somewhere.

Given we make assumptions about a Handle maybe the from_raw should already be unsafe and have additional documentation.

Again because this should not be called outside of generated code I still think the current code is sound, though would benefit from making those assumptions and guarantees more clear (through docs, unsafe and/or assertions)

[mhammond]

Given we make assumptions about a Handle maybe the from_raw should already be unsafe and have additional documentation.

Yeah, I agree that this seems more about from_raw then the assumption a handle is valid - otherwise we'd need to treat many more things as unsafe too?

Best I can tell though, we could do all the above without many changes at all - but does that even make sense?

[mhammond]

otherwise we'd need to treat many more things as unsafe too?

Not sure what I was thinking really - all use of handles is ultimately unsafe and all callers of these already are unsafe, so "all of the above" it is :)