Skip to content

Gco/non trivial event missed processing#372

Open
GuiCodron wants to merge 2 commits into
boost-ext:masterfrom
GuiCodron:gco/non-trivial-event-missed-processing
Open

Gco/non trivial event missed processing#372
GuiCodron wants to merge 2 commits into
boost-ext:masterfrom
GuiCodron:gco/non-trivial-event-missed-processing

Conversation

@GuiCodron
Copy link
Copy Markdown
Contributor

Problem:

  • Non trivial on_exit/on_entry can hide on_entry<>/on_exit<> in parents state machine.
    This is due to a dual pass done to process non trivial event:
  • First we process non trivial event, then we process generic event if it failed.
    PR Gco/fix double exit processing #370 amplify this bug:
    on_exit are now processed like on_entry and return true if any child state handle the non trivial event, this imply that generic event will not be processed for parent states.

Solution:

  • Fix the problem by doing a single pass of processing that will trigger the correct event (either mapped or generic).
    This is done by updating mapping generation to take into account that on_entry/on_exit/unexpected_event have a specific generic event case.

Reviewers:
@krzysztof-jusiak

Detected new erroneous behavior with non trivial on_entry/on_exit:
When using non trivial on_entry/on_exit (on_entry<e1> instead of
on_entry<_>), all the on_entry<_> in the parents sm will not be called.
@GuiCodron GuiCodron force-pushed the gco/non-trivial-event-missed-processing branch 2 times, most recently from 4cfc667 to 4fdc199 Compare August 3, 2020 09:47
Generate mappings with mapped and generic events for on_entry, on_exit
and unexpected events.
Remove the dual pass processing done in state_machine.hpp since the
improved get_event_mapping_impl_helper directly generate the good mapping.
@GuiCodron GuiCodron force-pushed the gco/non-trivial-event-missed-processing branch from 4fdc199 to a8af61e Compare August 3, 2020 11:33
@guiserle
Copy link
Copy Markdown
Contributor

guiserle commented May 17, 2023

Apologies in advance to bring this old issue to life, but I am seeing the very same behavior and I wonder whether it is planned to apply a fix or not.

I can provide some examples that highlight the issue:

This example works as expected: https://gcc.godbolt.org/z/vxxceGzW3

However, if I replace state<Child> + sml::on_entry<_> by state<Child> + sml::on_entry<e2> on parent state machine, child on_entry<_> event is not processed. example

Similarly, replacing "s2"_s + sml::on_exit<_> by "s2"_s + sml::on_exit<e3> on child state machine, parent on_exit<_> is not processed. example

For completeness, if both child and parent state machines use the specific on_entry/on_exit version, everything works as expected

As a side note, if I am not wrong, child on_exit hides on_exit on parent state machine, while parent on_entry hides on_entry on child state machine, so the hiding may happen on both "directions"

