Ralith / hecs

A handy ECS
Apache License 2.0
924 stars 81 forks source link

World::query_one(_mut) is unsound #300

Closed i509VCB closed 1 year ago

i509VCB commented 1 year ago

Given the following example, it is possible to be given an &mut reference of the same value twice.

An example (this will need to be run a few times to exhibit the race condition):

use std::sync::{Barrier, Arc};

use hecs::World;

let mut world = World::new();
let entity = world.spawn(255u32);

// query_one_mut here is unsound because it returns multiple exclusive references to the `u32` inserted above.
let (value, again) = world.query_one_mut::<(&mut u32, &mut u32)>(entity).unwrap();

// A barrier is used to ensure that the two threads will race each other.
let barrier = Arc::new(Barrier::new(2));
let b1 = Arc::clone(&barrier);
let b2 = Arc::clone(&barrier);

std::thread::scope(|scope| {
    let handle1 = scope.spawn(|| {
        b1.wait();
        *value = 300;
    });

    let handle2 = scope.spawn(|| {
        b2.wait();
        *again = 400;
    });

    handle1.join().unwrap();
    handle2.join().unwrap();
});

assert_eq!(*value, 300);
adamreichold commented 1 year ago

Seem like this entry point does not call query::assert_borrow.

Ralith commented 1 year ago

Whoops, good catch! I should have time to fix this tomorrow if nobody beats me to it.