Has anyone ever had a need to wrap all the agent's event handlers?

[eao197]

I've encountered a case where every agent's event handlers have to be wrapped with special prologue and epilogue. I mean that every event handler should look like that:

void my_agent::evt_first(mhood_t<msg_first> cmd)
{
  special_context_initializer special_context{...};
  ... // Main business logic of the event-handler.
  special_context.finalize();
}

void my_agent::evt_second(mhood_t<msg_second> cmd)
{
  special_context_initializer special_context{...};
  ... // Main business logic of the event-handler.
  special_context.finalize();
}

void my_agent::evt_third(mhood_t<msg_third> cmd)
{
  special_context_initializer special_context{...};
  ... // Main business logic of the event-handler.
  special_context.finalize();
}

... // And so on.

There are, of course, several ways to do it.

The obvious one is to use a helper method with a lambda:

class my_agent : public so_5::agent_t
{
  template<typename Lambda>
  void
  wrap_event_handler(Lambda && handler)
  {
    special_context_initializer special_context{...};
    handler();
    special_context.finalize();  
  }

  void evt_first(mhood_t<msg_first> cmd)
  {
    wrap_event_handler([&]() {
      ... // Main business logic of the event-handler.
    });
  }

  void evt_second(mhood_t<msg_second> cmd)
  {
    wrap_event_handler([&]() {
      ... // Main business logic of the event-handler.
    });
  }

  ... // And so on.
};

I don't like that way because it's boring and intrusive to agents that implement business logic.

Another way is to make own dispatcher that will do something like that:

void my_custom_dispatcher::process_one_demand(
    so_5::execution_demand_t & demand)
{
  special_context_initializer special_context{...};
  demand.call_handler(worker_thread_id());
  special_context.finalize();
}

This approach allows to write "clean" agents, but it also has own drawbacks:

• we can't use standard dispatchers from SObjectizer or so5extra;
• if special_context.finialize() throws then we have to catch and handle an exception and this handling won't be related to SObjectizer's exception_reaction. There is a big difference: if finalize throws from the event handler then the exception will be considered as an agent's fault and handled in an ordinary way (invocation of so_exception_reaction and so on). But if an exception happens outside of the event handler, then it won't be related to the agent and it's not good from my point of view.

This leads to an idea about a special wrapper that could be installed by a user and will be used for invocation of every agent's event handler (for evt_start/evt_finish too).

It may look like that (please note that this is just a very, very first draft and may change dramatically over time):

// It's a part of SO-5.
namespace so_5
{

// Interface to be used for calling the actual event handler.
// This interface will be implemented by SObjectizer.
class event_handler_invoker_t
{
protected:
  ~event_handler_invoker_t() = default;

public:
  virtual void
  invoke() = 0;
};

// Interface of the wrapper. The user has to implement it.
class event_handler_wrapper_t
{
public:
  virtual ~event_handler_wrapper_t() = default;

  virtual void
  wrap_then_invoke(event_handler_invoker_t & invoker) = 0;
};

using event_handler_wrapper_shptr_t =
  std::shared_ptr<event_handler_wrapper_t>;

} // namespace so_5

// This is part of the user code.
class my_agent : public so_5::agent_t
{
  // Implementation of the wrapper I need.
  class my_wrapper final : public so_5::event_handler_wrapper_t
  {
  public:
    void
    wrap_then_invoke(so_5::event_handler_invoker_t invoker) override
    {
      special_context_initializer special_context{...};
      invoker.invoke();
      special_context.finalize();
    }
  };
  ...
public:
  my_agent(context_t ctx, ...)
    : so_5::agent_t{
      // Wrapper could be specified via tuning options.
      ctx + so_5::wrap_event_handlers(std::make_shared<my_wrapper>(...)) }
  {...}
  ...
};

When SObjectizer sees that a wrapper is set for the agent then SObjectizer changes a way of calling event handlers. For example, the current implementation of agent_t::process_message could be changed this way:

void
agent_t::process_message(
   current_thread_id_t working_thread_id,
   execution_demand_t & d,
   thread_safety_t thread_safety,
   event_handler_method_t method )
{
   // Since v.5.8.5 pending demands may be skipped after dereg.
   if( agent_status_t::shutdown_with_skipping_pending_demands ==
         d.m_receiver->m_current_status )
   {
      // Demand has to be ignored.
      return;
   }

   impl::agent_impl::working_thread_id_sentinel_t sentinel{
         d.m_receiver->m_working_thread_id,
         // v.5.7.3
         // If event_handler is thread_safe-handler then null_thread_id
         // has to be used instead of the actual working_thread_id.
         so_5::thread_safe == thread_safety
            ? null_current_thread_id()
            : working_thread_id
      };

   try
   {
      // If a wrapper is present...
      if( event_handler_wrapper_t * opt_wrapper =
         d.m_receiver->so_query_event_handler_wrapper() )
      {
         // Then it has to be used for calling the handler.
         std_event_handler_invoker_t invoker{ d, method };
         opt_wrapper->wrap_then_invoke( invoker );
      }
      else
      {
         // There is no user-specified wrapper, the event handler
         // has to be called directly.
         method( d.m_message_ref );
      }
   }
   catch( const std::exception & x )
   {
      impl::process_unhandled_exception(
            working_thread_id, x, *(d.m_receiver) );
   }
   catch( ... ) // Since v.5.5.24.3
   {
      impl::process_unhandled_unknown_exception(
            working_thread_id, *(d.m_receiver) );
   }
}

