From 5ab67696cc314dc9ca463114ecee826bcafbe9e7 Mon Sep 17 00:00:00 2001 From: Klaus Aehlig Date: Fri, 26 Apr 2024 17:47:49 +0200 Subject: Fix race condition in task queue To avoid lock contention, our task system queues tasks in several queues. In order to know when there is no more work to be done, a global counter keeps track of the total work. Here, it is important that this counter be incremented before a value is added to the queue, for otherwise some other thread could pick it up and decrement the total work load, hence getting us in a state where the counter underestimates the total amount of work to be done. Fix the order of those operations. --- CHANGELOG.md | 4 ++++ src/buildtool/multithreading/notification_queue.hpp | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15a54239..1b8bac3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,10 @@ A feature release on top of `1.2.0`, backwards compatible. `git` for fetching and the URL is passed to `git` unchanged. - Improved portability and update of the bundled dependencies. - Various minor improvements and typo fixes in the documentation. +- Fixed a race condition in the task queue that could cause (with + probability roughly 1e-5) a premature termination of the queue + resulting in spurious analysis failures without explanation (despite + "failed to analyse target"). - Fixed a race condition in an internal cache of `just execute` used for keeping track of running operations. - The built-in rule `"install"` now properly enforces that the diff --git a/src/buildtool/multithreading/notification_queue.hpp b/src/buildtool/multithreading/notification_queue.hpp index f84e47c6..a9f476c2 100644 --- a/src/buildtool/multithreading/notification_queue.hpp +++ b/src/buildtool/multithreading/notification_queue.hpp @@ -126,11 +126,11 @@ class NotificationQueue { // finished) template void push(FunctionType&& f) { + total_workload_->Increment(); { std::unique_lock lock{mutex_}; queue_.emplace_back(std::forward(f)); } - total_workload_->Increment(); ready_.notify_one(); } @@ -143,9 +143,9 @@ class NotificationQueue { if (!lock) { return false; } + total_workload_->Increment(); queue_.emplace_back(std::forward(f)); } - total_workload_->Increment(); ready_.notify_one(); return true; } -- cgit v1.2.3