mtrebi
commented
6 years ago First of all, Thank you! I really appreciate your time and effort to look into my code.
So, I wrote the SafeQueue with the idea of being independent from the ThreadPool so that, this implementation can be used in any multi-threading application. This is why needs its own mutex.
Then, the second mutex (the m_conditional_mutex) has a different purpose than locking the queue. This mutex, as its name implies, is used to conditionally wait when the queue is empty. It waits until it receives a signal via notify_all (executed when adding a new task in the pool). This turns out to be very useful and efficient because we can don't have to keep executing the loop and checking the queue status (polling) all the time. This would waste resources. Instead, we stop the execution when the queue is empty, and we continue as soon as a new task is added.
Regarding the m_shutdown variable, I am not sure if it should have a lock. This variable is not really a shared resource because it belongs to the pool. The value of this variable is modified twice. Once in the constructor of the Pool and once in the shutdown. It's never modified from the individual threads. For this reason, its actually not a shared resource.
Hope this helps!
PS> My apologies for the late reply, but I've started a new job in a new city and I've been really busy lately.
Great implementation of thread pool! It is short and clear.
I have a question:
There is "m_conditional_mutex" in class ThreadPool And there is "m_mutex" in class SafeQueue
Do these two mutex serve the same purpose i.e. to lock the queue? Queue is the shared resource that everybody try to get hold of. So we need a lock. But one lock is enough.
Another potential issue is that, "m_shutdown" in class ThreadPool is also a shared resource. It should have a lock associated with it.
These are the only two shared resources that existed in the system. Therefore, two locks are required in total.