diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 80c6f27d2e3d6..55c9ca20001ec 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -295,6 +295,8 @@ impl MultiThreadedExecutor { self.ready_systems.set(system_index, false); + // SAFETY: Since `self.can_run` returned true earlier, it must have called + // `update_archetype_component_access` for each run condition. if !self.should_run(system_index, system, conditions, world) { self.skip_system_and_signal_dependents(system_index); continue; @@ -313,7 +315,9 @@ impl MultiThreadedExecutor { break; } - // SAFETY: No other reference to this system exists. + // SAFETY: + // - No other reference to this system exists. + // - `self.can_run` has been called, which calls `update_archetype_component_access` with this system. unsafe { self.spawn_system_task(scope, system_index, systems, world); } @@ -383,7 +387,11 @@ impl MultiThreadedExecutor { true } - fn should_run( + /// # Safety + /// + /// `update_archetype_component` must have been called with `world` + /// for each run condition in `conditions`. + unsafe fn should_run( &mut self, system_index: usize, _system: &BoxedSystem, @@ -396,7 +404,8 @@ impl MultiThreadedExecutor { continue; } - // evaluate system set's conditions + // Evaluate the system set's conditions. + // SAFETY: `update_archetype_component_access` has been called for each run condition. let set_conditions_met = evaluate_and_fold_conditions(&mut conditions.set_conditions[set_idx], world); @@ -409,7 +418,8 @@ impl MultiThreadedExecutor { self.evaluated_sets.insert(set_idx); } - // evaluate system's conditions + // Evaluate the system's conditions. + // SAFETY: `update_archetype_component_access` has been called for each run condition. let system_conditions_met = evaluate_and_fold_conditions(&mut conditions.system_conditions[system_index], world); @@ -423,7 +433,9 @@ impl MultiThreadedExecutor { } /// # Safety - /// Caller must not alias systems that are running. + /// - Caller must not alias systems that are running. + /// - `update_archetype_component_access` must have been called with `world` + /// on the system assocaited with `system_index`. unsafe fn spawn_system_task<'scope>( &mut self, scope: &Scope<'_, 'scope, ()>, @@ -444,7 +456,9 @@ impl MultiThreadedExecutor { #[cfg(feature = "trace")] let system_guard = system_span.enter(); let res = std::panic::catch_unwind(AssertUnwindSafe(|| { - // SAFETY: access is compatible + // SAFETY: + // - Access: TODO. + // - `update_archetype_component_access` has been called. unsafe { system.run_unsafe((), world) }; })); #[cfg(feature = "trace")] @@ -605,7 +619,11 @@ fn apply_system_buffers( } } -fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &World) -> bool { +/// # Safety +/// +/// `update_archetype_component_access` must have been called +/// with `world` for each condition in `conditions`. +unsafe fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &World) -> bool { // not short-circuiting is intentional #[allow(clippy::unnecessary_fold)] conditions diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index ec782e715bd19..19108bf8c2254 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -164,6 +164,8 @@ where // so the caller will guarantee that no other systems will conflict with `a` or `b`. // Since these closures are `!Send + !Sync + !'static`, they can never be called // in parallel, so their world accesses will not conflict with each other. + // Additionally, `update_archetype_component_access` has been called, + // which forwards to the implementations for `self.a` and `self.b`. |input| self.a.run_unsafe(input, world), |input| self.b.run_unsafe(input, world), ) diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index d82c6ff153bee..97304332af7e6 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -6,7 +6,7 @@ use crate::{ check_system_change_tick, ExclusiveSystemParam, ExclusiveSystemParamItem, In, IntoSystem, System, SystemMeta, }, - world::{World, WorldId}, + world::World, }; use bevy_utils::all_tuples; @@ -25,7 +25,6 @@ where func: F, param_state: Option<::State>, system_meta: SystemMeta, - world_id: Option, // NOTE: PhantomData T> gives this safe Send/Sync impls marker: PhantomData Marker>, } @@ -43,7 +42,6 @@ where func, param_state: None, system_meta: SystemMeta::new::(), - world_id: None, marker: PhantomData, } } @@ -132,7 +130,6 @@ where #[inline] fn initialize(&mut self, world: &mut World) { - self.world_id = Some(world.id()); self.system_meta.last_run = world.change_tick().relative_to(Tick::MAX); self.param_state = Some(F::Param::init(world, &mut self.system_meta)); } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 0e88ad0187af4..a7f820254b633 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -452,10 +452,11 @@ where unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out { let change_tick = world.increment_change_tick(); - // Safety: - // We update the archetype component access correctly based on `Param`'s requirements - // in `update_archetype_component_access`. - // Our caller upholds the requirements. + // SAFETY: + // - The caller has invoked `update_archetype_component_access`, which will panic + // if the world does not match. + // - All world accesses used by `F::Param` have been registered, so the caller + // will ensure that there are no data access conflicts. let params = F::Param::get_param( self.param_state.as_mut().expect(Self::PARAM_MESSAGE), &self.system_meta, diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 307dd2bc7da61..3d9f4bb95f2ab 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -49,11 +49,17 @@ pub trait System: Send + Sync + 'static { /// 1. This system is the only system running on the given world across all threads. /// 2. This system only runs in parallel with other systems that do not conflict with the /// [`System::archetype_component_access()`]. + /// + /// Additionally, the method [`Self::update_archetype_component_access`] must be called at some + /// point before this one, with the same exact [`World`]. If `update_archetype_component_access` + /// panics (or otherwise does not return for any reason), this method must not be called. unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out; /// Runs the system with the given input in the world. fn run(&mut self, input: Self::In, world: &mut World) -> Self::Out { self.update_archetype_component_access(world); - // SAFETY: world and resources are exclusively borrowed + // SAFETY: + // - World and resources are exclusively borrowed, which ensures no data access conflicts. + // - `update_archetype_component_access` has been called. unsafe { self.run_unsafe(input, world) } } fn apply_buffers(&mut self, world: &mut World);