fix mutable aliases for a very short time if WorldCell
is already borrowed (#6639)
# Objective Consider the test ```rust let cell = world.cell(); let _value_a = cell.resource_mut::<A>(); let _value_b = cell.resource_mut::<A>(); ``` Currently, this will roughly execute ```rust // first call let value = unsafe { self.world .get_non_send_unchecked_mut_with_id(component_id)? }; return Some(WorldBorrowMut::new(value, archetype_component_id, self.access))) // second call let value = unsafe { self.world .get_non_send_unchecked_mut_with_id(component_id)? }; return Some(WorldBorrowMut::new(value, archetype_component_id, self.access))) ``` where `WorldBorrowMut::new` will panic if the resource is already borrowed. This means, that `_value_a` will be created, the access checked (OK), then `value_b` will be created, and the access checked (`panic`). For a moment, both `_value_a` and `_value_b` existed as `&mut T` to the same location, which is insta-UB as far as I understand it. ## Solution Flip the order so that `WorldBorrowMut::new` first checks the access, _then_ fetches creates the value. To do that, we pass a `impl FnOnce() -> Mut<T>` instead of the `Mut<T>` directly: ```rust let get_value = || unsafe { self.world .get_non_send_unchecked_mut_with_id(component_id)? }; return Some(WorldBorrowMut::new(get_value, archetype_component_id, self.access))) ```
This commit is contained in:
parent
d44e86507f
commit
cf46dd2e7e
@ -88,21 +88,22 @@ pub struct WorldBorrow<'w, T> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl<'w, T> WorldBorrow<'w, T> {
|
impl<'w, T> WorldBorrow<'w, T> {
|
||||||
fn new(
|
fn try_new(
|
||||||
value: &'w T,
|
value: impl FnOnce() -> Option<&'w T>,
|
||||||
archetype_component_id: ArchetypeComponentId,
|
archetype_component_id: ArchetypeComponentId,
|
||||||
access: Rc<RefCell<ArchetypeComponentAccess>>,
|
access: Rc<RefCell<ArchetypeComponentAccess>>,
|
||||||
) -> Self {
|
) -> Option<Self> {
|
||||||
assert!(
|
assert!(
|
||||||
access.borrow_mut().read(archetype_component_id),
|
access.borrow_mut().read(archetype_component_id),
|
||||||
"Attempted to immutably access {}, but it is already mutably borrowed",
|
"Attempted to immutably access {}, but it is already mutably borrowed",
|
||||||
std::any::type_name::<T>(),
|
std::any::type_name::<T>(),
|
||||||
);
|
);
|
||||||
Self {
|
let value = value()?;
|
||||||
|
Some(Self {
|
||||||
value,
|
value,
|
||||||
archetype_component_id,
|
archetype_component_id,
|
||||||
access,
|
access,
|
||||||
}
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -129,21 +130,22 @@ pub struct WorldBorrowMut<'w, T> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl<'w, T> WorldBorrowMut<'w, T> {
|
impl<'w, T> WorldBorrowMut<'w, T> {
|
||||||
fn new(
|
fn try_new(
|
||||||
value: Mut<'w, T>,
|
value: impl FnOnce() -> Option<Mut<'w, T>>,
|
||||||
archetype_component_id: ArchetypeComponentId,
|
archetype_component_id: ArchetypeComponentId,
|
||||||
access: Rc<RefCell<ArchetypeComponentAccess>>,
|
access: Rc<RefCell<ArchetypeComponentAccess>>,
|
||||||
) -> Self {
|
) -> Option<Self> {
|
||||||
assert!(
|
assert!(
|
||||||
access.borrow_mut().write(archetype_component_id),
|
access.borrow_mut().write(archetype_component_id),
|
||||||
"Attempted to mutably access {}, but it is already mutably borrowed",
|
"Attempted to mutably access {}, but it is already mutably borrowed",
|
||||||
std::any::type_name::<T>(),
|
std::any::type_name::<T>(),
|
||||||
);
|
);
|
||||||
Self {
|
let value = value()?;
|
||||||
|
Some(Self {
|
||||||
value,
|
value,
|
||||||
archetype_component_id,
|
archetype_component_id,
|
||||||
access,
|
access,
|
||||||
}
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -189,12 +191,12 @@ impl<'w> WorldCell<'w> {
|
|||||||
let archetype_component_id = self
|
let archetype_component_id = self
|
||||||
.world
|
.world
|
||||||
.get_resource_archetype_component_id(component_id)?;
|
.get_resource_archetype_component_id(component_id)?;
|
||||||
Some(WorldBorrow::new(
|
WorldBorrow::try_new(
|
||||||
// SAFETY: ComponentId matches TypeId
|
// SAFETY: ComponentId matches TypeId
|
||||||
unsafe { self.world.get_resource_with_id(component_id)? },
|
|| unsafe { self.world.get_resource_with_id(component_id) },
|
||||||
archetype_component_id,
|
archetype_component_id,
|
||||||
self.access.clone(),
|
self.access.clone(),
|
||||||
))
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Gets a reference to the resource of the given type
|
/// Gets a reference to the resource of the given type
|
||||||
@ -222,15 +224,12 @@ impl<'w> WorldCell<'w> {
|
|||||||
let archetype_component_id = self
|
let archetype_component_id = self
|
||||||
.world
|
.world
|
||||||
.get_resource_archetype_component_id(component_id)?;
|
.get_resource_archetype_component_id(component_id)?;
|
||||||
Some(WorldBorrowMut::new(
|
WorldBorrowMut::try_new(
|
||||||
// SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut
|
// SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut
|
||||||
unsafe {
|
|| unsafe { self.world.get_resource_unchecked_mut_with_id(component_id) },
|
||||||
self.world
|
|
||||||
.get_resource_unchecked_mut_with_id(component_id)?
|
|
||||||
},
|
|
||||||
archetype_component_id,
|
archetype_component_id,
|
||||||
self.access.clone(),
|
self.access.clone(),
|
||||||
))
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Gets a mutable reference to the resource of the given type
|
/// Gets a mutable reference to the resource of the given type
|
||||||
@ -258,12 +257,12 @@ impl<'w> WorldCell<'w> {
|
|||||||
let archetype_component_id = self
|
let archetype_component_id = self
|
||||||
.world
|
.world
|
||||||
.get_resource_archetype_component_id(component_id)?;
|
.get_resource_archetype_component_id(component_id)?;
|
||||||
Some(WorldBorrow::new(
|
WorldBorrow::try_new(
|
||||||
// SAFETY: ComponentId matches TypeId
|
// SAFETY: ComponentId matches TypeId
|
||||||
unsafe { self.world.get_non_send_with_id(component_id)? },
|
|| unsafe { self.world.get_non_send_with_id(component_id) },
|
||||||
archetype_component_id,
|
archetype_component_id,
|
||||||
self.access.clone(),
|
self.access.clone(),
|
||||||
))
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Gets an immutable reference to the non-send resource of the given type, if it exists.
|
/// Gets an immutable reference to the non-send resource of the given type, if it exists.
|
||||||
@ -291,15 +290,12 @@ impl<'w> WorldCell<'w> {
|
|||||||
let archetype_component_id = self
|
let archetype_component_id = self
|
||||||
.world
|
.world
|
||||||
.get_resource_archetype_component_id(component_id)?;
|
.get_resource_archetype_component_id(component_id)?;
|
||||||
Some(WorldBorrowMut::new(
|
WorldBorrowMut::try_new(
|
||||||
// SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut
|
// SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut
|
||||||
unsafe {
|
|| unsafe { self.world.get_non_send_unchecked_mut_with_id(component_id) },
|
||||||
self.world
|
|
||||||
.get_non_send_unchecked_mut_with_id(component_id)?
|
|
||||||
},
|
|
||||||
archetype_component_id,
|
archetype_component_id,
|
||||||
self.access.clone(),
|
self.access.clone(),
|
||||||
))
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Gets a mutable reference to the non-send resource of the given type, if it exists.
|
/// Gets a mutable reference to the non-send resource of the given type, if it exists.
|
||||||
|
Loading…
Reference in New Issue
Block a user