diff --git a/include/etl/fsm.h b/include/etl/fsm.h index 6382884a4..4eaa016a0 100644 --- a/include/etl/fsm.h +++ b/include/etl/fsm.h @@ -172,6 +172,19 @@ namespace etl } }; + //*************************************************************************** + /// Exception for call to receive/start/etc. while receive/start/etc. is already happening. + /// A call like that could result in an infinite loop or landing in an incorrect state. + //*************************************************************************** + class fsm_reentrant_transition_forbidden : public etl::fsm_exception + { + public: + fsm_reentrant_transition_forbidden(string_type file_name_, numeric_type line_number_) + : etl::fsm_exception(ETL_ERROR_TEXT("fsm:reentrant calls to start/receive/etc. forbidden", ETL_FSM_FILE_ID"G"), file_name_, line_number_) + { + } + }; + namespace private_fsm { template @@ -208,7 +221,48 @@ namespace etl struct check_ids : etl::integral_constant::value> { - }; + }; + + //*************************************************************************** + /// RAII detection mechanism to catch reentrant calls to methods that might + /// transition the state machine to a different state. + /// This is not a mutex. + //*************************************************************************** + class fsm_reentrancy_guard + { + public: + //******************************************* + /// Constructor. + /// Checks if another method has locked reentrancy. + //******************************************* + fsm_reentrancy_guard(bool& transition_guard_flag) + : is_locked(transition_guard_flag) + { + ETL_ASSERT(!is_locked, ETL_ERROR(etl::fsm_reentrant_transition_forbidden)); + is_locked = true; + } + + //******************************************* + /// Destructor. + /// Releases lock on reentrancy. + //******************************************* + ~fsm_reentrancy_guard() ETL_NOEXCEPT + { + is_locked = false; + } + + private: + // Reference to the flag signifying a lock on the state machine. + bool& is_locked; + + // Copy & move semantics disabled since this is a guard. + fsm_reentrancy_guard(fsm_reentrancy_guard const&) ETL_DELETE; + fsm_reentrancy_guard& operator= (fsm_reentrancy_guard const&) ETL_DELETE; +#if ETL_USING_CPP11 + fsm_reentrancy_guard(fsm_reentrancy_guard&&) ETL_DELETE; + fsm_reentrancy_guard& operator= (fsm_reentrancy_guard&&) ETL_DELETE; +#endif + }; } class ifsm_state; @@ -417,6 +471,7 @@ namespace etl , p_state(ETL_NULLPTR) , state_list(ETL_NULLPTR) , number_of_states(0U) + , is_processing_state_change(false) { } @@ -467,6 +522,8 @@ namespace etl //******************************************* virtual void start(bool call_on_enter_state = true) { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + // Can only be started once. if (!is_started()) { @@ -497,6 +554,8 @@ namespace etl //******************************************* void receive(const etl::imessage& message) ETL_OVERRIDE { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + if (is_started()) { etl::fsm_state_id_t next_state_id = p_state->process_event(message); @@ -514,6 +573,8 @@ namespace etl //******************************************* etl::fsm_state_id_t transition_to(etl::fsm_state_id_t new_state_id) { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + if (is_started()) { return process_state_change(new_state_id); @@ -576,6 +637,8 @@ namespace etl //******************************************* virtual void reset(bool call_on_exit_state = false) { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + if (is_started() && call_on_exit_state) { p_state->on_exit_state(); @@ -655,6 +718,7 @@ namespace etl etl::ifsm_state* p_state; ///< A pointer to the current state. etl::ifsm_state** state_list; ///< The list of added states. etl::fsm_state_id_t number_of_states; ///< The number of states. + bool is_processing_state_change; ///< Whether a method call that could potentially trigger a state change is active }; //************************************************************************************************* diff --git a/include/etl/generators/fsm_generator.h b/include/etl/generators/fsm_generator.h index 161c413b9..0887af6c0 100644 --- a/include/etl/generators/fsm_generator.h +++ b/include/etl/generators/fsm_generator.h @@ -180,25 +180,26 @@ namespace etl }; //*************************************************************************** - /// Exception for forbidden state changes. + /// Exception for message received but not started. //*************************************************************************** - class fsm_state_composite_state_change_forbidden : public etl::fsm_exception + class fsm_not_started : public etl::fsm_exception { public: - fsm_state_composite_state_change_forbidden(string_type file_name_, numeric_type line_number_) - : etl::fsm_exception(ETL_ERROR_TEXT("fsm:change in composite state forbidden", ETL_FSM_FILE_ID"E"), file_name_, line_number_) + fsm_not_started(string_type file_name_, numeric_type line_number_) + : etl::fsm_exception(ETL_ERROR_TEXT("fsm:not started", ETL_FSM_FILE_ID"F"), file_name_, line_number_) { } }; //*************************************************************************** - /// Exception for message received but not started. + /// Exception for call to receive/start/etc. while receive/start/etc. is already happening. + /// A call like that could result in an infinite loop or landing in an incorrect state. //*************************************************************************** - class fsm_not_started : public etl::fsm_exception + class fsm_reentrant_transition_forbidden : public etl::fsm_exception { public: - fsm_not_started(string_type file_name_, numeric_type line_number_) - : etl::fsm_exception(ETL_ERROR_TEXT("fsm:not started", ETL_FSM_FILE_ID"F"), file_name_, line_number_) + fsm_reentrant_transition_forbidden(string_type file_name_, numeric_type line_number_) + : etl::fsm_exception(ETL_ERROR_TEXT("fsm:reentrant calls to start/receive/etc. forbidden", ETL_FSM_FILE_ID"G"), file_name_, line_number_) { } }; @@ -240,6 +241,47 @@ namespace etl : etl::integral_constant::value> { }; + + //*************************************************************************** + /// RAII detection mechanism to catch reentrant calls to methods that might + /// transition the state machine to a different state. + /// This is not a mutex. + //*************************************************************************** + class fsm_reentrancy_guard + { + public: + //******************************************* + /// Constructor. + /// Checks if another method has locked reentrancy. + //******************************************* + fsm_reentrancy_guard(bool& transition_guard_flag) + : is_locked(transition_guard_flag) + { + ETL_ASSERT(!is_locked, ETL_ERROR(etl::fsm_reentrant_transition_forbidden)); + is_locked = true; + } + + //******************************************* + /// Destructor. + /// Releases lock on reentrancy. + //******************************************* + ~fsm_reentrancy_guard() ETL_NOEXCEPT + { + is_locked = false; + } + + private: + // Reference to the flag signifying a lock on the state machine. + bool& is_locked; + + // Copy & move semantics disabled since this is a guard. + fsm_reentrancy_guard(fsm_reentrancy_guard const&) ETL_DELETE; + fsm_reentrancy_guard& operator= (fsm_reentrancy_guard const&) ETL_DELETE; +#if ETL_USING_CPP11 + fsm_reentrancy_guard(fsm_reentrancy_guard&&) ETL_DELETE; + fsm_reentrancy_guard& operator= (fsm_reentrancy_guard&&) ETL_DELETE; +#endif + }; } class ifsm_state; @@ -356,7 +398,6 @@ namespace etl if (p_default_child == ETL_NULLPTR) { - p_active_child = &state; p_default_child = &state; } } @@ -456,6 +497,7 @@ namespace etl , p_state(ETL_NULLPTR) , state_list(ETL_NULLPTR) , number_of_states(0U) + , is_processing_state_change(false) { } @@ -506,8 +548,10 @@ namespace etl //******************************************* virtual void start(bool call_on_enter_state = true) { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + // Can only be started once. - if (p_state == ETL_NULLPTR) + if (!is_started()) { p_state = state_list[0]; ETL_ASSERT(p_state != ETL_NULLPTR, ETL_ERROR(etl::fsm_null_state_exception)); @@ -536,6 +580,8 @@ namespace etl //******************************************* void receive(const etl::imessage& message) ETL_OVERRIDE { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + if (is_started()) { etl::fsm_state_id_t next_state_id = p_state->process_event(message); @@ -553,6 +599,8 @@ namespace etl //******************************************* etl::fsm_state_id_t transition_to(etl::fsm_state_id_t new_state_id) { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + if (is_started()) { return process_state_change(new_state_id); @@ -615,7 +663,9 @@ namespace etl //******************************************* virtual void reset(bool call_on_exit_state = false) { - if ((p_state != ETL_NULLPTR) && call_on_exit_state) + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + + if (is_started() && call_on_exit_state) { p_state->on_exit_state(); } @@ -662,6 +712,12 @@ namespace etl //******************************************* virtual etl::fsm_state_id_t process_state_change(etl::fsm_state_id_t next_state_id) { + if (is_self_transition(next_state_id)) + { + p_state->on_exit_state(); + next_state_id = p_state->on_enter_state(); + } + if (have_changed_state(next_state_id)) { ETL_ASSERT_OR_RETURN_VALUE(next_state_id < number_of_states, ETL_ERROR(etl::fsm_state_id_exception), p_state->get_state_id()); @@ -681,11 +737,6 @@ namespace etl } } while (p_next_state != p_state); // Have we changed state again? } - else if (is_self_transition(next_state_id)) - { - p_state->on_exit_state(); - p_state->on_enter_state(); - } return p_state->get_state_id(); } @@ -693,6 +744,7 @@ namespace etl etl::ifsm_state* p_state; ///< A pointer to the current state. etl::ifsm_state** state_list; ///< The list of added states. etl::fsm_state_id_t number_of_states; ///< The number of states. + bool is_processing_state_change; ///< Whether a method call that could potentially trigger a state change is active }; //************************************************************************************************* diff --git a/include/etl/hfsm.h b/include/etl/hfsm.h index c3f2062c3..6db390f0f 100644 --- a/include/etl/hfsm.h +++ b/include/etl/hfsm.h @@ -59,6 +59,8 @@ namespace etl //******************************************* void start(bool call_on_enter_state = true) ETL_OVERRIDE { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + // Can only be started once. if (!is_started()) { @@ -98,6 +100,8 @@ namespace etl //******************************************* virtual void reset(bool call_on_exit_state = false) ETL_OVERRIDE { + private_fsm::fsm_reentrancy_guard transition_lock(is_processing_state_change); + if (is_started() && call_on_exit_state) { do_exits(ETL_NULLPTR, p_state); diff --git a/test/test_hfsm_transition_on_enter.cpp b/test/test_hfsm_transition_on_enter.cpp index ff994c10e..a83c3d1a9 100644 --- a/test/test_hfsm_transition_on_enter.cpp +++ b/test/test_hfsm_transition_on_enter.cpp @@ -31,6 +31,7 @@ SOFTWARE. #include "etl/hfsm.h" #include "etl/string.h" #include +#include // entry && mode=RxEventDeviation // ┌─────────────────────────────────────────────────────┐ @@ -358,6 +359,25 @@ namespace sm.reset(true); CHECK_EQUAL("X1X2X3", sm.test_data.c_str()); } + + //************************************************************************* + TEST(reentrant_receives) + { + EntryTestSM sm; + sm.start(false); + sm.receive(ToS5Event{}); + sm.test_data.clear(); + sm.runMode = EntryTestSM::RxEventDuringTransition; + CHECK_THROW(sm.receive(ToS6Event{}),etl::fsm_reentrant_transition_forbidden); + } + + //************************************************************************* + TEST(reentrant_receives_on_start) + { + EntryTestSM sm; + sm.runMode = EntryTestSM::RxEventDuringTransition; + CHECK_THROW(sm.start(true),etl::fsm_reentrant_transition_forbidden); + } } } // namespace