Simple thread pool in C++
A few points:

Your EnterCriticalSection/LeaveCriticalSection code should be placed into a scoped RAII object. This saves you from explicitly calling leave, makes your code smaller, and avoids a deadlock if the synchronized code throws.

Do something similar with everything that requires cleanup (like mThreads = new HANDLE[mThreadCount];).

The interface of your class should receive the tasks by reference and store them by pointer within. This way, you make it clear to client code that the class doesn't take ownership of the tasks it receives. (see Third Edit below).

Use reinterpret_cast instead of C-style casts (because C-style casts = bad, mmmkay?!)

using namespace std;


Don't do this. It greatly restricts what client code (whoever includes your class) can write, and the errors generated in client code are almost always obscure. You're better off repeating that in local scopes (if you must) or (better still) not using it (consider writing using std::string; and similar, in the scopes you use the type, or simply declaring std::string x;).

Edit: If bool ThreadPool::ScheduleTask(Task* task) never returns false, why does it return bool? Shouldn't it be void?

Second Edit: Consder using an unsigned int for the number of threads (mThreadCount).

Third edit ( Hi Loki :) )

Regarding ownership of the threads and object lifetime management (how to implement safely and indicate it in the public interface of the class).

Consider this client code:

ThreadPool runner();

// bad bad code: don't call new with raw pointers like this in practice
// but easy to write to make my point below.
BackgroundTask *t = new BackgroundTask(); // class BackgroundTask: public Task
                                          // defined elsewhere

runner.ScheduleTask(t);


What you don't know looking at this code: should you delete t after the ThreadPool completes running? Will runner delete the task itself? If it does, t will point to deleted memory after runner finishes. If it doesn't, you will have to remember to check when runner has finished, and call delete explicitly, every single time you use the thread pool.

You could implement the ThreadPool to delete all tasks after execution, but then, an enterprising programmer (maybe yourself) could write:

ThreadPool runner();
BackgroundTask t();
runner.ScheduleTask(&amp;t); // pass address of automatic storage object


and this will crash your program when your ThreadPool deletes the task.

Another client code alternative, this one much much worse:

void crashy(ThreadPool&amp; pool) {
    BackgroundTask t;
    pool.ScheduleTask(&amp;t);
} // t goes out of scope here and is destroyed but p still holds it's address

ThreadPool p;
crashy(p); // some compilers tend to happily accept this (maybe with a warning)
// p now has a pointer to a deleted BackgroundTask instance


The interface doesn't indicate ownership and makes you interrupt your coding, to look at the implementation, just so you know how to write client code that doesn't leak or crash.

Solutions:

You could change your interface to take a Task&amp; instead. (Similar to passing by pointer), the only drawback is that you need to make sure the tasks you pass to ScheduleTask must still be in scope until the ThreadPool stops running (in other words, you can still write the nice crashy function).

This is better, as at least you don't need to wonder if it will delete the task (which passing by pointer may, or may not suggest).

Better yet, change your interface to take a std::unique_ptr instead:

class ThreadPool
{
public :
    bool ScheduleTask(std::unique_ptr&lt;Task&gt; task) // &lt;&lt; std::unique_ptr by val.
    {
        EnterCriticalSection(&amp;mCriticalSection);
        mTasks.push_back( std::move(task) ); // &lt;&lt; notice std::move
        mNumberOfTasksNotFinished++;
        LeaveCriticalSection(&amp;mCriticalSection); // also notice non-raii unlock;
                                                 // if push_back throws on 
                                                 // allocating new list node,
                                                 // your whole app is deadlocked

        WakeConditionVariable(&amp;mConditionVariable);
        return true;
    }

private :

    list&lt;std::unique_ptr&lt;Task&gt;&gt; mTasks; // &lt;&lt; notice std::unique_ptr
    ... 
};


Client code:

ThreadPool runner();
std::unique_ptr&lt;Task&gt; p(new BackgroundTask());
runner.ScheduleTask(std::move(p)); // works, no lifetime management problems
                                   // and no ambguity either


Since the function takes a unique_ptr now, you now know you need to pass a pointer and you also know you are passing ownership as well.