Terminating custom std::thread pool

Question

Ive been playing around with multithreaded game engine architecture and thread pools lately. Now ive implemented a basic Kernel class. This class has an std::vector<std::thread>, which represents the threadpool. Now, the following function is run by a single thread in the pool:

while(m_IsRunning)
{
    std::unique_lock<std::mutex> kernelstateLocker(m_KernelStateMutex);
    m_KernelStateCond.wait(kernelstateLocker);
    if(m_KernelState == KernelState::KernelShutdown || m_KernelState == KernelState::KernelTerminate)
    {
        kernelstateLocker.unlock();
        //std::cout << "Worker #" << _workerID << std::endl console log here
        break;
    }
    else if(m_KernelState == KernelState::KernelWorkAvailable)
    {
        ...
}

As you can see, a thread wakes up if the KernelState variable changes. This can happen when a Task is added to the queue, or when the Kernel shuts down. The kernel shutdown condition variable gets called by the main program thread, via m_KernelStateCond.notify_all(). However, as i added cout's as seen in the comment, only one of at times up to 8 worker threads would print its name and id, indicating that the others never terminated. Does anybody know why this is, and how i can terminate all of the threads in my pool? In case it matters, my platform is TDM-GCC-64 5.1 on Windows 10 64-bit.

Update:

As per comment request and SO rules, here is the code that calls the condition variable.

std::unique_lock<std::mutex> shutdownLocker(m_IsRunningMutex);
m_ShutdownCond.wait(shutdownLocker, [this](){ return !m_IsRunning; });
if(!m_IsRunning)
{
    shutdownLocker.unlock();
    m_KernelStateMutex.lock();
    m_KernelState = KernelState::KernelShutdown;
    m_KernelStateMutex.unlock();
    m_KernelStateCond.notify_all();
}

Im pretty sure this part of my code is working, since at least one of the thread workers actually closes. And for completeness, here is my full Kernel class:

class Kernel : public Singleton<Kernel>
{
public:
    void boot(unsigned int _workerCount);
    void run();
    void shutdown();

    void addTask(std::shared_ptr<Task> _task);
private:
    friend class Singleton<Kernel>;
    Kernel();
    ~Kernel();

    bool m_IsRunning;
    KernelState m_KernelState;

    std::vector<std::thread> m_Workers;

    std::queue<std::shared_ptr<Task>> m_Tasks;
    std::vector<std::shared_ptr<Task>> m_LoopTasks;

    std::condition_variable m_KernelStateCond;
    std::mutex m_KernelStateMutex;

    void workTask(unsigned int _workerID);
};

Please post a [mcve]. If I were to guess, it's because you do not acquire the mutex before notifying the condition variable. In order to guarantee correct sequencing of all related threads, the same mutex that's used to wait on a condition variable must also be acquired in order to notify the same condition variable. However, since you did not show the relevant code, no authoritative answer can be given, until you edit your post and include a [mcve], with the emphasis on both "minimal" AND "complete" part. — Sam Varshavchik, Mar 10 '16 at 13:37
P.S. You do not need an explicit "kernelstateLocker.unlock();" when the unique lock goes out of scope and gets destroyed, this gets taken care of automatically. — Sam Varshavchik, Mar 10 '16 at 13:38
It might be easier to use one of many existing thread pool implementations. With boost::asio it's even easier ([example](http://stackoverflow.com/questions/19500404/how-to-create-a-thread-pool-using-boost-in-c)) — rustyx, Mar 10 '16 at 13:48
@SamVarshavchik well true, youre right about the unique lock. forgot about that. Updated my answer, is that better? — tubberd, Mar 10 '16 at 14:01
@rustyx: im not a fan of boost, for my own reasons, id like to avoid it where the STL and custom implementations can do the job just fine. — tubberd, Mar 10 '16 at 14:01
This is not a [minimal complete verifiable example](http://stackoverflow.com/help/mcve). I can not copy the code into my IDE, make it run and reproduce your problem. Therefore all my would be answers are guesses and I'd rather not make myself look stupid. The code you post has to run on something like http://ideone.com and reproduce the problem, otherwise there is very little chance you will get a useful answer. — nwp, Mar 10 '16 at 14:09
@calcyss, no, you are to post MCVE. Reduce the code to the minimal version where the problem is reproducible. By the way, while doing so, you are likely to found the problem on your own! — SergeyA, Mar 10 '16 at 14:14
Is m_isRunning supposed to be protected by the weird m_IsRunningMutex (which I don't see declared anywhere), or by the m_KernelStateMutex? Either way, the worker doesn't seem to be synchronizing access to this boolean correctly — Useless, Mar 10 '16 at 14:20
well, the boolean is read-only for the workers and is only written to at the start of the program, with `Kernel::boot` so i thought a mutes wasn't necessary on the workers end, since they sleep 90% of the time. — tubberd, Mar 10 '16 at 14:27
okay guys hold on, i'll wrap my head around it and rewrite this code later on so i can give you a clear, reproducable example. I just don't have the time right now, so please be patient and don't close this question (yet). — tubberd, Mar 10 '16 at 14:28

score 0 · Accepted Answer · answered Mar 10 '16 at 21:05

I figured the problem out. It was not a problem with my thread pool implementation per se, but it had to do with the fact that there were still tasks arriving while the kernel was shutting down. So some of the worker threads never shut down, and thus, the kernel got stuck. Adding constraints to the tasks solved this.

Terminating custom std::thread pool

Update:

1 Answers1