Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

observe on WorldEntityMut may cause the reference to be invalidated #16212

Open
nakedible opened this issue Nov 2, 2024 · 0 comments · May be fixed by #16219
Open

observe on WorldEntityMut may cause the reference to be invalidated #16212

nakedible opened this issue Nov 2, 2024 · 0 comments · May be fixed by #16219
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Design This issue requires design work to think about how it would best be accomplished
Milestone

Comments

@nakedible
Copy link
Contributor

Bevy version

0.15.0-rc.2 (present in 0.14.0) as well

What you did

    #[derive(Event)]
    struct Boom;

    #[derive(Event)]
    struct BoomTomorrow;

    fn boom(trigger: Trigger<Boom>, mut commands: Commands) {
        println!("boom, despawning entity: {:?}", trigger.entity());
        commands.entity(trigger.entity()).despawn();
    }

    fn boom_tomorrow(trigger: Trigger<Boom>, mut commands: Commands) {
        // boom tomorrow
    }

    #[test]
    fn test_boom() {
        let mut app = App::new();

        let mut id = app.world_mut().spawn(Name::new("a")).observe(boom).id();
        let mut id2 = app.world_mut().spawn(Name::new("b")).id();
        
        app.world_mut().flush();
        println!("flushed");
        
        let mut a = app.world_mut().entity_mut(id);
        a.trigger(Boom);
        a.observe(boom_tomorrow);
        let name = a.get::<Name>().unwrap();
        assert_eq!(name.as_str(), "b");
    }

Result

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out; finished in 0.00s

What went wrong

  • WorldEntityMut depends on the fact that the EntityLocation it caches does not change while it is alive, or changes when it itself expects it to.
  • This means that there must be no way to flush the command queue while WorldEntityMut lives, as this might cause queued despawns to be run.
  • observe internally calls world.spawn().
  • world.spawn() implicitly has a flush() in it, as evidenced in bug report Commands apply at unexpected times with exclusive World access #14621.
  • Hence, entity gets despawned while WorldEntityMut exists, which means any accesses will use outdated EntityLocation

Additional information

Also related as context #16034.

@nakedible nakedible added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Nov 2, 2024
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Nov 2, 2024
@alice-i-cecile alice-i-cecile modified the milestones: 0.15, 0.16 Nov 2, 2024
@alice-i-cecile alice-i-cecile added S-Needs-Design This issue requires design work to think about how it would best be accomplished D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Nov 2, 2024
@nakedible nakedible linked a pull request Nov 3, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants