From d144763734f1c6cadd237ade732b08b9001e1aa3 Mon Sep 17 00:00:00 2001
From: PROMETHIA-27 <electriccobras@gmail.com>
Date: Mon, 12 Sep 2022 04:15:55 +0000
Subject: [PATCH] Remove `Sync` bound from `Local` (#5483)

# Objective

Currently, `Local` has a `Sync` bound. Theoretically this is unnecessary as a local can only ever be accessed from its own system, ensuring exclusive access on one thread. This PR removes this restriction.

## Solution

- By removing the `Resource` bound from `Local` and adding the new `SyncCell` threading primative, `Local` can have the `Sync` bound removed.

## Changelog

### Added

- Added `SyncCell` to `bevy_utils`

### Changed

- Removed `Resource` bound from `Local`
- `Local` is now wrapped in a `SyncCell`

## Migration Guide

- Any code relying on `Local<T>` having `T: Resource` may have to be changed, but this is unlikely.

Co-authored-by: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com>
---
 crates/bevy_ecs/src/system/system_param.rs | 17 +++++-----
 crates/bevy_utils/src/lib.rs               |  1 +
 crates/bevy_utils/src/synccell.rs          | 39 ++++++++++++++++++++++
 3 files changed, 49 insertions(+), 8 deletions(-)
 create mode 100644 crates/bevy_utils/src/synccell.rs

diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs
index fa6ad3f26fb7b3..ef956c25d553e3 100644
--- a/crates/bevy_ecs/src/system/system_param.rs
+++ b/crates/bevy_ecs/src/system/system_param.rs
@@ -15,6 +15,7 @@ pub use bevy_ecs_macros::Resource;
 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::{
     borrow::Cow,
     fmt::Debug,
@@ -674,10 +675,10 @@ 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: FromWorld + Send + Sync + 'static>(&'a mut T);
+pub struct Local<'a, T: FromWorld + Send + 'static>(&'a mut T);
 
 // SAFETY: Local only accesses internal state
-unsafe impl<T: Send + Sync + 'static> ReadOnlySystemParamFetch for LocalState<T> {}
+unsafe impl<T: Send + 'static> ReadOnlySystemParamFetch for LocalState<T> {}
 
 impl<'a, T: FromWorld + Send + Sync + 'static> Debug for Local<'a, T>
 where
@@ -706,20 +707,20 @@ impl<'a, T: FromWorld + Send + Sync + 'static> DerefMut for Local<'a, T> {
 
 /// The [`SystemParamState`] of [`Local<T>`].
 #[doc(hidden)]
-pub struct LocalState<T: Send + Sync + 'static>(T);
+pub struct LocalState<T: Send + 'static>(SyncCell<T>);
 
-impl<'a, T: Send + Sync + 'static + FromWorld> SystemParam for Local<'a, T> {
+impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> {
     type Fetch = LocalState<T>;
 }
 
 // SAFETY: only local state is accessed
-unsafe impl<T: FromWorld + Send + Sync + 'static> SystemParamState for LocalState<T> {
+unsafe impl<T: FromWorld + Send + 'static> SystemParamState for LocalState<T> {
     fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self {
-        Self(T::from_world(world))
+        Self(SyncCell::new(T::from_world(world)))
     }
 }
 
-impl<'w, 's, T: Send + Sync + 'static + FromWorld> SystemParamFetch<'w, 's> for LocalState<T> {
+impl<'w, 's, T: FromWorld + Send + 'static> SystemParamFetch<'w, 's> for LocalState<T> {
     type Item = Local<'s, T>;
 
     #[inline]
@@ -729,7 +730,7 @@ impl<'w, 's, T: Send + Sync + 'static + FromWorld> SystemParamFetch<'w, 's> for
         _world: &'w World,
         _change_tick: u32,
     ) -> Self::Item {
-        Local(&mut state.0)
+        Local(state.0.get())
     }
 }
 
diff --git a/crates/bevy_utils/src/lib.rs b/crates/bevy_utils/src/lib.rs
index 2b09fdbf0f3b28..d942ad4feda4d6 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 00000000000000..838915fe2500a3
--- /dev/null
+++ b/crates/bevy_utils/src/synccell.rs
@@ -0,0 +1,39 @@
+/// 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<T: ?Sized> {
+    inner: T,
+}
+
+impl<T: Sized> SyncCell<T> {
+    /// 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<T: ?Sized> SyncCell<T> {
+    /// 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<T> {
+        // 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<T>) }
+    }
+}
+
+// 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<T: ?Sized> Sync for SyncCell<T> {}