Easy to use Thread Pool Implementation awhile ago I asked about a thread pool implementation I had made. I took the advice down and modified it a lot over time to fit my needs. It has been serving me extremely well in all of my projects but I was wondering if there was anything else I could improve on?

Here is the class:

class ThreadPool{
public:
    /*  Constructor */
    ThreadPool(uint8_t numThreads) {
        assert(numThreads &gt; 0);
        createThreads(numThreads);
    }

    //Destructor
    ~ThreadPool(){
        shutdown = true;
        notifier.notify_all();
        for(int i = 0; i &lt; threads.size(); ++i){
            threads[i].join();
        }
    }

    //add any arg # function to queue
    template &lt;typename Func, typename... Args &gt;
    auto push(Func&amp;&amp; f, Args&amp;&amp;... args){

    //get return type of the function
    typedef decltype(f(args...)) retType;

    std::packaged_task&lt;retType()&gt; task(std::bind(f, args...));

    std::future&lt;retType&gt; future = task.get_future();

    {
        // lock jobQueue mutex, add job to the job queue 
        std::unique_lock&lt;std::mutex&gt; lock(JobMutex);

        jobQueue.emplace(std::packaged_task&lt;void()&gt;(std::move(task)));
    }
    notifier.notify_one();

    return future;
    }

private:

    std::vector&lt;std::thread&gt; threads;
    std::queue&lt;std::packaged_task&lt;void()&gt;&gt; jobQueue;
    std::condition_variable notifier;
    std::mutex JobMutex;
    std::atomic&lt;bool&gt; shutdown = false;

    void createThreads(uint8_t numThreads) {
        auto threadFunc = [this]() {
            while (true) {
                std::packaged_task&lt;void()&gt; job;

                {
                    std::unique_lock&lt;std::mutex&gt; lock(JobMutex);
                    notifier.wait(lock, [this] {return !jobQueue.empty(); });

                    if(shutdown){
                        break;
                    }

                    //strange bug where it will continue even if the job queue is empty
                    if (jobQueue.size() &lt; 1)
                        continue;

                    job = std::move(jobQueue.front());

                    jobQueue.pop();
                }
                job();
            }

        };
        threads.reserve(numThreads);
        for (int i = 0; i != numThreads; ++i) {
            threads.emplace_back(std::thread(threadFunc));
        }
    }


    ThreadPool(const ThreadPool&amp; other) = delete; 
    void operator=(const ThreadPool&amp; other) = delete;

}; /* end ThreadPool Class */
You have a logical bug in the code. Imagine all jobs are finished and then shutdown is called. You'll never leave the notifier.wait(...). 

Also, if a job exists but shutdown is called you quit instantly without doing it - I do not think that this is a good idea. I think you should finish all jobs prior to quitting. Make a special hard reset routine for the immediate exit. Though, it probably requires usage of exceptions.

Also, consider what will happen if an exception is called inside the function f. In the thread pool one of the threads will die and kill the whole program - most likely. It is not good. It should somehow forward the error to the caller. To this end I have thread pool functionality divided - made a supplementary class for user that wraps whatever additional functionality is needed.

Technical issues:


std::bind(f, args...) should be std::bind(f, std::forward&lt;Args&gt;(args)...) to ensure perfect forwarding as otherwise you might end up with unnecessary copying of data.
numThreads should be defaulted to std::thread::hardware_concurrency and I believe and uint8_t is getting too small for some of the latest processors. Additionally the ThreadPool should have the functionality to tell to its user how many threads it has.
I am not too fond of std::future - it is somewhat incomplete as Executors TS is still incomplete. Normally I'd expect that user would want to call the push task several times to complete a single operation. In which, case you generate multiple std::future&lt;void&gt;. It would be better to somehow to make it into a single wait instead of several. But I think I might be nitpicking - honestly I'd wait for Executor TS near completion and try to implement or get from somewhere something with the same interface as long as Executor TS is unavailable.
The declarations 

ThreadPool(const ThreadPool&amp; other) = delete; 
void operator=(const ThreadPool&amp; other) = delete;



are redundant as the class already has a std::mutex that makes the whole class non-copyable and non-movable. Yeah, it isn't important.