http://codereview.stackexchange.com/questions/84697/timeout-watchdog-using-a-standby-thread
he simple but generic timeout class to be used watching for network connections, user input, filesystem events, and is intended to have a very simple interface specific to only our use cases (i.e. no satisfy-all attitude).
Intended steps to use:
- Construct
- Activate
- Potentially react to a timeout, deactivates itself
- Re-activate
- Destruct cleanly
After triggering the alarm
the guard is expected to be inactive until explicitly activated. The code expected to be reasonably tested (that is one of the issues).
This posted code as a testable Visual Studio 2013 project lives on the GitHub.
Besides a general feedback on the code quality - or if, please, please, please, you see a bug, I would love to hear about these areas:
-
Destruction. Although I did my best to tell the guard thread to end, I am still concerned about having to
join()
in the destructor. Generally I love my destructors short and sweet for emergency landings - is it possible here? Is there an STL way to brutally kill that thread? -
Tests. Existing ones test for intended simple scenarios. I am not sure this is enough to claim that the code works as intended. Is it? I did not find a better way to test timing edge cases. Also, as tests are time-dependant, they occasionally spuriously fail when run on slow VMs. Or are they? Is it sufficient for code like this to know that if tests run somewhere consistently? If I increase timeouts the spurious fails go away, but that lengthens the overall project test run.
Below are the current header, implementation, and tests files to save the GitHub trip.
Header:
#pragma once
namespace utility
{
/**
The `clock` alias is for easy switching to `steady_clock` once Microsoft fixes it
*/
typedef std::chrono::system_clock clock;
/**
The `TimeoutGuard` class triggers the `alarm` callback from the `guard_thread`
if `touch` was not called for at least the `timeout` duration.
Because of the way the `guard_thread` sleeps, the actual detection may happen
as late as after `timeout` + `naptime` duration. Hence it is possible that the alarm
will not be called if the `TimeoutGuard` instance is touched within the
'timeout` and `timeout` + `naptime` timeframe.
If not provided, by default the `naptime` is same as `timeout`.
The `TimeoutGuard` is not active after construction, whicn means, that the
`guard_thread` will block until it is activated by calling the `watch` method.
The `TimeoutGuard` class is not copyable and not moveable.
*/
class TimeoutGuard
{
public:
TimeoutGuard(
clock::duration timeout,
std::function<void( void )> alarm,
clock::duration naptime
);
TimeoutGuard(
clock::duration timeout,
std::function<void( void )> alarm
);
~TimeoutGuard();
TimeoutGuard( const TimeoutGuard & ) = delete;
TimeoutGuard & operator=(const TimeoutGuard & ) = delete;
TimeoutGuard( TimeoutGuard && ) = delete;
TimeoutGuard & operator=( TimeoutGuard && ) = delete;
void watch();
void touch();
private:
void guard();
clock::duration timeout;
clock::duration naptime;
std::function<void( void )> alarm;
std::atomic_bool idle;
std::atomic_bool live;
std::atomic<clock::time_point> touched;
std::thread guard_thread;
std::mutex guard_mutex;
std::condition_variable wakeup;
};
}
Here is the implementation:
#include "stdafx.h"
#include "TimeoutGuard.h"
namespace utility
{
TimeoutGuard::TimeoutGuard(
clock::duration timeout,
std::function<void( void )> alarm,
clock::duration naptime
)
: timeout( timeout )
, alarm( alarm )
, naptime( naptime )
{
idle.store( true );
live.store( true );
guard_thread = std::thread( std::bind( &TimeoutGuard::guard, this ) );
}
TimeoutGuard::TimeoutGuard(
clock::duration timeout,
std::function<void( void )> alarm
)
: TimeoutGuard( timeout, alarm, timeout )
{};
TimeoutGuard::~TimeoutGuard()
{
live.store( false );
wakeup.notify_all();
guard_thread.join();
}
void TimeoutGuard::guard()
{
while ( live.load() )
{
if ( idle.load() )
{
// Sleep indefinitely until either told to become active or destruct
std::unique_lock<std::mutex> live_lock( guard_mutex );
wakeup.wait( live_lock, [this]() { return ! this->idle.load() || ! this->live.load(); } );
};