Panic when borrowing different components of the same type from different entities

[rotoclone]

It seems that if you have multiple entities that all have the same types of components (I believe this is referred to as an archetype), you can only mutably borrow one of those components across all the entities at once.

A minimal example:

let mut world = World::new();

let entity_1 = world.spawn((1,));
let entity_2 = world.spawn((2,));

let component_1 = world.get::<&mut i32>(entity_1).unwrap();
let component_2 = world.get::<&mut i32>(entity_2).unwrap(); // panic here: 'i32 already borrowed'

The i32 components are totally separate, so it seems weird to not be allowed to have mutable references to both of them at once.

I believe the panic is coming from here: https://github.com/Ralith/hecs/blob/9c08031ccea2eddb02d2bb14d73b8db35a829d29/src/archetype.rs#L130-L136

Here's the relevant part of the backtrace if it's helpful:

thread 'main' panicked at 'i32 already borrowed', <snip>\src\github.com-1ecc6299db9ec823\hecs-0.9.0\src\archetype.rs:134:13
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library\std\src\panicking.rs:584
   1: core::panicking::panic_fmt
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library\core\src\panicking.rs:142
   2: hecs::archetype::Archetype::borrow_mut<i32>
             at <snip>\src\github.com-1ecc6299db9ec823\hecs-0.9.0\src\archetype.rs:134
   3: hecs::entity_ref::RefMut<i32>::new<i32>
             at <snip>\src\github.com-1ecc6299db9ec823\hecs-0.9.0\src\entity_ref.rs:172
   4: hecs::entity_ref::impl$15::get_component<i32>
             at <snip>\src\github.com-1ecc6299db9ec823\hecs-0.9.0\src\entity_ref.rs:264
   5: hecs::entity_ref::EntityRef::get<ref_mut$<i32> >
             at <snip>\src\github.com-1ecc6299db9ec823\hecs-0.9.0\src\entity_ref.rs:63
   6: hecs::world::World::get<ref_mut$<i32> >
             at <snip>\src\github.com-1ecc6299db9ec823\hecs-0.9.0\src\world.rs:475

In contrast, this doesn't panic:

let mut world = World::new();

let entity_1 = world.spawn((1, true)); // <-- added boolean
let entity_2 = world.spawn((2,));

let component_1 = world.get::<&mut i32>(entity_1).unwrap();
let component_2 = world.get::<&mut i32>(entity_2).unwrap();

I think that's because the first entity has a different archetype due to the boolean component.

And manually dropping the first borrowed component also avoids the panic:

let mut world = World::new();

let entity_1 = world.spawn((1,));
let entity_2 = world.spawn((2,));

let component_1 = world.get::<&mut i32>(entity_1).unwrap();
drop(component_1); // <-- added this
let component_2 = world.get::<&mut i32>(entity_2).unwrap();

I'm not sure if this can be fixed without a big architecture change, but it would be nice to not have to worry about this when messing with components via random access.

[adamreichold]

Borrow tracking happens on a per-archetype level meaning borrow flags exist only for each archetype, not for each entity as this is much more efficient.

If you need more granular access, interior mutability meaning &Cell<i32> might work and there is get_mut_n which checks that entity_1 and entity_2 are distinct at runtime to allow mutable concurrent access.

[Ralith]

@adamreichold is correct about the behavior of dynamic borrowchecking here, but there's a subtlety:

you can only mutably borrow one of those components across all the entities at once.

This is not true, you just have to use a different interface that exists to solve this problem. Heavily repeated random access is best accomplished through a View, which can be obtained from a query via e.g. QueryBorrow::view. This allows you to borrow all of the components that match your query, and then split that borrow further.

[adamreichold]

This is not true, you just have to use a different interface that exists to solve this problem.

Sorry for xkcd #386, but even a view will not give concurrent mutable access to the components of multiple entities, expect for the mentioned get_mut_n API. A view makes it efficient to access the same component of multiple entities one after the other, but View::get_mut is strictly as powerful as repeated calls to World::get.

But I noticed that hecs::View does allow that, but I think this only because I made a mistake when porting it from rs-ecs to hecs: The lifetime of the returned QueryItem must be 'm of the borrow of the view, not 'q of the query.

As it is currently implemented, this allows breaking aliasing discipline, e.g.

use hecs::World;

fn main() {
    let mut world = World::new();

    let entity_1 = world.spawn((1,));

    let mut query = world.query_mut::<&mut i32>();
    let mut view = query.view();

    let component_1: &mut i32 = view.get_mut(entity_1).unwrap();
    let component_2: &mut i32 = view.get_mut(entity_1).unwrap();
    assert!(std::ptr::eq(component_1, component_2));
}

[Ralith]

expect for the mentioned get_mut_n API

That API was the point, yeah.

But I noticed that hecs::View does allow that, but I think this only because I made a mistake when porting it from rs-ecs to hecs: The lifetime of the returned QueryItem must be 'm of the borrow of the view, not 'q of the query.

Oops! Will try to get a fix out for that tonight.

[rotoclone]

Borrow tracking happens on a per-archetype level meaning borrow flags exist only for each archetype, not for each entity as this is much more efficient.

Ah, I suspected that was the case.

there is get_mut_n which checks that entity_1 and entity_2 are distinct at runtime to allow mutable concurrent access.

I didn't know about get_mut_n, that seems to do what I want, thanks!