As for me this feature looks interesting, but I have some doubts. And I wondered if someone had the necessity of having something similar. It will be very useful to know such examples (if they are present).

PS. It seems that this feature could be used for implementation of coroutines support, but maybe it's not the right way to go for the case of coroutines.

[ilpropheta]

Hi @eao197 !
I had a similar need in a profiling scenario where I needed to measure the execution time of each handler. Aside from that, my use cases were quite limited, typically just one or two wrappers at most. For example, one simple case involved converting the incoming message into another type that was internal to a specific project, and this applied to all handlers. In that situation, the lambda approach worked well (and was testable).

In my opinion, the proposed approach is overly complicated. If users need to introduce their own event_handler_wrapper_t, I’m afraid part of the simplification you’re aiming for is lost. It also introduces yet another mechanism that people need to understand and document. At that point, I would prefer the lambda approach anyway, and it doesn’t even require virtual dispatch.

It also introduces some readability risks, since the logic becomes split into two parts: the wrapper and the event handler. And what if the next developer sees the subscription but forgets to check wrap_event_handlers?

BTW there is maybe a typo here:

ctx + so_5::wrap_event_handlers(std::make_shared<my_agent>(...)) }

I think you mean:

ctx + so_5::wrap_event_handlers(std::make_shared<my_wrapper>(...)) }

[eao197]

At that point, I would prefer the lambda approach anyway, and it doesn’t even require virtual dispatch.

The good thing is that this approach can be used right now without a need to add a new feature to SObjectizer.

The bad thing is that there could be cases when we have to deal with already written agents those we don't want (or can't) modify. Or there could be a need to have various types of agents that should be written by using such wrappers.

Now I'm considering a case where there is a library for a specific domain that requires settings of several thread-specific variables on all worker threads where this library is used. Without SObjectizer the use of the library looks like that:

void first_worker_thread()
{
  establish_library_specific_context();
  use_library_function_one();
  push_results_to_main_thread();
}

void second_worker_thread()
{
  establish_library_specific_context();
  use_library_function_two();
  push_results_to_main_thread();
}

int main()
{
  initialize_library();
  std::jthread first_thread{ first_worker_thread };
  std::jthread second_thread{ second_worker_thread };
  ...
  collect_result_from_worker_threads();
  ...
}

And I thinking how it could be converted to SObjectizer-based agents where several agents could share common worker threads (thread_pool dispatcher). There could be different types of agents, some of them use this library, some don't.

I don't want to repeat wrap_event_handler in every agent that needs to call the library.

And what if the next developer sees the subscription but forgets to check wrap_event_handlers?

I hope this case would be caught by unit-tests.

[ilpropheta]

I hope this case would be caught by unit-tests.

What I meant is a bit different: the logic behind all event handlers would literally be split across different parts of the source code. You might say that message limits or delivery filters already do something similar, but I see those as different since they can prevent a message from being handled at all. Here, instead, we're spreading the handling logic across two functions, and the programmer has to mentally piece together the flow by jumping between them. This is not necessarily a problem, I am just thinking loudly.

Don't get me wrong. I see the value of such a feature and your example is a good rationale. Since you asked, I am trying to share ideas.
I’m just unsure about the (potential) complexity and usability of it, and I'm not sure I have enough experience to judge how often it would actually be used.

Let me also ask one thing. You said:

we have to deal with already written agents those we don't want (or can't) modify

Just to confirm: since you handle this through context options, you can pass them from outside without changing the agent's code, right? For example:

env.introduce_coop([&](so_5::coop_t& coop) {
   coop.make_agent<Agent1>(); // will benefit from the feature
   coop.make_agent<Agent2>(); // will benefit from the feature
   coop.make_agent<Agent3>(); // will benefit from the feature
   coop.make_agent<Agent4>();
   coop.make_agent<Agent5>();
}

This changes into:

agent_context_t context{ env };
context = context + wrap_event_handlers(std::make_shared<my_wrapper>(...)) ;
// special agents
auto agent1 = make_unique<Agent1>(context);
auto agent2 = make_unique<Agent2>(context);
auto agent3 = make_unique<Agent3>(context);
auto coop = env.make_coop();
coop->add_agent(move(agent1));
coop->add_agent(move(agent2));
coop->add_agent(move(agent3));
// ordinary agents
coop->make_agent<Agent4>();
coop->make_agent<Agent5>();
env.register_coop(move(coop));

Is my interpretation correct? Since we discussed something similar recently, I'm just checking that I've understood the pattern the right way :)

