Skip to content

Commit

Permalink
Replace some unsafe system executor code with safe code (#8274)
Browse files Browse the repository at this point in the history
# Objective

The function `SyncUnsafeCell::from_mut` returns `&SyncUnsafeCell<T>`,
even though it could return `&mut SyncUnsafeCell<T>`. This means it is
not possible to call `get_mut` on the returned value, so you need to use
unsafe code to get exclusive access back.

## Solution

Return `&mut Self` instead of `&Self` in `SyncUnsafeCell::from_mut`.
This is consistent with my proposal for `UnsafeCell::from_mut`:
rust-lang/libs-team#198.

Replace an unsafe pointer dereference with a safe call to `get_mut`.

---

## Changelog

+ The function `bevy_utils::SyncUnsafeCell::get_mut` now returns a value
of type `&mut SyncUnsafeCell<T>`. Previously, this returned an immutable
reference.

## Migration Guide

The function `bevy_utils::SyncUnsafeCell::get_mut` now returns a value
of type `&mut SyncUnsafeCell<T>`. Previously, this returned an immutable
reference.
  • Loading branch information
joseph-gio authored Mar 31, 2023
1 parent 711efed commit ae39b07
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
4 changes: 1 addition & 3 deletions crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,8 @@ impl SystemExecutor for MultiThreadedExecutor {

if self.apply_final_buffers {
// Do one final apply buffers after all systems have completed
// SAFETY: all systems have completed, and so no outstanding accesses remain
let world = unsafe { &mut *world.get() };
// Commands should be applied while on the scope's thread, not the executor's thread
apply_system_buffers(&self.unapplied_systems, systems, world);
apply_system_buffers(&self.unapplied_systems, systems, world.get_mut());
self.unapplied_systems.clear();
debug_assert!(self.unapplied_systems.is_clear());
}
Expand Down
12 changes: 7 additions & 5 deletions crates/bevy_utils/src/syncunsafecell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,13 @@ impl<T: ?Sized> SyncUnsafeCell<T> {
}

#[inline]
/// Returns a `&SyncUnsafeCell<T>` from a `&mut T`.
pub fn from_mut(t: &mut T) -> &SyncUnsafeCell<T> {
// SAFETY: `&mut` ensures unique access, and `UnsafeCell<T>` and `SyncUnsafeCell<T>`
// have #[repr(transparent)]
unsafe { &*(t as *mut T as *const SyncUnsafeCell<T>) }
/// Returns a `&mut SyncUnsafeCell<T>` from a `&mut T`.
pub fn from_mut(t: &mut T) -> &mut SyncUnsafeCell<T> {
let ptr = t as *mut T as *mut SyncUnsafeCell<T>;
// SAFETY: `ptr` must be safe to mutably dereference, since it was originally
// obtained from a mutable reference. `SyncUnsafeCell` has the same representation
// as the original type `T`, since the former is annotated with #[repr(transparent)].
unsafe { &mut *ptr }
}
}

Expand Down

0 comments on commit ae39b07

Please sign in to comment.