`InstanceId` can be used to bypass `Gd` not being `Send`
- Dominant language
- Rust
- Stars
- 5.2k
- Forks
- 312
- Avg merge
- 12h 59m
- Merged PRs (30d)
- 9
Description
Currently `Gd` does not implement `Send`, because it is unsound to send to another thread. However you can easily bypass this restriction by using `InstanceId` and calling [`Gd::try_from_instance_id()`](https://godot-rust.github.io/docs/gdext/master/godot/obj/struct.Gd.html#method.try_from_instance_id).
Until we have a proper solution for multi-threaded code, this loophole should probably be available so that people who desperately need it can use it. But when we are going to support multi-threaded code, this loophole will need to be closed to ensure soundness.
My suggestion would be:
- Make `InstanceId` not implement `Send` or `Sync`. This will make it impossible to bypass multi-threading by merely passing an `InstanceId` to another thread.
- Make it impossible or unsafe to construct an `InstanceId` from a `u64`. This makes it so that you cant bypass the previous restriction by just passing the integer representation of an `InstanceId` to another thread.
This means we're effectively adding [strict provenance](https://doc.rust-lang.org/stable/core/ptr/index.html#strict-provenance) to `InstanceId`. Where it is only safe to obtain an `InstanceId` which has a known provenance, and any attempt to create an `InstanceId` out of thin air would be library UB since it can potentially violate arbitrary restrictions.
I am not sure if it is possible to call `try_from_instance_id` another way, such as through [`Object::call()`](https://godot-rust.github.io/docs/gdext/master/godot/classes/struct.Object.html#method.call). If that is possible then this suggestion would likely mean that that would need to be made unsafe as well. That would be non-ideal, but if we are going to impose any extra soundness criteria on top of Godot it may be unavoidable.
Contributor guide
Assessment
This issue has not been assessed yet.