diff --git a/Userland/Libraries/LibThreading/ThreadPool.h b/Userland/Libraries/LibThreading/ThreadPool.h index 7992d113719..709eda0ac98 100644 --- a/Userland/Libraries/LibThreading/ThreadPool.h +++ b/Userland/Libraries/LibThreading/ThreadPool.h @@ -36,6 +36,13 @@ struct ThreadPoolLooper { return IterationDecision::Continue; pool.m_mutex.lock(); + // broadcast on m_work_done here since it is possible the + // wait_for_all loop missed the previous broadcast when work was + // actually done. Without this broadcast the ThreadPool could + // deadlock as there is no remaining work to be done, so this thread + // never resumes and the wait_for_all loop never wakes as there is no + // more work to be completed. + pool.m_work_done.broadcast(); pool.m_work_available.wait(); pool.m_mutex.unlock(); } @@ -76,7 +83,9 @@ public: { m_should_exit.store(true, AK::MemoryOrder::memory_order_release); for (auto& worker : m_workers) { - m_work_available.broadcast(); + while (!worker->has_exited()) { + m_work_available.broadcast(); + } (void)worker->join(); } } @@ -91,18 +100,19 @@ public: void wait_for_all() { - while (true) { - if (m_work_queue.with_locked([](auto& queue) { return queue.is_empty(); })) - break; - m_mutex.lock(); - m_work_done.wait(); - m_mutex.unlock(); + { + MutexLocker lock(m_mutex); + m_work_done.wait_while([this]() { + return m_work_queue.with_locked([](auto& queue) { + return !queue.is_empty(); + }); + }); } - - while (m_busy_count.load(AK::MemoryOrder::memory_order_acquire) > 0) { - m_mutex.lock(); - m_work_done.wait(); - m_mutex.unlock(); + { + MutexLocker lock(m_mutex); + m_work_done.wait_while([this] { + return m_busy_count.load(AK::MemoryOrder::memory_order_acquire) > 0; + }); } }