alternative async_send using logger

[magni-mar]

No description provided.

[jbbjarnason]

Hmm 🤔 why return a function? I would rather like it being void.
Ex.
async_send(true, logger_);

[omarhogni]

I actually like the function return. The lifetime of the function is then on the caller side and
you don't end up referencing a logger inside ipc that has a questionable lifetime.

[magni-mar]

I agree with @omarhogni , ready to merge?

[jbbjarnason]

Nope if it returns a function it is not async_send

[jbbjarnason]

I actually like the function return. The lifetime of the function is then on the caller side and you don't end up referencing a logger inside ipc that has a questionable lifetime.

With this implementation you are keeping the reference of the signal within the lambda closure, so you can end up with dangling reference in both cases. So this impl does not solve safety.

As you know the safe way would be weak_ptr, either the signal would be stored as weak_ptr or the logger instance. I am still more inclined to the logger being passed and directly executated async_send.

[magni-mar]

That is correct, I implemented the function with the logger being a weak_ptr and the async_send being called directly. Please see changes.

[jbbjarnason]

Suggestions:

auto async_send(auto&& value, is_logger auto&& logger) -> void {
  static_assert(std::is_lvalue_reference_v<decltype(logger)>);
  return async_send(std::forward<decltype(value)>(value), [&logger, name = name()](std::error_code err, std::size_t){
    if (err) {
      logger.warn("{} unable to send reason: {}", name, err.message());
   }
  }
}
// or
// this is safe
template<is_logger logger_t>
auto async_send(auto&& value, std::weak_ptr<logger_t> logger) -> void {
  return async_send(std::forward<decltype(value)>(value), [logger,  name = name()](std::error_code err, std::size_t){
    if (err) {
      if (auto lock{ logger.lock() }) {
        logger->warn("{} unable to send reason: {}", name, err.message());
      } else {
        fmt::print(stderr, "{} unable to send reason: {}", name, err.message());
      }
   }
  }
}
// or 
auto async_send(auto&& value) -> void {
  return async_send(std::forward<decltype(value)>(value), [name = name()](std::error_code err, std::size_t){
    if (err) {
      logger::warn("{} unable to send reason: {}", name, err.message()); // static function call to exported logger func
   }
  }
}
// or 
auto async_send(auto&& value) -> void {
  return async_send(std::forward<decltype(value)>(value), [name = name()](std::error_code err, std::size_t){
    if (err) {
      fmt::print(stderr, "{} unable to send reason: {}", name, err.message()); // static function call to exported logger func
   }
  }
}
// or
// this is safe, but requires probably refactoring of many classes
auto make_callable_async_send(std::weak_ptr<logger_t> logger)  -> std::function<void(value_t)> {
  return [self = weak_from_this(), logger](value_t const& value){
    if (auto self_lock{ self.lock() }) {
      self_lock->async_send .....
    }
  };
}

[jbbjarnason]

@omarhogni , would you like some of the above suggestions or should we close this?

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[jbbjarnason]

will be discussed later if needed