godot-rust / godot-rust/gdext

`InstanceId` can be used to bypass `Gd` not being `Send`

Open
#886 2 comments 1 reaction 0 assignees View on GitHub
bug c: threads ub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.