boost-ext / sml

C++14 State Machine library
https://boost-ext.github.io/sml
Boost Software License 1.0
1.14k stars 177 forks source link

Events not processed when executed from sml::back::process in a separate thread. #189

Open cblauvelt opened 6 years ago

cblauvelt commented 6 years ago

The state machine does not seem to execute actions when sml::back::process() is called in a separate thread. Example is below.

#include <iostream>
#include <queue>
#include <system_error>
#include <thread>
#include <vector>

using namespace std;

#include "sml.hpp"
namespace sml = boost::sml;

namespace TcpClientStateMachine
{
    struct AsyncTcpClientPrivate{
        vector<thread> threads;

        template<class T>
        void asyncResolve(const T& expr){
            threads.emplace_back([&](){
                expr(std::error_code{});
            });
        }

        ~AsyncTcpClientPrivate() {
            for(auto& thread : threads) {
                thread.join();
            }
        }
    };

    // states
    struct Resolving {};
    struct ConnectingSocket {};

    namespace Event {
      struct ResolveComplete{};
      struct ErrorConnecting{
        ErrorConnecting(...){}
      };
    };

    // actions
    struct Resolve {
      template<class TEvent>
        auto operator()(const TEvent& event, sml::back::process<Event::ResolveComplete, Event::ErrorConnecting> processEvent, AsyncTcpClientPrivate& client) {
            cout << "Resolving . . . " << endl;
            client.asyncResolve([&processEvent](const std::error_code& error){
                if(!error) {
                   processEvent(Event::ResolveComplete{});
                } else {
                   processEvent(Event::ErrorConnecting{error});
                }
            });
        }
    };

    struct Connect {
        auto operator()() {
            cout << "Connecting . . . " << endl;
        }
    };

    struct ConnectingStateMachine {
        Resolve resolve;
        Connect connect;

        auto operator()() const {
          using namespace boost::sml;
            /**
             * Initial state: *initial_state
             * Transition DSL: src_state + event [ guard ] / action = dst_state
             */
            return make_transition_table(
                * state<Resolving>          + on_entry<_>                      / resolve,
                  state<Resolving>          + event<Event::ResolveComplete>    / connect    = state<ConnectingSocket>,
                  state<Resolving>          + event<Event::ErrorConnecting>    / [] { std::puts("error"); } = X
            );
        }
    };

}

int main(){
    TcpClientStateMachine::AsyncTcpClientPrivate tcp_client{};
    sml::sm<TcpClientStateMachine::ConnectingStateMachine, sml::process_queue<std::queue>> sm{tcp_client};

    std::this_thread::sleep_for(2s);
}
cblauvelt commented 6 years ago

@krzysztof-jusiak Were you able to reproduce this? Is there any additional information I can provide?

Deadolus commented 4 years ago

I have stumbled on the same problem and I have confirmed this behaviour still exists. The event seems to be never processed by the statemachine, even though it seems to be added to the queue. I used a slightly different version than the version posted by cblauvelt. Adding it here for completions sake.

#include <boost/sml.hpp>
#include <queue>
#include <system_error>
#include <iostream>
#include <thread>
#include <unistd.h>

namespace sml = boost::sml;
using namespace std;

namespace TcpClientStateMachine
{
  namespace detail {
    struct AsyncTcpClientPrivate{
      template<class T>
        void asyncResolve(const T& expr){
          //std::thread t ([&expr]() {
          std::thread t ([](const T& expr) {
              cout << "In thread" << endl;
              std::this_thread::sleep_for(2s);
              cout << "Executing" << endl;
              expr(std::error_code{});
              cout << "Closing thread" << endl;
              }, std::cref(expr));
          t.detach();
        }
    };
  }

  using AsyncTcpClientPrivate = detail::AsyncTcpClientPrivate;

  // states
  struct Resolving {};
  struct ConnectingSocket {};

  namespace Event {
    struct ResolveComplete{};
    struct ErrorConnecting{
      ErrorConnecting(...){}
    };
  };

  // actions
  struct Resolve {
    template<class TEvent>
      auto operator()(const TEvent& event, sml::back::process<Event::ResolveComplete, Event::ErrorConnecting> processEvent, AsyncTcpClientPrivate& client) {
        cout << "Resolving . . . " << endl;
        client.asyncResolve([&processEvent](const std::error_code& error){
            if(!error) {
            processEvent(Event::ResolveComplete{});
            } else {
            processEvent(Event::ErrorConnecting{error});
            }
            });
      }
  };

  struct Connect {
    auto operator()() {
      cout << "Connecting . . . " << endl;
    }
  };

  struct ConnectingStateMachine {
    Resolve resolve;
    Connect connect;

    auto operator()() const {
      using namespace boost::sml;
      /**
       * Initial state: *initial_state
       * Transition DSL: src_state + event [ guard ] / action = dst_state
       */
      return make_transition_table(
          * state<Resolving>          + on_entry<_>                      / resolve,
          //state<Resolving>          + event<Event::ResolveComplete>    / connect    = state<ConnectingSocket>,
          state<Resolving>          + event<Event::ResolveComplete>    / connect    = X,
          state<Resolving>          + event<Event::ErrorConnecting>    / [] { std::puts("error"); } = X
          );
    }
  };

}

int main(){
  TcpClientStateMachine::AsyncTcpClientPrivate tcp_client{};
  sml::sm<TcpClientStateMachine::ConnectingStateMachine, sml::process_queue<std::queue>> sm{tcp_client};
  while (!sm.is(sml::X)) {
    sm.visit_current_states(
        [](auto state) { std::cout << "State " << state.c_str()<<std::endl; });
    sleep(1);
  }
}
idotta commented 1 year ago

Did anyone manage to process the queue correctly after back::process(TEvent) is called? The only workaround I found was to call a function that would process an empty, unexpected event. But that only works with a wrapper class on your SM, as you would need to inject a pointer to this wrapper in the SM constructor.

rhaschke commented 1 year ago

This turns out to be a show-stopper for me as well. I found this workaround: https://stackoverflow.com/questions/64387248/boostext-sml-is-there-a-way-to-store-a-callback-to-later-process-an-event

However, isn't there a clean method to trigger events via sml::back::process from a thread?

rhaschke commented 1 year ago

Looks like sml::back::process just queues events into the process_queue. Those events are subsequently processed when all actions were processed. However, in case of asynchronous processing, events are only later pushed into the queue. Right after all actions were processed, there are no events to process in the queue. Later, if an event is queued, there is no trigger anymore to actually process it.