godot-rust / godot-rust/gdext

Deal with cross-thread access to user-defined GodotClass instances

Open
#18 20 comments 5 reactions 0 assignees View on GitHub
c: ffi c: threads feature hard
Dominant language
Rust
Stars
5.2k
Forks
312
Avg merge
12h 59m
Merged PRs (30d)
9

Description

Currently, `RefCell` and `i32` are used to store user-defined `GodotClass` instances and their reference counts:

https://github.com/godot-rust/gdextension/blob/a64f22409e05d1353492fded4aa36a4b4e6b710d/godot-core/src/storage.rs#L15-L21

In a multi-threaded configuration of the engine, this may lead to unsoundness when the instances are accessed by GDScript or engine code. Noting that `GodotClass` does not have a `Send` or `Sync` requirement, this can be as simple as a non-`Send` field in the type.

@Bromeon in Discord:

> I plan to have a mix of multiple things:
> * the "you need to take some responsibility of the non-Rust code"
> * no access to Godot thread APIs as direct Rust classes (Thread, Mutex, Semaphore) by default, and > unsafe access when there is a threads feature enabled
> * GDScript->Rust calls could validate the thread ID and compare it with the one through which the extension library was initialized (which I hope is the main thread)
> A Cargo feature thread would loosen those restrictions and give access to multithreading, in an unsafe way.
> We could then either conditionally implement Send/Sync on Gd, or maybe better, provide an explicit Send/Sync type to transport references across thread boundaries.
Users would still need to adhere to the Godot thread guidelines.

As prior art, the GDNative bindings deals with the problem with the [user-data wrapper system](https://godot-rust.github.io/docs/gdnative/export/user_data/index.html).

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.