Skip to content

Commit

Permalink
unsafeify World::entities_mut (bevyengine#4093)
Browse files Browse the repository at this point in the history
# Objective
make bevy ecs a lil bit less unsound

## Solution
make unsound API unsafe so that there is an unsafe block to blame:

```rust
use bevy_ecs::prelude::*;

#[derive(Debug, Component)]
struct Foo(u8);

fn main() {
    let mut world = World::new();
    let e1 = world.spawn().id();
    let e2 = world.spawn().insert(Foo(2)).id();
    world.entities_mut().meta[0] = world.entities_mut().meta[1].clone();
    let foo = world.entity(e1).get::<Foo>().unwrap();
    // whoo i love having components i dont have
    dbg!(foo);
}
```

This is not _strictly_ speaking UB, however: 
- `Query::get_multiple` cannot work if this is allowed
- bevy_ecs is a pile of unsafe code whose soundness generally depends on the world being in a "correct" state with "no funny business" so it seems best to disallow this
- it is trivial to get bevy to panic inside of functions with safety invariants that have been violated (the entity location is not valid)
- it seems to violate what the safety invariant on `Entities::flush` is trying to ensure
  • Loading branch information
BoxyUwU authored and ItsDoot committed Feb 1, 2023
1 parent 7fdc6c7 commit 7700acb
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 4 deletions.
8 changes: 7 additions & 1 deletion crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl<'a> core::iter::ExactSizeIterator for ReserveEntitiesIterator<'a> {}

#[derive(Debug, Default)]
pub struct Entities {
pub meta: Vec<EntityMeta>,
pub(crate) meta: Vec<EntityMeta>,

/// The `pending` and `free_cursor` fields describe three sets of Entity IDs
/// that have been freed or are in the process of being allocated:
Expand Down Expand Up @@ -544,6 +544,12 @@ impl Entities {
}
}

/// Accessor for getting the length of the vec in `self.meta`
#[inline]
pub fn meta_len(&self) -> usize {
self.meta.len()
}

#[inline]
pub fn len(&self) -> u32 {
self.len
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,12 @@ impl World {
}

/// Retrieves this world's [Entities] collection mutably
///
/// # Safety
/// Mutable reference must not be used to put the [`Entities`] data
/// in an invalid state for this [`World`]
#[inline]
pub fn entities_mut(&mut self) -> &mut Entities {
pub unsafe fn entities_mut(&mut self) -> &mut Entities {
&mut self.entities
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl Plugin for RenderPlugin {

// reserve all existing app entities for use in render_app
// they can only be spawned using `get_or_spawn()`
let meta_len = app_world.entities().meta.len();
let meta_len = app_world.entities().meta_len();
render_app
.world
.entities()
Expand All @@ -198,7 +198,7 @@ impl Plugin for RenderPlugin {
// flushing as "invalid" ensures that app world entities aren't added as "empty archetype" entities by default
// these entities cannot be accessed without spawning directly onto them
// this _only_ works as expected because clear_entities() is called at the end of every frame.
render_app.world.entities_mut().flush_as_invalid();
unsafe { render_app.world.entities_mut() }.flush_as_invalid();
}

{
Expand Down

0 comments on commit 7700acb

Please sign in to comment.