godot-rust / gdext

Rust bindings for Godot 4
https://mastodon.gamedev.place/@GodotRust
Mozilla Public License 2.0
3.13k stars 198 forks source link

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

Open lilizoey opened 2 months ago

lilizoey commented 2 months ago

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().

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:

This means we're effectively adding 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(). 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.

Yarwin commented 2 months ago

Note – in some cases InstanceId can be used as, well, InstanceId, used to recognize given instance while passing message from the thread. While it is not a huge deal (InstanceId stil can be converted to u64) by itself, proper deprecation warnings would be much appreciated.

Bigger issue might be some thread-safe APIs (according to docs - godot's servers – https://docs.godotengine.org/en/stable/tutorials/performance/thread_safe_apis.html#global-scope) that operate with InstanceIds. Example: https://docs.godotengine.org/en/stable/classes/class_physicsserver3d.html#class-physicsserver3d-method-area-attach-object-instance-id https://docs.godotengine.org/en/stable/classes/class_physicsserver3d.html#class-physicsserver3d-method-area-get-object-instance-id

hammerandtongs commented 2 months ago

Fwiw https://github.com/jrockett6/bevy_godot4/issues/1#issuecomment-2224877086

and yes https://github.com/search?q=repo%3Ajrockett6%2Fbevy_godot4+try_from&type=code