HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Threadpool implementation in C++11

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationthreadpoolstackoverflow

Problem

I am implementing a very primitive threadpool with C++11 using mutex and conditional variable. The class ThreadPool is the main thread holder and will spawn a number of threads upon construction, and join all threads on destruction. The main thread adds task of type function into the queue and then wakes up any waiting threads through condition variable.

As a new C++11 learner, is there anything I can improve for the following code (or fix bugs)? For example, making the task function more generic and implementing the lock-free mechanism (best practice?).

```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include

// Class: ThreadPool
class ThreadPool {

// Class: Worker
class Worker {

public:

// Constructor.
explicit Worker(ThreadPool* master) : _master{master} {};

// Functor.
void operator ()() {

::std::function task;

while(true) {
{ // Acquire lock.
::std::unique_lock lock(_master->_mutex);

_master->_condition.wait(
lock,
[&] () { return !_master->_tasks.empty() || _master->_terminate.load(); }
);

if(_master->_terminate.load()) return;

task = _master->_tasks.front();
_master->_tasks.pop_front();
} // Release lock.

task();
}
}

private:

ThreadPool* _master;
};

public:

ThreadPool(size_t);
template
void enqueue(F f);
~ThreadPool();

ThreadPool(const ThreadPool&) = delete;
ThreadPool& operator = (const ThreadPool&) = delete;

private:

friend class Worker;

::std::vector _workers;
::std::deque > _tasks;
::std::mutex _mutex;
::std::condition_variable _condition;
::std::atomic_bool _terminate {false};
};

// Constructor
ThreadPool::ThreadPool(size_t threads) {
for(size_t i = 0;i
void ThreadPool::enqueue(F f) {

{ // Acquire lock
::std::unique_lock guard(_mutex);
_t

Solution

I don't see why this should have a hypothetical name:

void foo() {
    printf("hello by %p\n", ::std::this_thread::get_id());
    sleep(1); 
}


If this is meant as a test function, then name it as such. As obvious as the functionality may be, there's still need for such names in an otherwise serious program.

Also, you can use std::this_thread::sleep_for, which is as of C++11:

std::this_thread::sleep_for(std::chrono::seconds(1));


If you also have C++14, use a literal instead:

std::this_thread::sleep_for(1s);


(You'll also need `` for all of this.)

Code Snippets

void foo() {
    printf("hello by %p\n", ::std::this_thread::get_id());
    sleep(1); 
}
std::this_thread::sleep_for(std::chrono::seconds(1));
std::this_thread::sleep_for(1s);

Context

StackExchange Code Review Q#126026, answer score: 7

Revisions (0)

No revisions yet.