ThreadPool review

[warm basalt]

#include <bits/stdc++.h>
#include <mutex>
#include <vector>
#include <condition_variable>
#include <iostream>
#include <functional>
#include <queue>

using namespace std;

class ThreadPool {

public:
    ThreadPool(int _threads) : stop(false) {
        for (int i = 0; i < _threads; ++i) {
            threads.emplace_back([this] { executeTask(); });
        }
    }

    ~ThreadPool() {

        stop = true;
        for (auto& thread : threads) {
            if (thread.joinable()) {
                thread.join();
            }
        }
    }

    void addTask(function<void()> task) {
        {
            unique_lock<mutex> lock(m);
            functionQueue.push(task);
        }
        cv.notify_one(); 
    }

private:
    void executeTask() {
        while (true) {
            function<void()> task;
            {
                unique_lock<mutex> lock(m);
                cv.wait(lock, [this] { return stop || !functionQueue.empty(); });

                if (stop && functionQueue.empty()) {
                    return;
                }

                task = functionQueue.front();
                functionQueue.pop();
            }
            task();
        }
    }

    vector<thread> threads;
    condition_variable cv;
    mutex m;
    queue<function<void()>> functionQueue;
    bool stop;
};

int main() {
    return 0;
}

Hi so i am practicing multithreading and would like some thoughts on this basic implementation of a thread pool ,if there is something i could do better or some thing i could add

[devout nymph]

- #include <bits/stdc++.h>

- using namespace std;

[harsh tinsel]

You've got a race condition there. addTask() can push into the queue while a thread is in the critical section of executeTask(). If P is a thread pool thread who currently has the lock in executeTask, and Q is a an external thread holding another lock in addTask(), then at least to me it looks like this sequence of interleaved events can happen:

// [P]
task = functionQueue.front(); // <- takes a reference of the
                              //    task currently in front
// [Q]
functionQueue.push(task); // <- pushes a new task in front of
                          //    the one referenced by 'task'
// [P]
functionQueue.pop(); // <- discards the new task instead of
                     //    the one referenced by 'task'

The result is that the newly-added task is removed without ever being executed, and instead the task that was just executed will remain at the top of the queue for the next round.