PavelGuzenfeld added a commit to PavelGuzenfeld/sml that referenced this pull request May 26, 2026
…specific handler (boost-ext#372)

The old process_internal_event (mapped overload) used:

  process_event_impl<specific_mapping>(...) || process_internal_generic_event(...)

When a sub-SM had a specific on_entry<_, E> that appeared in the outer SM's
events_ids_t (via sub_internal_events propagation) but NOT in the outer SM's
own transition table, the mapped path dispatched into the sub-SM via
transitions<true_type>, which routes entry into the sub-SM and returns true.
The || then short-circuited process_internal_generic_event, so the outer SM's
generic on_entry<_> handler was never reached.

Fix: introduce with_default_event_mapping_t<E, DefaultE, TMappings> — a
three-way conditional type alias:
  1. DefaultE not in mappings → use E's mapping directly (or empty)
  2. DefaultE in mappings but E not → use DefaultE's mapping (pure fallback)
  3. Both in mappings → merge via unique_mappings_t (specific takes priority
     inside the merged transitions<> list, which is correct for same-SM use)

Apply the alias in get_event_mapping_impl_helper specialisations for
on_entry<S,E>, on_exit<S,E>, and unexpected_event<S,E>, using on_entry<S,_>
/ on_exit<S,_> / unexpected_event<S,_> as the respective defaults.

The || process_internal_generic_event(...) call is removed from the third
process_internal_event overload; the fallback is now encoded entirely within
the mapping lookup, so no separate generic dispatch is needed.

Tests: two new functional tests in test/ft/transitions.cpp —
composite_nontrivial_entry and composite_nontrivial_exit — verify that the
outer SM's generic handler fires alongside the sub-SM's specific handler for
both entry and exit events, in both the specific-trigger and generic-trigger
(fallback) cases.

All existing tests continue to pass.
PavelGuzenfeld added a commit to PavelGuzenfeld/sml that referenced this pull request May 26, 2026
…specific handler (boost-ext#372)

The old process_internal_event (mapped overload) used:

  process_event_impl<specific_mapping>(...) || process_internal_generic_event(...)

When a sub-SM had a specific on_entry<_, E> that appeared in the outer SM's
events_ids_t (via sub_internal_events propagation) but NOT in the outer SM's
own transition table, the mapped path dispatched into the sub-SM via
transitions<true_type>, which routes entry into the sub-SM and returns true.
The || then short-circuited process_internal_generic_event, so the outer SM's
generic on_entry<_> handler was never reached.

Fix: introduce with_default_event_mapping_t<E, DefaultE, TMappings> — a
three-way conditional type alias:
  1. DefaultE not in mappings → use E's mapping directly (or empty)
  2. DefaultE in mappings but E not → use DefaultE's mapping (pure fallback)
  3. Both in mappings → merge via unique_mappings_t (specific takes priority
     inside the merged transitions<> list, which is correct for same-SM use)

Apply the alias in get_event_mapping_impl_helper specialisations for
on_entry<S,E>, on_exit<S,E>, and unexpected_event<S,E>, using on_entry<S,_>
/ on_exit<S,_> / unexpected_event<S,_> as the respective defaults.

The || process_internal_generic_event(...) call is removed from the third
process_internal_event overload; the fallback is now encoded entirely within
the mapping lookup, so no separate generic dispatch is needed.

Tests: two new functional tests in test/ft/transitions.cpp —
composite_nontrivial_entry and composite_nontrivial_exit — verify that the
outer SM's generic handler fires alongside the sub-SM's specific handler for
both entry and exit events, in both the specific-trigger and generic-trigger
(fallback) cases.

All existing tests continue to pass.
kris-jusiak pushed a commit to PavelGuzenfeld/sml that referenced this pull request May 26, 2026
…specific handler (boost-ext#372)

The old process_internal_event (mapped overload) used:

  process_event_impl<specific_mapping>(...) || process_internal_generic_event(...)

When a sub-SM had a specific on_entry<_, E> that appeared in the outer SM's
events_ids_t (via sub_internal_events propagation) but NOT in the outer SM's
own transition table, the mapped path dispatched into the sub-SM via
transitions<true_type>, which routes entry into the sub-SM and returns true.
The || then short-circuited process_internal_generic_event, so the outer SM's
generic on_entry<_> handler was never reached.

Fix: introduce with_default_event_mapping_t<E, DefaultE, TMappings> — a
three-way conditional type alias:
  1. DefaultE not in mappings → use E's mapping directly (or empty)
  2. DefaultE in mappings but E not → use DefaultE's mapping (pure fallback)
  3. Both in mappings → merge via unique_mappings_t (specific takes priority
     inside the merged transitions<> list, which is correct for same-SM use)

Apply the alias in get_event_mapping_impl_helper specialisations for
on_entry<S,E>, on_exit<S,E>, and unexpected_event<S,E>, using on_entry<S,_>
/ on_exit<S,_> / unexpected_event<S,_> as the respective defaults.

The || process_internal_generic_event(...) call is removed from the third
process_internal_event overload; the fallback is now encoded entirely within
the mapping lookup, so no separate generic dispatch is needed.

Tests: two new functional tests in test/ft/transitions.cpp —
composite_nontrivial_entry and composite_nontrivial_exit — verify that the
outer SM's generic handler fires alongside the sub-SM's specific handler for
both entry and exit events, in both the specific-trigger and generic-trigger
(fallback) cases.

All existing tests continue to pass.
kris-jusiak pushed a commit that referenced this pull request May 26, 2026
…specific handler (#372)

The old process_internal_event (mapped overload) used:

  process_event_impl<specific_mapping>(...) || process_internal_generic_event(...)

When a sub-SM had a specific on_entry<_, E> that appeared in the outer SM's
events_ids_t (via sub_internal_events propagation) but NOT in the outer SM's
own transition table, the mapped path dispatched into the sub-SM via
transitions<true_type>, which routes entry into the sub-SM and returns true.
The || then short-circuited process_internal_generic_event, so the outer SM's
generic on_entry<_> handler was never reached.

Fix: introduce with_default_event_mapping_t<E, DefaultE, TMappings> — a
three-way conditional type alias:
  1. DefaultE not in mappings → use E's mapping directly (or empty)
  2. DefaultE in mappings but E not → use DefaultE's mapping (pure fallback)
  3. Both in mappings → merge via unique_mappings_t (specific takes priority
     inside the merged transitions<> list, which is correct for same-SM use)

Apply the alias in get_event_mapping_impl_helper specialisations for
on_entry<S,E>, on_exit<S,E>, and unexpected_event<S,E>, using on_entry<S,_>
/ on_exit<S,_> / unexpected_event<S,_> as the respective defaults.

The || process_internal_generic_event(...) call is removed from the third
process_internal_event overload; the fallback is now encoded entirely within
the mapping lookup, so no separate generic dispatch is needed.

Tests: two new functional tests in test/ft/transitions.cpp —
composite_nontrivial_entry and composite_nontrivial_exit — verify that the
outer SM's generic handler fires alongside the sub-SM's specific handler for
both entry and exit events, in both the specific-trigger and generic-trigger
(fallback) cases.

All existing tests continue to pass.
@PavelGuzenfeld
Copy link
Copy Markdown
Contributor

Fix and tests already in master via PR #684. with_default_event_mapping_t is at sml.hpp:1607, composite_nontrivial_entry/exit tests are in transitions.cpp:857-930. Can be closed.

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.

3 participants