[eao197]

What I meant is a bit different: the logic behind all event handlers would literally be split across different parts of the source code.

Yes, this is the goal. There could be cases where we have to glue two different parts of business logic together. The first part is the main business logic itself (like some specific calculation in my case). The second part is more related to infrastructure than to business logic itself (like setting the correct computation context, measurement of event handling time, logging of start/finish of event handlers and so on). And because such parts are different they can be splitted into separate code fragments.

You are also right that this may make code logic more complex. But here I'm one the typical C++ side: a user should have different tools to have a possibility to mix them as he/she likes.

Is my interpretation correct?

Yes, exactly.

However, specifying a wrapper via agent_context is one possible way. It has a drawback: there is no way to combine several wrappers. For example, let's suppose that agent itself installs a wrapper that prepares special calculation context. But we want to add another wrapper that will measure event handlers execution time.

Usually a value that is set in the agent_context will be overwritten if the same setter is called again. For example:

class calculation_agent : public so_5::agent_t
{
public:
  calculation_agent(context_t ctx)
    : so_5::agent_t{ ctx + wrap_event_handler(std::make_shared<my_wrapper>()) } // (2)
  {}
};
...
auto * agent = std::make_unique<calculation_agent>(
  so_5::agent_context_t{env} + std::make_shared<duration_meter>(...) ); // (1)

In that case the duration_meter wrapper created at (1) will be lost at (2).

So if agent_context will be used for storing wrappers then it should support several such wrappers.

[ilpropheta]

But here I'm one the typical C++ side: a user should have different tools to have a possibility to mix them as he/she likes.

I agree.

So if agent_context will be used for storing wrappers then it should support several such wrappers.

I see here a decorator pattern coming to light. A sort of decorate_event_handler functionality, that naturally supports a chain of nested decorators? Or just a flat list, like a composite?

[eao197]

The first draft of how nested wrappers can be supported.

This is how description of event_handler_invoker and event_handler_wrapper could look like:

class event_handler_wrapper_t;
using event_handler_wrapper_shptr_t =
  std::shared_ptr<event_handler_wrapper_t>;

class event_handler_invoker_t
{
  friend class event_handler_wrapper_t;

protected:
  ~event_handler_invoker_t() = default;

  // NOTE: this method is protected and is available
  // only for the friends of the invoker class.
  virtual void
  invoke() = 0;
};

class event_handler_wrapper_t
{
private:
  event_handler_wrapper_shptr_t m_nested;

protected:
  void
  do_invoke(
    const execution_demand_t & src_demand,
    event_handler_invoker_t & invoker) /* const ??? */
  {
    if(m_nested)
      m_nested->wrap_then_invoke(src_demand, invoker);
    else
      // NOTE: only event_handler_wrapper can do this call.
      // Because of that all users wrappers have to use
      // event_handler_wrapper_t::do_invoke() method instead
      // of calling event_handler_invoker directly.
      ivoker.invoke();
  }

public:
  event_handler_wrapper_t() = default;
  virtual ~event_handler_wrapper_t() = default;

  void
  set_nested(event_handler_wrapper_shptr_t nested)
  {
    if(m_nested)
      throw exception_t{ ... };
    m_nested = std::move(nested);
  }

  virtual void
  wrap_then_invoke(
    const execution_demand_t & src_demand,
    event_handler_invoker_t & invoker) = 0;
};

The idea is that when a wrapper is added to the agent context via:

ctx + wrap_event_handler(wrapper_B)

then the agent context checks the presence of the already specified wrapper. If there is another wrapper_A that was specified earlier then the agent context:

• calls wrapper_B.set_nested(wrapper_A). The ownership of wrapper_A is now transferred to wrapper_B;
• stores wrapper_B as the single wrapper.

With such an approach several wrappers can be added to the agent context, but they will be chained together via set_nested calls and the context will hold just one pointer to the head of this chain.

The implementation of wrap_the_invoke method should use the do_invoke method from the base class, for example:

class my_wrapper final : public so_5::event_handler_wrapper_t
{
public:
  void
  wrap_then_invoke(
    const so_5::execution_demand_t & src_demand,
    so_5::event_handler_invoker_t & invoker) override
  {
    special_context_initializer special_context{...};
    this->do_invoke(src_demand, invoker);
    special_context.finalize();
  }
};