Skip to content

Conversation

@TestingPlant
Copy link
Contributor

Requires multi-threaded system handlers to be Fn + Send + Sync to avoid undefined behavior from having multiple &mut references to the same value and moving !Send and !Sync types across threads.

The current API which accepts FnMut for SystemAPI::each and the each variants (each_entity, each_iter) allows for undefined behavior because it can share types that are !Send and !Sync across threads when multi_threaded is enabled, and it also creates multiple &mut references to the same FnMut object in multiple threads.

Example of this problem:

struct NoShare {
    id: std::thread::ThreadId,

    // Used to prevent implementing Send and Sync
    _phantom: std::marker::PhantomData<*const ()>
}

impl NoShare {
    fn new() -> Self {
        return NoShare {
            id: std::thread::current().id(),
            _phantom: std::marker::PhantomData
        };
    }

    fn check(&self) {
        let current = std::thread::current().id();
        let expected = self.id;
        if current != expected {
            eprintln!("NoShare was shared from thread {expected:?} to {current:?}");
        }
    }
}

fn main() {
    use flecs_ecs::prelude::*;

    #[derive(Component, Copy, Clone, Debug)]
    struct Foo {
        a: u64,
    }

    let world = World::new();

    for _ in 0..4 {
        world.entity().set(Foo { a: 5 });
    }

    let mut no_share = NoShare::new();

    let system = system!("a", world, &Foo)
        .multi_threaded()
        .each(move |_| {
            no_share.check();
        });

    let mut app = world.app();
    app.set_threads(4);
    app.run();
}

outputs

NoShare was shared from thread ThreadId(1) to ThreadId(2)
NoShare was shared from thread ThreadId(1) to ThreadId(3)
NoShare was shared from thread ThreadId(1) to ThreadId(4)

showing how the value is illegally shared.

To fix this, there are now par variants of the each functions which accept Fn + Send + Sync.

…each

Requires multi-threaded system handlers to be Fn + Send + Sync to
avoid undefined behavior from having multiple &mut references to
the same value and moving !Send and !Sync types across threads.
@Indra-db
Copy link
Owner

Indra-db commented Jun 3, 2025

@TestingPlant Was there a reason why par_run does not exist? Thank you for catching this and the PR!

@TestingPlant
Copy link
Contributor Author

Was there a reason why par_run does not exist?

Thanks for reminding me about it, I just forgot to add it. I added it now in a61c162

@Indra-db Indra-db merged commit e906eba into Indra-db:main Jul 30, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants