Portable periodic/one-shot timer thread - follow-up

This is a much improved version (I hope) of code in this previous code review of mine.

I have:

• renamed several things. Are the names good?

• made it lazily start the worker thread only after the first timer is added. Is it clear that the implementation uses one worker thread to handle an unlimited number of timer callbacks?

• added the ability to bind additional arguments to the timer callback.

• added helper functions that match the API of setTimeout and setInterval from JavaScript. I expect many developers to immediately recognize it and know how to use it right away. Do you agree or are they completely redundant with addTimer?

• made it completely thread safe (as far as I can tell), and added guarantees that make it easy to use reliably. Can you see any concurrency problems?

• tried to make the class much more self-documenting. Does the header file provide sufficient information at the right level of detail? I tried to document the thread-safety guarantees I provide in the code. Are they sufficiently specified?

Questions:

• Do you think it is a good idea to add an RAII class that automatically calls clearTimeout for a timer when it goes out of scope? Should every resource acquisition have a scope guard?

• Have I used const and noexcept appropriately?

timerthread.h

```
#ifndef TIMERTHREAD_H
#define TIMERTHREAD_H

#include
#include
#include
#include
#include
#include

#ifdef HAVE_PLATFORMTHREAD_H
#include "platformthread.h"
#else
#include
#include
#include
#endif

class TimerThread
{
public:
// Each Timer is assigned a unique ID of type timer_id
using timer_id = std::uint64_t;

// Valid IDs are guaranteed not to be this value
static timer_id constexpr no_timer = timer_id(0);

//
// Callback storage

// Function object we actually use
using handler_type = std::function;

// Function object that we boil down to handler_type with std::bind
template
using bound_handler_type = std::function;

I think I spotted a possible deadlock. It will occur in case clearTimer or clear are called from within a timer handler (i.e. from the worker thread).

The statement timer.waitCond->wait(lock); in destroy_impl() will block forever since the corresponding notify_all is done from timerThreadWorker() after the handler callback has returned.

To fix this, I propose changing it to:

if (std::this_thread::get_id() != worker.get_id()) {
    timer.waitCond->wait(lock);
}

StackExchange Code Review Q#127552, answer score: 7