Hang when running custom system impl inspired by bevy's combinator systems, regression from 0.9

[inodentry]

Bevy version

0.10.1

Regression from 0.9.1

System

OS: macOS 13.1 Ventura
CPU: Apple M1 Pro

What you did

I have the following code. This is a custom system combinator akin to Bevy's PipeSystem, with the difference that the first system returns an Option<T> and the second system is only called if the first system returned Some.

I have carried this code forward over the past few versions of Bevy. It used to work. Now in 0.10 it no longer does. This is interesting, because I looked at Bevy's combinator system implementation (https://github.com/bevyengine/bevy/blob/v0.10.1/crates/bevy_ecs/src/system/combinator.rs#L124) for reference and made sure that my code mirrors that. I expect my thing to work, given that it is adapted from something that is already included in Bevy itself. With previous versions of Bevy, I used the implementation of PipeSystem as my reference to learn from.

Code

pub struct ChainOptionalSystem<SystemIn, SystemSome> {
    system_in: SystemIn,
    system_some: SystemSome,
    name: Cow<'static, str>,
    component_access: Access<ComponentId>,
    archetype_component_access: Access<ArchetypeComponentId>,
}

impl<T, O: Default, SystemIn: System<Out = Option<T>>, SystemSome: System<In = T, Out = O>> System for ChainOptionalSystem<SystemIn, SystemSome> {
    type In = SystemIn::In;
    type Out = O;

    fn name(&self) -> Cow<'static, str> {
        self.name.clone()
    }

    fn type_id(&self) -> std::any::TypeId {
        std::any::TypeId::of::<Self>()
    }

    fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
        &self.archetype_component_access
    }

    fn component_access(&self) -> &Access<ComponentId> {
        &self.component_access
    }

    fn is_send(&self) -> bool {
        self.system_in.is_send() && self.system_some.is_send()
    }

    fn is_exclusive(&self) -> bool {
        self.system_in.is_send() || self.system_some.is_send()
    }

    unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out {
        if let Some(t) = self.system_in.run_unsafe(input, world) {
            self.system_some.run_unsafe(t, world)
        } else {
            O::default()
        }
    }

    fn apply_buffers(&mut self, world: &mut World) {
        self.system_in.apply_buffers(world);
        self.system_some.apply_buffers(world);
    }

    fn initialize(&mut self, world: &mut World) {
        self.system_in.initialize(world);
        self.system_some.initialize(world);
        self.component_access
            .extend(self.system_in.component_access());
        self.component_access
            .extend(self.system_some.component_access());
    }

    fn update_archetype_component_access(&mut self, world: &World) {
        self.system_in.update_archetype_component_access(world);
        self.system_some.update_archetype_component_access(world);
        self.archetype_component_access
            .extend(self.system_in.archetype_component_access());
        self.archetype_component_access
            .extend(self.system_some.archetype_component_access());
    }

    fn check_change_tick(&mut self, change_tick: u32) {
        self.system_in.check_change_tick(change_tick);
        self.system_some.check_change_tick(change_tick);
    }

    fn get_last_change_tick(&self) -> u32 {
        self.system_in.get_last_change_tick()
    }

    fn set_last_change_tick(&mut self, last_change_tick: u32) {
        self.system_in.set_last_change_tick(last_change_tick);
        self.system_some.set_last_change_tick(last_change_tick);
    }

    fn default_system_sets(&self) -> Vec<Box<dyn SystemSet>> {
        let mut default_sets = self.system_in.default_system_sets();
        default_sets.append(&mut self.system_some.default_system_sets());
        default_sets
    }
}

pub trait IntoChainOptionalSystem<T, Out, SysSome, ParamIn, ParamSome>:
    IntoSystem<(), Option<T>, ParamIn> + Sized
where
    SysSome: IntoSystem<T, Out, ParamSome>,
{
    fn chain_optional(self, system: SysSome) -> ChainOptionalSystem<Self::System, SysSome::System>;
}

impl<T, Out, SysIn, SysSome, ParamIn, ParamSome>
    IntoChainOptionalSystem<T, Out, SysSome, ParamIn, ParamSome> for SysIn
where
    SysIn: IntoSystem<(), Option<T>, ParamIn>,
    SysSome: IntoSystem<T, Out, ParamSome>,
{
    fn chain_optional(self, system: SysSome) -> ChainOptionalSystem<SysIn::System, SysSome::System> {
        let system_in = IntoSystem::into_system(self);
        let system_some = IntoSystem::into_system(system);

        ChainOptionalSystem {
            name: Cow::Owned(format!("ChainOptional({} -> {})", system_in.name(), system_some.name())),
            system_in,
            system_some,
            archetype_component_access: Default::default(),
            component_access: Default::default(),
        }
    }
}

I am using this thing as a building block for handling Bevy UI buttons, among other things.

Code

#[derive(Component)]
pub struct UiDisabled;

