Synchronization on shutdown. about thread-pool HOT 5 OPEN

I don't think that this is sufficient. The wait predicate AFAIK is to guard against spurious wakeup. It's probably recommended, but I don't think that it solves the race condition.

Your call operator should look more like this:

void operator()()
{
  std::unique_lock<std::mutex> lock(m_pool->m_conditional_mutex);
  while (!m_pool->m_shutdown) {
  {
    if (m_pool->m_queue.empty()) {
      m_pool->m_conditional_lock.wait(lock, [this] { return this->m_pool->m_shutdown || this->m_pool->m_queue.empty() == false; });
    }
    std::function<void()> func;
    if (m_pool->m_queue.dequeue(func)) {
      func();
    }
  }
}

The problem described is then fixed by locking the mutex in your shutdown() function. Note that wait() releases the lock when called and acquires the lock when woken. You need to acquire the lock in order to prevent wait() being called after the call to notify_all() in shutdown().

void shutdown()
{
  {
    std::lock_guard<std::mutex> lock(m_conditional_mutex);
    m_shutdown = true;
    m_conditional_lock.notify_all();
  }
  
  for (int i = 0; i < m_threads.size(); ++i)
  {
    if (m_threads[i].joinable())
    {
      m_threads[i].join();
    }
  }
}

Similarly, in submit(), you need to acquire the lock before calling notify_all() to prevent notify_all() being called between checking whether the queue is empty and calling wait().

{
  std::lock_guard<std::mutex> lock(m_conditional_mutex);
  m_conditional_lock.notify_one();
}

from thread-pool.

https://stackoverflow.com/questions/35775501/c-should-condition-variable-be-notified-under-lock

from thread-pool.

https://stackoverflow.com/questions/17101922/do-i-have-to-acquire-lock-before-calling-condition-variable-notify-one/17102100

from thread-pool.

@yvoinov In the case of submit(), if we don't lock the mutex then I believe that notify_one() could be called between checking whether the queue is empty and wait(). In that case, we will wait forever, or until something else notifies the condition variable. I placed the lock to ensure that we only notify if the other thread is currently waiting. If you don't lock then how else do you prevent this race condition?

https://stackoverflow.com/questions/20982270/sync-is-unreliable-using-stdatomic-and-stdcondition-variable

from thread-pool.

@yvoinov In the case of submit(), if we don't lock the mutex then I believe that notify_one() could be called between checking whether the queue is empty and wait(). In that case, we will wait forever, or until something else notifies the condition variable. I placed the lock to ensure that we only notify if the other thread is currently waiting. If you don't lock then how else do you prevent this race condition?

https://stackoverflow.com/questions/20982270/sync-is-unreliable-using-stdatomic-and-stdcondition-variable

After a lot of testing, I'm using quite different thread pool implementation:

https://github.com/yvoinov/thread-pool-cpp

There is no race condition on submit. Without lock notify_*. This implementation works perfectly, no deadlocks, no races.

To prevent lost wakeup it is enough to lock conditional variable mutex during notify_one(). It described here:

http://www.modernescpp.com/index.php/c-core-guidelines-be-aware-of-the-traps-of-condition-variables

from thread-pool.