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

Stack overflow with ambiguity detection and .run_if(in_state(...)) #9641

Closed
SorteKanin opened this issue Aug 30, 2023 · 7 comments · Fixed by #9650
Closed

Stack overflow with ambiguity detection and .run_if(in_state(...)) #9641

SorteKanin opened this issue Aug 30, 2023 · 7 comments · Fixed by #9650
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior S-Needs-Investigation This issue requires detective work to figure out what's going wrong

Comments

@SorteKanin
Copy link

SorteKanin commented Aug 30, 2023

Bevy version

bevy = "0.11.2"

[Optional] Relevant system information

> cargo --version -v
cargo 1.71.1 (7f1d04c00 2023-07-29)
release: 1.71.1
commit-hash: 7f1d04c0053083b98fa50b69b6f56e339b0556a8
commit-date: 2023-07-29
host: x86_64-pc-windows-msvc
libgit2: 1.6.4 (sys:0.17.1 vendored)
libcurl: 8.0.1-DEV (sys:0.4.61+curl-8.0.1 vendored ssl:Schannel)
os: Windows 10.0.19045 (Windows 10 Home) [64-bit]

What you did

Minimal example:

use bevy::{
    ecs::schedule::{LogLevel, ScheduleBuildSettings},
    prelude::*,
};

#[derive(States, Clone, Copy, Default, Debug, PartialEq, Eq, Hash)]
enum DummyState {
    #[default]
    Foo,
    Bar,
}

fn main() {
    App::new()
        .edit_schedule(Update, |schedule| {
            schedule.set_build_settings(ScheduleBuildSettings {
                ambiguity_detection: LogLevel::Error,
                ..default()
            });
        })
        .add_state::<DummyState>()
        .add_plugins(DefaultPlugins)
        .add_systems(
            Update,
            (dummy_system, dummy_system).run_if(in_state(DummyState::Foo)),
        )
        .run()
}

fn dummy_system() {}

What went wrong

thread 'main' has overflowed its stack upon running.

Additional information

Note that using distributive_run_if seems to fix the problem. Also adding just one system (i.e. dummy_system.run_if(in_state(DummyState::Foo)) does not exhibit the problem. Problem happens even if different systems are used, the fact that it's the same system in the tuple is unrelated.

Only confirmed on Windows. Would be great if someone with a Linux/Mac machine, could confirm/deny if this happens on other platforms.

@SorteKanin SorteKanin added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Aug 30, 2023
@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 Aug 30, 2023
@EmiOnGit
Copy link
Contributor

EmiOnGit commented Aug 30, 2023

Can confirm the same problem on linux

@hymm
Copy link
Contributor

hymm commented Aug 30, 2023

I can't reproduce on main branch, so it might have been fixed already. Can someone else test main to confirm?

@EmiOnGit
Copy link
Contributor

I can't reproduce on main branch, so it might have been fixed already. Can someone else test main to confirm?

Same for me; main works without problems
@SorteKanin can you check?

@SorteKanin
Copy link
Author

Yes it does seem to be fixed on the main branch (should've checked before probably...). Closing.

@hymm
Copy link
Contributor

hymm commented Aug 30, 2023

I bisected this back to #9260. If I revert that change and run the above code it still stack overflows. Since that change was just moving systems around, the problem likely still exists on main too. Will try and see if I can come up with a better minimal reproduction.

@hymm hymm reopened this Aug 30, 2023
@alice-i-cecile alice-i-cecile added the S-Needs-Investigation This issue requires detective work to figure out what's going wrong label Aug 30, 2023
@hymm
Copy link
Contributor

hymm commented Aug 30, 2023

use bevy::prelude::*;

fn main() {
    let mut schedule = Schedule::default();

    fn exclusive(_world: &mut World) {}
    
    fn dummy() {}
    
    schedule.set_build_settings(bevy::ecs::schedule::ScheduleBuildSettings {
        ambiguity_detection: bevy::ecs::schedule::LogLevel::Error,
        ..Default::default()
    });
    
    schedule.add_systems((exclusive, (dummy, dummy).run_if(|| true)));
    
    schedule.initialize(&mut World::new()).unwrap();
}

This stack overflows. Probably has something to do with anonymous system sets and exclusive systems.

@hymm
Copy link
Contributor

hymm commented Aug 30, 2023

This turned out to be a problem with getting an anonymous system sets name when the report_sets settings was true. See above PR for more details.

github-merge-queue bot pushed a commit that referenced this issue Aug 31, 2023
# Objective

- Fixes #9641
- Anonymous sets are named by their system members. When
`ScheduleBuildSettings::report_sets` is on, systems are named by their
sets. So when getting the anonymous set name this would cause an
infinite recursion.

## Solution
- When getting the anonymous system set name, don't get their system's
names with the sets the systems belong to.

## Other Possible solutions
- An alternate solution might be to skip anonymous sets when getting the
system's name for an anonymous set's name.
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this issue Jan 9, 2024
# Objective

- Fixes bevyengine#9641
- Anonymous sets are named by their system members. When
`ScheduleBuildSettings::report_sets` is on, systems are named by their
sets. So when getting the anonymous set name this would cause an
infinite recursion.

## Solution
- When getting the anonymous system set name, don't get their system's
names with the sets the systems belong to.

## Other Possible solutions
- An alternate solution might be to skip anonymous sets when getting the
system's name for an anonymous set's name.
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 S-Needs-Investigation This issue requires detective work to figure out what's going wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants