Skip to content

Commit

Permalink
prevent memory leak when dropping ParallelSystemContainer
Browse files Browse the repository at this point in the history
  • Loading branch information
Frizi committed May 16, 2021
1 parent 883abbb commit 731e3b1
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 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, ptr::NonNull};
use std::{borrow::Cow, cell::UnsafeCell};

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

pub struct ParallelSystemContainer {
system: NonNull<dyn System<In = (), Out = ()>>,
system: Box<UnsafeCell<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 @@ -121,7 +121,8 @@ unsafe impl Sync for ParallelSystemContainer {}
impl ParallelSystemContainer {
pub(crate) fn from_descriptor(descriptor: ParallelSystemDescriptor) -> Self {
ParallelSystemContainer {
system: unsafe { NonNull::new_unchecked(Box::into_raw(descriptor.system)) },
// 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 _) },
should_run: false,
run_criteria_index: None,
run_criteria_label: None,
Expand All @@ -138,20 +139,20 @@ impl ParallelSystemContainer {
}

pub fn system(&self) -> &dyn System<In = (), Out = ()> {
// SAFE: statically enforced shared access.
unsafe { self.system.as_ref() }
let ptr = self.system.as_ref().get();
// SAFE: statically enforced shared access, box never holds a dangling pointer
unsafe { ptr.as_ref().unwrap() }
}

pub fn system_mut(&mut self) -> &mut dyn System<In = (), Out = ()> {
// SAFE: statically enforced exclusive access.
unsafe { self.system.as_mut() }
self.system.as_mut().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 = ()> {
&mut *self.system.as_ptr()
self.system.as_ref().get().as_mut().unwrap()
}

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

0 comments on commit 731e3b1

Please sign in to comment.