pub fn button_handler<B: Component + Clone, Params>(handler: impl IntoSystem<B, (), Params>) -> impl System<In = (), Out = ()> {
    on_button_interact.chain_optional(handler)
}

fn on_button_interact<B: Component + Clone>(
    query: Query<(&Interaction, &B), (Changed<Interaction>, With<Button>, Without<UiDisabled>)>,
) -> Option<B> {
    for (interaction, b) in query.iter() {
        if *interaction == Interaction::Clicked {
            return Some(b.clone());
        }
    }
    None
}

It could be used like this:

Code

// Example button implementation that transitions to a specific state
#[derive(Component, Clone)]
struct StateTransitionButton(AppState);

fn setup_menu(/* ... */) {
    // new game button
    commands.spawn((
        StateTransitionButton(AppState::LoadingGame),
        ButtonBundle {
            // ...
        },
    ));
    // settings button
    commands.spawn((
        StateTransitionButton(AppState::Settings),
        ButtonBundle {
            // ...
        },
    ));
    // credits button
    commands.spawn((
        StateTransitionButton(AppState::ShowCredits),
        ButtonBundle {
            // ...
        },
    ));
}

fn handle_state_transition_button(
    In(btn_data): In<StateTransitionButton>,
    // arbitrary system params
    mut state: ResMut<NextState<AppState>>,
) {
    state.set(btn_data.0);
}

app.add_system(setup_menu.in_schedule(OnEnter(AppState::Menu)));
app.add_system(button_handler(handle_state_transition_button));

What went wrong

If I add the button handler system to my app, Bevy hangs when it tries to run it. The window becomes frozen and unresponsive.

Additional information

Like I said, this exact thing worked in previous versions of Bevy. I am just porting it to 0.10 now. I made my custom impl System by looking at the code of Bevy's PipeSystem, and now the new generic combinator system.

I tried running my app in lldb to try to track down what is going on. I could keep stepping long after my run_unsafe impl, past the end of the body of the task spawned by bevy's multithreaded executor. Eventually I reached here: https://github.com/bevyengine/bevy/blob/v0.10.1/crates/bevy_tasks/src/task_pool.rs#L500 and that is when the hang happened. Continuing to step the debugger only showed assembly listings from libpthread. I suspect we are getting stuck in this execute_forever thing. I do not understand this code and I have no idea what it does and why. It seems to have been introduced in #7415 .

I am stumped. Please help. This whole thing is so weird.

[JoJoJet]

Not sure why this is hanging, but a workaround should be possible.

I have the following code. This is a custom system combinator akin to Bevy's PipeSystem, with the difference that the first system returns an Option and the second system is only called if the first system returned Some.

This should be doable using CombinatorSystem, by implementing the Combine trait:

pub type ChainOptionalSystem<A, B> = CombinatorSystem<ChainOptionalMarker, A, B>;

#[doc(hidden)]
pub struct ChainOptionalMarker;

impl<A, B> Combine<A, B> for ChainOptionalMarker
where
     A: System<Out = Option<B::In>>,
     B: System,
{
    type In = A::In;
    type Out = B::Out;
    fn combine(
        input: A::In,
        a: impl FnOnce(A::In) -> A::Out,
        b: impl FnOnce(B::In) -> B::Oout
    ) -> B::Out {
        if let Some(value) = a(input) {
            b(value)
        } else {
            todo!()
        }
    }
}

[inodentry]

Well, thanks! :) Sure, I can get the job done in other ways.

I also had another similar custom System impl for Result, which calls a different secondary system depending on whether the first system returns Ok or Err. I don't think that's (directly) representable using the Combine trait, because it has 3 systems. Fortunately, I am not really using it anywhere important, so it's not a deal breaker to me that it no longer works.

It would be good to figure out why it is hanging, though. There is a chance that it might be an issue that goes deeper in bevy.

If you have any ideas for anything I could investigate, let me know! :)

[JoJoJet]

I think that @hymm or @james7132 would be more likely to be able to help with the problem at hand. I may be able to help with another workaround though.

If I understand your requirements correctly, your result combinator should be expressible using something like:

type ResultSystem<A, B> = CombinatorSystem<A, B, ResultCombinator>

pub struct ResultCombinator;

impl<A, B> Combine<A, B> for ResultCombinator
where
     A: System,
     B: System<Out = A::Out>,
{
    type In = Result<A::In, B::In>;
    type Out = A::Out;
    fn combine(
        input: Self::In,
        a: impl FnOnce(A::In) -> A::Out,
        b: impl FnOnce(B::In) -> B::Oout
    ) -> Self::Out {
        match input {
            Ok(x) => a(x),
            Err(e) => b(e),
        }
    }
}

fn system_a() -> Result<T, U> {}
fn system_ok(In(_): In<T>) {}
fn system_err(In(_): In<U>) {}

schedule.add_systems(system_a.pipe(ResultSystem::new(system_ok, system_err)));

[hymm]

is there a minimal example or public code I could use to check this?