Catch reentrant calls to FSM and HFSM start, receive, etc.

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 <typename T = void>
@@ -208,7 +221,48 @@ namespace etl
     struct check_ids<Id, TState0, TRest...>
       : etl::integral_constant<bool, (TState0::STATE_ID == Id) && private_fsm::check_ids<Id + 1, TRest...>::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
   };
 
   //*************************************************************************************************

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<bool, (TState0::STATE_ID == Id) && private_fsm::check_ids<Id + 1, TRest...>::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,18 +737,14 @@ 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();
     }
 
     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
   };
 
   //*************************************************************************************************

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);

test/test_hfsm_transition_on_enter.cpp

@@ -31,6 +31,7 @@ SOFTWARE.
 #include "etl/hfsm.h"
 #include "etl/string.h"
 #include <array>
+#include <memory>
 
 //                             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