Thread pool worker implementation

As an exercise in using C++11 features I decided to make a thread pool class.

I would like to have a review on the code with focus on:

• Standards compliance / Portability issues / Best practices

• Thread Safety / Exception handling

• API

• Performance issues (Yes I know std::mutex is slow on windows, but besides that :)

• Any ideas on how I can get rid of the specialization on void return type?

I have inlined all methods in the class declaration to make it easier to read here. In the actual implementation they are not inlined. Sorry for the failed line-breaks on some places, the CR code window is a bit narrower than I'd like.

```
#include
#include
#include
#include

/// A typical thread worker queue that can execute arbitrary jobs.
///
///
///
/// * Thread Safety : Full.
/// * Exception Safety: Strong.
class WorkQueue{
public:
/// Constructors a new work queue object.
/// (Optional) number of workers, less than 0 to
/// auto-detect (may fail on esoteric platforms).
explicit WorkQueue(int numWorkers = -1){
if (numWorkers Will abort all pending jobs and run any in-progress jobs to
/// completion upon destruction.
~WorkQueue(){
abort();
}

/// Stops work queue and finishes jobs currently being executed.
/// Queued jobs that have not begun execution will have their promises
/// broken.
void abort(){
m_exit = true;
m_finish_work = false;
m_signal.notify_all();
joinAll();

{
std::lock_guard lg(m_mutex);
m_work.clear();
}
}

/// Stops new work from being submitted to this work queue.
void stop(){
m_exit = true;
m_finish_work = true;
m_signal.notify_all();
}

/// Wait for completion of all submitted work. No more work will
/// be allowed to be submitted.
void waitForCompletion(){
stop();
joinAll();
assert(m_wo

Please note that the code should be presented in the same way that you use it to avoid spurious errors. Compiling your code as is, gave me an error on the specialization of submit.

Naming

• pair_t is a rather generic name that does not tell what is stored

• m_work could be improved a little although I don't have any idea yet

• m_mutex should be renamed to m_work_mutex to indicate what it is for (or bundle it together with m_work to make it impossible to access m_work without locking m_mutex)

• m_signal could take at least some explanation about its uses: what does it signal? Maybe the name can reflect this as well.

• m_exit should be named something like m_accept_no_more_work (or accept_more_work and its logic should be inverted because negation in a (boolean) variable name makes reasoning harder)

• m_workers could be name m_worker_threads

Derive the return type automatically

Why bother the user with giving the return type to submit when it can be extracted?

template 
auto submit(FunctionObject &&function) -> std::future;

Remove code duplication in submit

Your two submit functions have largely the same code which indicates that your switch on the template parameter is too coarse. The only differing part is in the try{} block. The obvious solution is to extract the differing parts into an own function. For shortness I will use a template function specialization:

template <>
inline void WorkQueue::execute_and_set_data(const DataPointer &data) {
    data->second();
    data->first.set_value();
}

template 
void WorkQueue::execute_and_set_data(const DataPointer &data) {
    data->first.set_value(data->second());
}

These functions use some template types:

template 
using PromiseFunctionPair =
    std::pair, std::function>;

template 
using DataPointer = std::shared_ptr>;

And they replace the call in the try catch block. Because of the nontrivial dependency between ReturnType and these types they cannot be used for automatic template parameter deduction:

try {
    execute_and_set_data(data);
}

However, I would recommend to replace PromiseFunctionPair by a templated struct with better names for the members than first and second which would then allow automatic template parameter deduction.

Missing includes

• you are using std::vector without #include

• you are using assert without #include

Typo

• f.get() should probably be f0.get()?

Besides the mentioned flaws I really enjoyed reading your code :)

StackExchange Code Review Q#60363, answer score: 13