-
Notifications
You must be signed in to change notification settings - Fork 234
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add an ADR which calls for replacing
HandleMap
s with Arc
s.
Combined with [ADR-0004](https://github.com/mozilla/uniffi-rs/blob/main/docs/adr/0004-only-threadsafe-interfaces.md ) this is the last significant decision to be made on the path to allowing us to [pass around object instances](#419).
- Loading branch information
Showing
1 changed file
with
105 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
# Use `Arc<>` pointers instead of `HandleMap`s | ||
|
||
* Status: proposed | ||
* Deciders: mhammond; proposed: rfkelly, travis, jhugman, dmose | ||
* Date: 2021-04-19 | ||
|
||
Discussion and approval: [PR 430](https://github.com/mozilla/uniffi-rs/pull/430) | ||
Technical Story: [Issue 419](https://github.com/mozilla/uniffi-rs/issues/419) | ||
|
||
## Context and Problem Statement | ||
|
||
uniffi currently leans heavily on the HandleMap struct in the ffi-support crate. | ||
This means that external consumers of uniffi-wrapped interfaces never see | ||
any pointers to structs - instead, they get what is (roughly) an index into | ||
an array, with the pointer being in that array. | ||
|
||
This has a number of safety characteristics which is particularly important for | ||
hand-written FFI interfaces, but does cause some issues in evolving uniffi in | ||
directions we consider important. In particular, it means that the lifetimes of | ||
uniffi wrapped structs is very rigid and thus only suitable as the `self` for | ||
the struct and not as generic arguments and dictionary members. | ||
|
||
This ADR considers ways to evolve the lifetime restrictions so that | ||
references to structs can be used ore widely than currently allowed. | ||
|
||
## Decision Drivers | ||
|
||
* We desire the ability to have more flexible life-times for our interfaces so | ||
they can be stored in dictionaries or other interfaces, and be returned by | ||
functions other than constructors. | ||
|
||
* We would like to keep the uniffi implementation as simple as possible while | ||
providing a suitable degree of safety - in particular, a promise that it | ||
should be impossible to misuse the generated bindings in a way that causes | ||
us into Rust's "undefined behavior" (and in particular, avoiding things like | ||
use-after-free issues) | ||
|
||
* We would like to keep the overhead of uniffi as small as possible so that it | ||
is a viable solution to more use-cases. | ||
|
||
## Considered Options | ||
|
||
* [Option 1] Build the capability into HandleMaps. This would involve deciding | ||
on how we want to track lifetimes (eg, via a reference counting or garbage | ||
collection) and actually building it. | ||
|
||
* [Option 2] We replace the use of HandleMaps with Rust `Arc<>` pointers - in | ||
other words, all uniffi generated bindings start passing raw pointers around. | ||
|
||
## Decision Outcome | ||
|
||
Chosen option: | ||
|
||
* **[Option 2] We replace the use of HandleMaps with Rust `Arc<>` pointers** | ||
|
||
This decision is taken because: | ||
|
||
* We believe the additional safety offered by `HandleMap`s is far less | ||
important for this use-case because the code using these pointers is | ||
generated instead of hand-written. | ||
|
||
* Correctly implementing better lifetime management in a thread-safe way is not | ||
trivial and subtle errors there would defeat all the safely mechanisms the | ||
`HandleMap`s offer. Ultimately we just end up reimplementing "`Arc<>` anyway, | ||
and the one in the stdlib is far more likely to be correct. | ||
|
||
### Positive Consequences | ||
|
||
* There will be less overhead in our generated code - both performance overhead | ||
and cognitive overload - it will be much easier to rationalize about how | ||
the generated code actually works and performs. | ||
|
||
### Negative Consequences | ||
|
||
* Errors in our generated code might cause pointer misuse and lead to "use | ||
after free" type issues. | ||
|
||
## Pros and Cons of the Options | ||
|
||
### [Option 1] | ||
|
||
* Good, because raw pointers aren't handed out anywhere. | ||
|
||
* Bad, because we need to reimplement safe reference counting or garbage | ||
collection. | ||
|
||
* Bad, because code generation remains somewhat complex. | ||
|
||
### [Option 2] | ||
|
||
* Good, because we can reuse the Rust standard library and have confidence in | ||
its implementation. | ||
|
||
* Good, because the code generated by uniffi is clearer and easier to understand. | ||
|
||
* Bad, because handing raw pointers around means bugs in the generated code or | ||
intentional misuse of the bindings might cause vulnerabilities. | ||
|
||
## Links | ||
|
||
* Thom discusses this a but in [this issue](https://github.com/mozilla/uniffi-rs/issues/244) | ||
and agrees with the assertion that raw pointer make sense when all | ||
the relevant code is generated. | ||
|
||
* Ryan discusses his general approval for this approac in [this issue](https://github.com/mozilla/uniffi-rs/issues/419) |