From 8aa7668ba6d9049644b02fcb6fbdba00fd8fdf67 Mon Sep 17 00:00:00 2001
From: Alberto Sartori <alberto.sartori@huawei.com>
Date: Wed, 25 Oct 2023 17:38:28 +0200
Subject: just execute: fix race condition during garbage collection...

...of the internal cache used for keeping track of running operations.

(cherry-picked from bec4dbd9bd995f73eab1ff803e7895187068ddb0)
---
 CHANGELOG.md                                                      | 5 +++++
 src/buildtool/execution_api/execution_service/operation_cache.cpp | 3 ---
 src/buildtool/execution_api/execution_service/operation_cache.hpp | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7ab2e63f..c6c9f854 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 Bug fixes on top of release `1.2.2`.
 
+### Fixes
+
+- Fixed a race condition in an internal cache of `just execute` used for keeping
+  track of running operations.
+
 ## Release `1.2.2` (2023-10-17)
 
 Bug fixes on top of release `1.2.1`.
diff --git a/src/buildtool/execution_api/execution_service/operation_cache.cpp b/src/buildtool/execution_api/execution_service/operation_cache.cpp
index 7fccdd41..1502016e 100644
--- a/src/buildtool/execution_api/execution_service/operation_cache.cpp
+++ b/src/buildtool/execution_api/execution_service/operation_cache.cpp
@@ -19,13 +19,11 @@
 #include "google/protobuf/timestamp.pb.h"
 
 void OperationCache::GarbageCollection() {
-    std::shared_lock slock{mutex_};
     if (cache_.size() > (threshold_ << 1U)) {
         std::vector<std::pair<std::string, ::google::longrunning::Operation>>
             tmp;
         tmp.reserve(cache_.size());
         std::copy(cache_.begin(), cache_.end(), std::back_insert_iterator(tmp));
-        slock.release();
         std::sort(tmp.begin(), tmp.end(), [](auto const& x, auto const& y) {
             ::google::protobuf::Timestamp tx;
             ::google::protobuf::Timestamp ty;
@@ -35,7 +33,6 @@ void OperationCache::GarbageCollection() {
         });
 
         std::size_t deleted = 0;
-        std::unique_lock ulock{mutex_};
         for (auto const& [key, op] : tmp) {
             if (op.done()) {
                 DropInternal(key);
diff --git a/src/buildtool/execution_api/execution_service/operation_cache.hpp b/src/buildtool/execution_api/execution_service/operation_cache.hpp
index 25ca49a7..2467add7 100644
--- a/src/buildtool/execution_api/execution_service/operation_cache.hpp
+++ b/src/buildtool/execution_api/execution_service/operation_cache.hpp
@@ -63,8 +63,8 @@ class OperationCache {
     std::size_t threshold_{1U << kDefaultExponent};
 
     void SetInternal(std::string const& action, Operation const& op) {
-        GarbageCollection();
         std::unique_lock lock{mutex_};
+        GarbageCollection();
         cache_[action] = op;
     }
 
-- 
cgit v1.2.3