Skip to content

Commit

Permalink
Remove some superfluous unsafe code (#3297)
Browse files Browse the repository at this point in the history
# Objective

- This `unsafe` is weird

## Solution

- Don't use `unsafe` here

Hopefully this isn't already in an open PR.
  • Loading branch information
DJMcNab committed Dec 11, 2021
1 parent d8abfdb commit 0ee4195
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 18 deletions.
10 changes: 6 additions & 4 deletions crates/bevy_ecs/src/schedule/executor_parallel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,21 +175,23 @@ impl ParallelExecutor {
fn prepare_systems<'scope>(
&mut self,
scope: &mut Scope<'scope, ()>,
systems: &'scope [ParallelSystemContainer],
systems: &'scope mut [ParallelSystemContainer],
world: &'scope World,
) {
#[cfg(feature = "trace")]
let span = bevy_utils::tracing::info_span!("prepare_systems");
#[cfg(feature = "trace")]
let _guard = span.enter();
self.should_run.clear();
for (index, system_data) in self.system_metadata.iter_mut().enumerate() {
for (index, (system_data, system)) in
self.system_metadata.iter_mut().zip(systems).enumerate()
{
// Spawn the system task.
if systems[index].should_run() {
if system.should_run() {
self.should_run.set(index, true);
let start_receiver = system_data.start_receiver.clone();
let finish_sender = self.finish_sender.clone();
let system = unsafe { systems[index].system_mut_unsafe() };
let system = system.system_mut();
#[cfg(feature = "trace")] // NB: outside the task to get the TLS current span
let system_span = bevy_utils::tracing::info_span!("system", name = &*system.name());
let task = async move {
Expand Down
19 changes: 5 additions & 14 deletions crates/bevy_ecs/src/schedule/system_container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
},
system::{ExclusiveSystem, System},
};
use std::{borrow::Cow, cell::UnsafeCell};
use std::borrow::Cow;

/// System metadata like its name, labels, order requirements and component access.
pub trait SystemContainer: GraphNode<Label = BoxedSystemLabel> {
Expand Down Expand Up @@ -106,7 +106,7 @@ impl SystemContainer for ExclusiveSystemContainer {
}

pub struct ParallelSystemContainer {
system: Box<UnsafeCell<dyn System<In = (), Out = ()>>>,
system: Box<dyn System<In = (), Out = ()>>,
pub(crate) run_criteria_index: Option<usize>,
pub(crate) run_criteria_label: Option<BoxedRunCriteriaLabel>,
pub(crate) should_run: bool,
Expand All @@ -123,8 +123,7 @@ unsafe impl Sync for ParallelSystemContainer {}
impl ParallelSystemContainer {
pub(crate) fn from_descriptor(descriptor: ParallelSystemDescriptor) -> Self {
ParallelSystemContainer {
// SAFE: it is fine to wrap inner value with UnsafeCell, as it is repr(transparent)
system: unsafe { Box::from_raw(Box::into_raw(descriptor.system) as *mut _) },
system: descriptor.system,
should_run: false,
run_criteria_index: None,
run_criteria_label: None,
Expand All @@ -141,19 +140,11 @@ impl ParallelSystemContainer {
}

pub fn system(&self) -> &dyn System<In = (), Out = ()> {
// SAFE: statically enforced shared access
unsafe { self.system.get().as_ref().unwrap() }
&*self.system
}

pub fn system_mut(&mut self) -> &mut dyn System<In = (), Out = ()> {
self.system.get_mut()
}

/// # Safety
/// Ensure no other borrows exist along with this one.
#[allow(clippy::mut_from_ref)]
pub unsafe fn system_mut_unsafe(&self) -> &mut dyn System<In = (), Out = ()> {
self.system.get().as_mut().unwrap()
&mut *self.system
}

pub fn should_run(&self) -> bool {
Expand Down

0 comments on commit 0ee4195

Please sign in to comment.