C++ function timeout wrapper

Question

Based on How to implement timeout for function in c++, I wrote this wrapper:

template <typename t_time, typename t_function, typename... t_params>
inline std::conditional_t<
    // if 't_function' return type is 'void'
    std::is_void_v<std::invoke_result_t<t_function, const bool &, t_params...>>,
    // the 'execute' wrapper will return 'bool', which will be 'true' if the
    // 'p_function' executes in less 'p_max_time', or 'false' otherwise
    bool,
    // else it will result a 'std::optional' with the return type of
    // 't_function', which will contain a value of that type, if the
    // 'p_function' executes in less 'p_max_time', or empty otherwise
    std::optional<std::invoke_result_t<t_function, const bool &, t_params...>>>
execute(t_time p_max_time, t_function &p_function, t_params &&... p_params) {
  std::mutex _mutex;
  std::condition_variable _cond;
  bool _timeout{false};

  typedef typename std::invoke_result_t<t_function, const bool &, t_params...>
      t_ret;

  if constexpr (std::is_void_v<t_ret>) {
    std::thread _th([&]() -> void {
      p_function(_timeout, std::forward<t_params>(p_params)...);
      _cond.notify_one();
    });

    std::unique_lock<std::mutex> _lock{_mutex};
    if (_cond.wait_for(_lock, p_max_time) != std::cv_status::timeout) {
      _th.join();
      return true;
    }
    
    _timeout = true;
    _th.detach();
    return false;
  } else {
    t_ret _ret;

    std::thread _th([&]() -> void {
      _ret = p_function(_timeout, std::forward<t_params>(p_params)...);
      _cond.notify_one();
    });

    std::unique_lock<std::mutex> _lock{_mutex};
    if (_cond.wait_for(_lock, p_max_time) != std::cv_status::timeout) {
      _th.join();
      return {std::move(_ret)};
    }
    _timeout = true;
    
    _th.detach();
    return {};
  }
}

Unlike the code in the answers for the question I referenced, I would not like to throw an exception in the execute wrapper. If p_function does not return, the wrapper will return a bool, true if p_function executed in at most p_max_time, false otherwise. If p_function returns T, the wrapper will return std::optional<T> which will have a value if p_function does not exceed p_max_time, or it will be empty otherwise.

The const bool & parameter required for p_function is used to inform p_function that its execution exceeded p_max_time, so p_function may stop its execution, though execute will not count on it.

Here is an example:

    auto _function = [](const bool &p_is_timeout, int &&p_i) -> void {
      std::this_thread::sleep_for(1s);
      if (p_is_timeout) {
        std::cout << "timeout";
      } else {
        std::cout << "i = " << p_i << '\n';
      }
    };
    int _i{4};

    if (!async::execute(200ms, _function, std::move(_i))) {
      std::cout << "TIMEOUT!!\n";
    }

So, the problem is _th.detach() causes crash when I execute some test functions in a row. I if change it to _th.join(), the crash no longer occurs, but, obviously, the function that calls the wrapper has to wait for p_function to end, which is not desired.

How can I make execute detach _th thread, without causing crash?

Answer 1

Your lambda needs access to the local variables _ret and cond these don't exist after the end of execute so your code has undefined behaviour. Lambdas that capture by reference should only be used when the lambda has the same lifetime as the code that they're defined in.

You'd need to define your state variables in the heap so that they exist after the end of the function. For example you could use a shared_ptr:

template <typename Result>
struct state
{
  std::mutex _mutex;
  std::condition_variable _cond;
  Result _ret;
};
template <>
struct state<void>
{
  std::mutex _mutex;
  std::condition_variable _cond;
};

template <typename t_time, typename t_function, typename... t_params>
inline std::conditional_t<
    // if 't_function' return type is 'void'
    std::is_void_v<std::invoke_result_t<t_function, const bool &, t_params...>>,
    // the 'execute' wrapper will return 'bool', which will be 'true' if the
    // 'p_function' executes in less 'p_max_time', or 'false' otherwise
    bool,
    // else it will result a 'std::optional' with the return type of
    // 't_function', which will contain a value of that type, if the
    // 'p_function' executes in less 'p_max_time', or empty otherwise
    std::optional<std::invoke_result_t<t_function, const bool &, t_params...>>>
