Safety comments in MultiThreadedExecutor are incorrect
#7833
Comments
JoJoJet
added
A-ECS
alice-i-cecile
pushed a commit
that referenced
this issue
Apr 17, 2023
# Objective The implementation of `System::run_unsafe` for `FunctionSystem` requires that the world is the same one used to initialize the system. However, the `System` trait has no requirements that the world actually matches, which makes this implementation unsound. This was previously mentioned in #7605 (comment) Fixes part of #7833. ## Solution Add the safety invariant that `System::update_archetype_component_access` must be called prior to `System::run_unsafe`. Since `FunctionSystem::update_archetype_component_access` properly validates the world, this ensures that `run_unsafe` is not called with a mismatched world. Most exclusive systems are not required to be run on the same world that they are initialized with, so this is not a concern for them. Systems formed by combining an exclusive system with a regular system *do* require the world to match, however the validation is done inside of `System::run` when needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Both places where
System::run_unsafeis called inMultiThreadedExecutorstill have placeholder safety comments. They simply state that the safety invariants are upheld, without making any attempt to explain how they are upheld.bevy/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Line 443 in de10dce
bevy/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Line 612 in de10dce
The text was updated successfully, but these errors were encountered: