From 58915533b5eb0cc8cfd353d89b3eae416769a8a1 Mon Sep 17 00:00:00 2001 From: PROMETHIA Date: Fri, 29 Jul 2022 10:13:50 -0400 Subject: [PATCH 1/6] Impl `SyncCell` --- crates/bevy_utils/src/lib.rs | 1 + crates/bevy_utils/src/synccell.rs | 36 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 crates/bevy_utils/src/synccell.rs diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs index 2b09fdbf0f3b2..d942ad4feda4d 100644 --- a/crates/bevy_utils/src/lib.rs +++ b/crates/bevy_utils/src/lib.rs @@ -6,6 +6,7 @@ pub mod futures; pub mod label; mod short_names; pub use short_names::get_short_name; +pub mod synccell; mod default; mod float_ord; diff --git a/crates/bevy_utils/src/synccell.rs b/crates/bevy_utils/src/synccell.rs new file mode 100644 index 0000000000000..a90ff5d8becd3 --- /dev/null +++ b/crates/bevy_utils/src/synccell.rs @@ -0,0 +1,36 @@ +/// See [`Exclusive`](https://github.com/rust-lang/rust/issues/98407) for stdlib's upcoming implementation, +/// which should replace this one entirely. +/// +/// Provides a wrapper that allows making any type unconditionally [`Sync`] by only providing mutable access. +#[repr(transparent)] +pub struct SyncCell { + inner: T, +} + +impl SyncCell { + /// Construct a new instance of a `SyncCell` from the given value. + pub fn new(inner: T) -> Self { + Self { inner } + } + + /// Deconstruct this `SyncCell` into its inner value. + pub fn to_inner(Self { inner }: Self) -> T { + inner + } +} + +impl SyncCell { + /// Get a reference to this `SyncCell`'s inner value. + pub fn get(&mut self) -> &mut T { + &mut self.inner + } + + /// Build a mutable reference to a `SyncCell` from a mutable reference + /// to its inner value, to skip constructing with [`new()`](SyncCell::new()). + pub fn from_mut(r: &'_ mut T) -> &'_ mut SyncCell { + // SAFETY: repr is ≥ C, so refs have the same layout; and `SyncCell` properties are `&mut`-agnostic + unsafe { &mut *(r as *mut T as *mut SyncCell) } + } +} + +unsafe impl Sync for SyncCell {} From 37817952bab95b4ba53df5d966c249e95b8d286c Mon Sep 17 00:00:00 2001 From: PROMETHIA Date: Fri, 29 Jul 2022 10:14:12 -0400 Subject: [PATCH 2/6] De-`Sync`'d `Resource` and `SyncCell`'d `Local` --- crates/bevy_ecs/src/system/commands/mod.rs | 12 ++++---- crates/bevy_ecs/src/system/system_param.rs | 35 +++++++++++----------- crates/bevy_render/src/extract_resource.rs | 4 +-- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 1521eebe529da..ac5b4d78133c1 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -342,7 +342,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(initialise_scoreboard); /// ``` - pub fn init_resource(&mut self) { + pub fn init_resource(&mut self) { self.queue.push(InitResource:: { _phantom: PhantomData::::default(), }); @@ -372,7 +372,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(system); /// ``` - pub fn insert_resource(&mut self, resource: R) { + pub fn insert_resource(&mut self, resource: R) { self.queue.push(InsertResource { resource }); } @@ -395,7 +395,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(system); /// ``` - pub fn remove_resource(&mut self) { + pub fn remove_resource(&mut self) { self.queue.push(RemoveResource:: { phantom: PhantomData, }); @@ -806,7 +806,7 @@ pub struct InitResource { _phantom: PhantomData, } -impl Command for InitResource { +impl Command for InitResource { fn write(self, world: &mut World) { world.init_resource::(); } @@ -816,7 +816,7 @@ pub struct InsertResource { pub resource: R, } -impl Command for InsertResource { +impl Command for InsertResource { fn write(self, world: &mut World) { world.insert_resource(self.resource); } @@ -826,7 +826,7 @@ pub struct RemoveResource { pub phantom: PhantomData, } -impl Command for RemoveResource { +impl Command for RemoveResource { fn write(self, world: &mut World) { world.remove_resource::(); } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index c3a419f0a8e20..e55ad4bdfe81e 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -14,6 +14,7 @@ use crate::{ pub use bevy_ecs_macros::SystemParam; use bevy_ecs_macros::{all_tuples, impl_param_set}; use bevy_ptr::UnsafeCellDeref; +use bevy_utils::synccell::SyncCell; use std::{ fmt::Debug, marker::PhantomData, @@ -217,9 +218,9 @@ pub struct ParamSetState SystemParamFetch<'w, 's>>(T); impl_param_set!(); -pub trait Resource: Send + Sync + 'static {} +pub trait Resource: Send + 'static {} -impl Resource for T where T: Send + Sync + 'static {} +impl Resource for T where T: Send + 'static {} /// Shared borrow of a resource. /// @@ -301,13 +302,13 @@ pub struct ResState { marker: PhantomData, } -impl<'a, T: Resource> SystemParam for Res<'a, T> { +impl<'a, T: Resource + Sync> SystemParam for Res<'a, T> { type Fetch = ResState; } // SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for ResState { +unsafe impl SystemParamState for ResState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { let component_id = world.initialize_resource::(); let combined_access = system_meta.component_access_set.combined_access_mut(); @@ -333,7 +334,7 @@ unsafe impl SystemParamState for ResState { } } -impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { +impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for ResState { type Item = Res<'w, T>; #[inline] @@ -366,7 +367,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { #[doc(hidden)] pub struct OptionResState(ResState); -impl<'a, T: Resource> SystemParam for Option> { +impl<'a, T: Resource + Sync> SystemParam for Option> { type Fetch = OptionResState; } @@ -375,13 +376,13 @@ unsafe impl ReadOnlySystemParamFetch for OptionResState {} // SAFETY: this impl defers to `ResState`, which initializes // and validates the correct world access -unsafe impl SystemParamState for OptionResState { +unsafe impl SystemParamState for OptionResState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(ResState::init(world, system_meta)) } } -impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResState { +impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for OptionResState { type Item = Option>; #[inline] @@ -409,13 +410,13 @@ pub struct ResMutState { marker: PhantomData, } -impl<'a, T: Resource> SystemParam for ResMut<'a, T> { +impl<'a, T: Resource + Sync> SystemParam for ResMut<'a, T> { type Fetch = ResMutState; } // SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for ResMutState { +unsafe impl SystemParamState for ResMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { let component_id = world.initialize_resource::(); let combined_access = system_meta.component_access_set.combined_access_mut(); @@ -444,7 +445,7 @@ unsafe impl SystemParamState for ResMutState { } } -impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResMutState { +impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for ResMutState { type Item = ResMut<'w, T>; #[inline] @@ -479,19 +480,19 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResMutState { #[doc(hidden)] pub struct OptionResMutState(ResMutState); -impl<'a, T: Resource> SystemParam for Option> { +impl<'a, T: Resource + Sync> SystemParam for Option> { type Fetch = OptionResMutState; } // SAFETY: this impl defers to `ResMutState`, which initializes // and validates the correct world access -unsafe impl SystemParamState for OptionResMutState { +unsafe impl SystemParamState for OptionResMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(ResMutState::init(world, system_meta)) } } -impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResMutState { +impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for OptionResMutState { type Item = Option>; #[inline] @@ -672,7 +673,7 @@ impl<'a, T: Resource> DerefMut for Local<'a, T> { /// The [`SystemParamState`] of [`Local`]. #[doc(hidden)] -pub struct LocalState(T); +pub struct LocalState(SyncCell); impl<'a, T: Resource + FromWorld> SystemParam for Local<'a, T> { type Fetch = LocalState; @@ -681,7 +682,7 @@ impl<'a, T: Resource + FromWorld> SystemParam for Local<'a, T> { // SAFETY: only local state is accessed unsafe impl SystemParamState for LocalState { fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self(T::from_world(world)) + Self(SyncCell::new(T::from_world(world))) } } @@ -695,7 +696,7 @@ impl<'w, 's, T: Resource + FromWorld> SystemParamFetch<'w, 's> for LocalState _world: &'w World, _change_tick: u32, ) -> Self::Item { - Local(&mut state.0) + Local(state.0.get()) } } diff --git a/crates/bevy_render/src/extract_resource.rs b/crates/bevy_render/src/extract_resource.rs index 5201021bd4fe0..7262ea4187950 100644 --- a/crates/bevy_render/src/extract_resource.rs +++ b/crates/bevy_render/src/extract_resource.rs @@ -12,8 +12,8 @@ use crate::{Extract, RenderApp, RenderStage}; /// /// Therefore the resource is transferred from the "main world" into the "render world" /// in the [`RenderStage::Extract`](crate::RenderStage::Extract) step. -pub trait ExtractResource: Resource { - type Source: Resource; +pub trait ExtractResource: Resource + Sync { + type Source: Resource + Sync; /// Defines how the resource is transferred into the "render world". fn extract_resource(source: &Self::Source) -> Self; From 8e6a936364dc51e85caf5888bfb8196e2cf0ebfb Mon Sep 17 00:00:00 2001 From: PROMETHIA Date: Fri, 29 Jul 2022 10:23:54 -0400 Subject: [PATCH 3/6] Remove `Resource` bound from `Local` add `Sync` bound back to `Resource` --- crates/bevy_ecs/src/system/commands/mod.rs | 12 ++++---- crates/bevy_ecs/src/system/system_param.rs | 32 +++++++++++----------- crates/bevy_render/src/extract_resource.rs | 4 +-- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index ac5b4d78133c1..1521eebe529da 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -342,7 +342,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(initialise_scoreboard); /// ``` - pub fn init_resource(&mut self) { + pub fn init_resource(&mut self) { self.queue.push(InitResource:: { _phantom: PhantomData::::default(), }); @@ -372,7 +372,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(system); /// ``` - pub fn insert_resource(&mut self, resource: R) { + pub fn insert_resource(&mut self, resource: R) { self.queue.push(InsertResource { resource }); } @@ -395,7 +395,7 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(system); /// ``` - pub fn remove_resource(&mut self) { + pub fn remove_resource(&mut self) { self.queue.push(RemoveResource:: { phantom: PhantomData, }); @@ -806,7 +806,7 @@ pub struct InitResource { _phantom: PhantomData, } -impl Command for InitResource { +impl Command for InitResource { fn write(self, world: &mut World) { world.init_resource::(); } @@ -816,7 +816,7 @@ pub struct InsertResource { pub resource: R, } -impl Command for InsertResource { +impl Command for InsertResource { fn write(self, world: &mut World) { world.insert_resource(self.resource); } @@ -826,7 +826,7 @@ pub struct RemoveResource { pub phantom: PhantomData, } -impl Command for RemoveResource { +impl Command for RemoveResource { fn write(self, world: &mut World) { world.remove_resource::(); } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index e55ad4bdfe81e..b92e39a4326c0 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -218,9 +218,9 @@ pub struct ParamSetState SystemParamFetch<'w, 's>>(T); impl_param_set!(); -pub trait Resource: Send + 'static {} +pub trait Resource: Send + Sync + 'static {} -impl Resource for T where T: Send + 'static {} +impl Resource for T where T: Send + Sync + 'static {} /// Shared borrow of a resource. /// @@ -302,13 +302,13 @@ pub struct ResState { marker: PhantomData, } -impl<'a, T: Resource + Sync> SystemParam for Res<'a, T> { +impl<'a, T: Resource> SystemParam for Res<'a, T> { type Fetch = ResState; } // SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for ResState { +unsafe impl SystemParamState for ResState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { let component_id = world.initialize_resource::(); let combined_access = system_meta.component_access_set.combined_access_mut(); @@ -334,7 +334,7 @@ unsafe impl SystemParamState for ResState { } } -impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for ResState { +impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { type Item = Res<'w, T>; #[inline] @@ -367,7 +367,7 @@ impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for ResState { #[doc(hidden)] pub struct OptionResState(ResState); -impl<'a, T: Resource + Sync> SystemParam for Option> { +impl<'a, T: Resource> SystemParam for Option> { type Fetch = OptionResState; } @@ -376,13 +376,13 @@ unsafe impl ReadOnlySystemParamFetch for OptionResState {} // SAFETY: this impl defers to `ResState`, which initializes // and validates the correct world access -unsafe impl SystemParamState for OptionResState { +unsafe impl SystemParamState for OptionResState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(ResState::init(world, system_meta)) } } -impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for OptionResState { +impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResState { type Item = Option>; #[inline] @@ -410,13 +410,13 @@ pub struct ResMutState { marker: PhantomData, } -impl<'a, T: Resource + Sync> SystemParam for ResMut<'a, T> { +impl<'a, T: Resource> SystemParam for ResMut<'a, T> { type Fetch = ResMutState; } // SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for ResMutState { +unsafe impl SystemParamState for ResMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { let component_id = world.initialize_resource::(); let combined_access = system_meta.component_access_set.combined_access_mut(); @@ -445,7 +445,7 @@ unsafe impl SystemParamState for ResMutState { } } -impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for ResMutState { +impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResMutState { type Item = ResMut<'w, T>; #[inline] @@ -480,19 +480,19 @@ impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for ResMutState { #[doc(hidden)] pub struct OptionResMutState(ResMutState); -impl<'a, T: Resource + Sync> SystemParam for Option> { +impl<'a, T: Resource> SystemParam for Option> { type Fetch = OptionResMutState; } // SAFETY: this impl defers to `ResMutState`, which initializes // and validates the correct world access -unsafe impl SystemParamState for OptionResMutState { +unsafe impl SystemParamState for OptionResMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(ResMutState::init(world, system_meta)) } } -impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for OptionResMutState { +impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResMutState { type Item = Option>; #[inline] @@ -641,7 +641,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for WorldState { /// // .add_system(reset_to_system(my_config)) /// # assert_is_system(reset_to_system(Config(10))); /// ``` -pub struct Local<'a, T: Resource>(&'a mut T); +pub struct Local<'a, T: Send + 'static>(&'a mut T); // SAFETY: Local only accesses internal state unsafe impl ReadOnlySystemParamFetch for LocalState {} @@ -673,7 +673,7 @@ impl<'a, T: Resource> DerefMut for Local<'a, T> { /// The [`SystemParamState`] of [`Local`]. #[doc(hidden)] -pub struct LocalState(SyncCell); +pub struct LocalState(SyncCell); impl<'a, T: Resource + FromWorld> SystemParam for Local<'a, T> { type Fetch = LocalState; diff --git a/crates/bevy_render/src/extract_resource.rs b/crates/bevy_render/src/extract_resource.rs index 7262ea4187950..5201021bd4fe0 100644 --- a/crates/bevy_render/src/extract_resource.rs +++ b/crates/bevy_render/src/extract_resource.rs @@ -12,8 +12,8 @@ use crate::{Extract, RenderApp, RenderStage}; /// /// Therefore the resource is transferred from the "main world" into the "render world" /// in the [`RenderStage::Extract`](crate::RenderStage::Extract) step. -pub trait ExtractResource: Resource + Sync { - type Source: Resource + Sync; +pub trait ExtractResource: Resource { + type Source: Resource; /// Defines how the resource is transferred into the "render world". fn extract_resource(source: &Self::Source) -> Self; From e90e0d041f17ea639be5fa7c8c09e67884f1f62c Mon Sep 17 00:00:00 2001 From: PROMETHIA Date: Fri, 29 Jul 2022 10:26:10 -0400 Subject: [PATCH 4/6] Finish removing `Resource` from `Local` --- crates/bevy_ecs/src/system/system_param.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index b92e39a4326c0..346072f179eb6 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -644,7 +644,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for WorldState { pub struct Local<'a, T: Send + 'static>(&'a mut T); // SAFETY: Local only accesses internal state -unsafe impl ReadOnlySystemParamFetch for LocalState {} +unsafe impl ReadOnlySystemParamFetch for LocalState {} impl<'a, T: Resource> Debug for Local<'a, T> where @@ -675,18 +675,18 @@ impl<'a, T: Resource> DerefMut for Local<'a, T> { #[doc(hidden)] pub struct LocalState(SyncCell); -impl<'a, T: Resource + FromWorld> SystemParam for Local<'a, T> { +impl<'a, T: Send + FromWorld + 'static> SystemParam for Local<'a, T> { type Fetch = LocalState; } // SAFETY: only local state is accessed -unsafe impl SystemParamState for LocalState { +unsafe impl SystemParamState for LocalState { fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { Self(SyncCell::new(T::from_world(world))) } } -impl<'w, 's, T: Resource + FromWorld> SystemParamFetch<'w, 's> for LocalState { +impl<'w, 's, T: Send + FromWorld + 'static> SystemParamFetch<'w, 's> for LocalState { type Item = Local<'s, T>; #[inline] From 3bbd274588ee308d980367616d2824fcb1ea5ba6 Mon Sep 17 00:00:00 2001 From: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com> Date: Tue, 16 Aug 2022 08:08:06 -0400 Subject: [PATCH 5/6] >= C -> transparent --- crates/bevy_utils/src/synccell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_utils/src/synccell.rs b/crates/bevy_utils/src/synccell.rs index a90ff5d8becd3..7aef69c15f2e8 100644 --- a/crates/bevy_utils/src/synccell.rs +++ b/crates/bevy_utils/src/synccell.rs @@ -28,7 +28,7 @@ impl SyncCell { /// Build a mutable reference to a `SyncCell` from a mutable reference /// to its inner value, to skip constructing with [`new()`](SyncCell::new()). pub fn from_mut(r: &'_ mut T) -> &'_ mut SyncCell { - // SAFETY: repr is ≥ C, so refs have the same layout; and `SyncCell` properties are `&mut`-agnostic + // SAFETY: repr is transparent, so refs have the same layout; and `SyncCell` properties are `&mut`-agnostic unsafe { &mut *(r as *mut T as *mut SyncCell) } } } From e1e71c09b3a3fbf4af7aa0929487f0d1cb7ce3f4 Mon Sep 17 00:00:00 2001 From: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com> Date: Tue, 16 Aug 2022 08:09:01 -0400 Subject: [PATCH 6/6] Safety comment Co-authored-by: JoJoJet --- crates/bevy_utils/src/synccell.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_utils/src/synccell.rs b/crates/bevy_utils/src/synccell.rs index 7aef69c15f2e8..838915fe2500a 100644 --- a/crates/bevy_utils/src/synccell.rs +++ b/crates/bevy_utils/src/synccell.rs @@ -33,4 +33,7 @@ impl SyncCell { } } +// SAFETY: `Sync` only allows multithreaded access via immutable reference. +// As `SyncCell` requires an exclusive reference to access the wrapped value, +// marking this type as `Sync` does not actually allow threaded access to the inner value. unsafe impl Sync for SyncCell {}