search

godot-rust / gdnative

Rust bindings for Godot 3
https://godot-rust.github.io
MIT License
3.62k stars 210 forks source link

Deref on the Unique typestate is unsound #635

Open ghost opened 3 years ago

ghost commented 3 years ago

As long as API method calls are allowed on references with typestates, their invariants cannot be soundly upheld without manual review.

Suppose that there is a method Node::add_as_child_to(other: Node) that adds this as a child to other. If such a method is called on a reference with Unique access, the access state is invalidated. There is no way we can identify such methods should they exist in the API without manual review of engine code (or at least documentation).

As such, Deref should not be implemented for the Unique typestate. Functions that create new instances should return Shared instead (and require a assume_safe call afterwards).

ghost commented 3 years ago

Distinguishing Unique is probably still useful for free and queue_free, but the Deref implementation should be removed. It should only be possible to create TRefs for Shared references.

ghost commented 3 years ago

I wonder if ThreadLocal can be preserved since user code outside Rust is assumed to follow the threading guidelines as part of the safety model? If so then it's only Unique that is the most fragile.