execute(t_time p_max_time, t_function &p_function, t_params &&... p_params) {
  typedef typename std::invoke_result_t<t_function, const bool &, t_params...>
      t_ret;
  auto _state = std::make_shared<state<t_ret>>();
  bool _timeout{false};

  if constexpr (std::is_void_v<t_ret>) {
    std::thread _th([&, _state]() -> void {
      p_function(_timeout, std::forward<t_params>(p_params)...);
      _state->_cond.notify_one();
    });

    std::unique_lock<std::mutex> _lock{_state->_mutex};
    if (_state->_cond.wait_for(_lock, p_max_time) != std::cv_status::timeout) {
      _th.join();
      return true;
    }
    
    _timeout = true;
    _th.detach();
    return false;
  } else {
    std::thread _th([&, _state]() -> void {
      _state->_ret = p_function(_timeout, std::forward<t_params>(p_params)...);
      _state->_cond.notify_one();
    });

    std::unique_lock<std::mutex> _lock{_state->_mutex};
    if (_state->_cond.wait_for(_lock, p_max_time) != std::cv_status::timeout) {
      _th.join();
      return {std::move(_state->_ret)};
    }
    _timeout = true;
    
    _th.detach();
    return {};
  }
}

Note for simplicity I've left in the capture of the function arguments and function by reference but you still need to ensure those references remain valid for as long as they're needed (e.g. if the timeout is short the function could exit before the target function executes or if the arguments are references then the invoked function can't use those references after the function exits).

If you have c++20 you might want to look into std::jthread

Answer 2

Based on answers and suggestions, I came up with:

template <typename t_time, typename t_function, typename... t_params>
inline std::conditional_t<
    // if 't_function' return type is 'void'
    std::is_void_v<
        std::invoke_result_t<t_function, std::function<bool()>, t_params...>>,
    // the 'execute' wrapper will return 'bool', which will be 'true' if the
    // 'p_function' executes in less 'p_max_time', or 'false' otherwise
    bool,
    // else it will result a 'std::optional' with the return type of
    // 't_function', which will contain a value of that type, if the
    // 'p_function' executes in less 'p_max_time', or empty otherwise
    std::optional<
        std::invoke_result_t<t_function, std::function<bool()>, t_params...>>>
execute(t_time p_max_time, t_function &p_function, t_params &&... p_params) {
  std::mutex _mutex;
  std::condition_variable _cond;
  auto _timeout = std::make_shared<bool>(false);

  auto _is_timeout = [_timeout]() { return *_timeout; };

  typedef typename std::invoke_result_t<t_function, std::function<bool()>,
                                        t_params...>
      t_ret;

  if constexpr (std::is_void_v<t_ret>) {
    std::thread _th([&]() -> void {
      p_function(_is_timeout, std::forward<t_params>(p_params)...);
      _cond.notify_one();
    });

    std::unique_lock<std::mutex> _lock{_mutex};
    if (_cond.wait_for(_lock, p_max_time) != std::cv_status::timeout) {
      _th.join();
      return true;
    }
    *_timeout = true;
    _th.join();
    return false;
  } else {
    t_ret _ret;

    std::thread _th([&]() -> void {
      _ret = p_function(_is_timeout, std::forward<t_params>(p_params)...);
      _cond.notify_one();
    });

    std::unique_lock<std::mutex> _lock{_mutex};
    if (_cond.wait_for(_lock, p_max_time) != std::cv_status::timeout) {
      _th.join();
      return {std::move(_ret)};
    }
    *_timeout = true;
    _th.join();
    return {};
  }
}

And the example becomes:

    auto _function = [](std::function<bool()> p_timeout, int &&p_i) -> void {
      std::this_thread::sleep_for(1s);
      if (p_timeout()) {
        std::cout << "timeout in work function\n";
      } else {
        std::cout << "i = " << p_i  << '\n';
      }
    };
    int _i{4};

    if (!execute(200ms, _function, std::move(_i))) {
       std::cout << "OK - timeout\n";
    }
    else {
       std::cout << "NOT OK - no timeout\n";
    }

I believe passing a std::function<bool()> to the work function (p_function) creates a good abstraction on how execute will control the timeout, and an easy way for p_function to check for it.

I also removed the std::thread::detach() calls.