Stiffstream / sobjectizer

An implementation of Actor, Publish-Subscribe, and CSP models in one rather small C++ framework. With performance, quality, and stability proved by years in the production.
https://stiffstream.com/en/products/sobjectizer.html
Other
481 stars 47 forks source link

noexcept for closing mchains #33

Closed eao197 closed 2 years ago

eao197 commented 3 years ago

At the moment abstract_message_chain_t::close() method and close_drop_content()/close_retain_content() methods is not marked as noexcept.

It seems that mchain closing shouldn't be a throwing operation, because it's often called from destructors.

This moment should be explored with additional care and those methods/functions have to be marked as noexcept if is possible from the implementation's point of view.

eao197 commented 2 years ago

It seems that abstract_message_chain_t::close() can't be made noexcept. It's because the constructor of std::lock_guard can throw. And if that constructor throws then close() can't complete its work.

I think that the only right way is to allow an exception to go out from close(). It means that close() can't be noexcept.

Handling of such an exception is the task of code that calls close().

But in some cases close() is called in contexts where there are not many choices for exception handling. For example, in destructors and in catch blocks. In such contexts a user has to write something like:

try {
  ...
}
catch(...) {
  // Can't recover from an exception, force termination if close throws.
  [&mchain]() noexcept {
    mchain.close(so_5::mchain_props::close_mode_t::drop_content);
  }
}

But writing code like that is a boring and error-prone task. Maybe it's better to have another overload of close() method. Something like:

struct terminate_if_throws_t {};
inline terminate_if_throws_t terminate_if_throws;

class abstract_message_chain_t : ...
{
public:
  ...
  virtual void close(mchain_props::close_mode_t close_mode) = 0;

  void close(mchain_props::close_mode_t close_mode, terminate_if_throws_t /*unused*/ ) noexcept
  {
    this->close(close_mode); // std::terminate() will be called if close() throws.
  }
  ...
};
eao197 commented 2 years ago

And maybe the more correct solution is the following one:

struct terminate_if_throws_t {};
inline terminate_if_throws_t terminate_if_throws;

struct enable_exceptions_t {};
inline enable_exceptions_t enable_exceptions;

class abstract_message_chain_t : ...
{
public:
  ...
  [[deprecated]]
  void close(mchain_props::close_mode_t close_mode)
  {
    this->close(close_mode, enable_exceptions);
  }

  void close(mchain_props::close_mode_t close_mode, terminate_if_throws_t /*unused*/ ) noexcept
  {
    this->close(close_mode, enable_exceptions); // std::terminate() will be called if close() throws.
  }

  virtual void close(mchain_props::close_mode_t close_mode, enable_exceptions_t /*unused*/ ) = 0;
  ...
};

That approach gives some defense against cases where close(mode) is called instead close(mode, terminate_if_throws).

Unfortunately, that way requires breaking compatibility. So it can be regarded for SO-5.8, but not for the 5.7 branch.

eao197 commented 2 years ago

Will be a part of upcoming v.5.7.3.