[GitHub] [arrow] westonpace commented on a change in pull request #10919: ARROW-13605: [C++] Use mutexes instead of atomics to avoid TSan warning

[GitBox <gi...@apache.org>]

westonpace commented on a change in pull request #10919:
URL: https://github.com/apache/arrow/pull/10919#discussion_r687164377



##########
File path: cpp/src/arrow/compute/exec/util.h
##########
@@ -192,41 +193,57 @@ class AtomicCounter {
  public:
   AtomicCounter() = default;
 
-  int count() const { return count_.load(); }
+  int count() const {
+    auto guard = mutex_.Lock();
+    return count_;
+  }
 
   util::optional<int> total() const {
-    int total = total_.load();
-    if (total == -1) return {};
-    return total;
+    auto guard = mutex_.Lock();
+    if (total_ == -1) return {};
+    return total_;
   }
 
   // return true if the counter is complete
   bool Increment() {
-    DCHECK_NE(count_.load(), total_.load());
-    int count = count_.fetch_add(1) + 1;
-    if (count != total_.load()) return false;
+    auto guard = mutex_.Lock();
+    DCHECK_NE(count_, total_);
+    count_++;
+    if (count_ != total_) return false;
     return DoneOnce();
   }
 
   // return true if the counter is complete
   bool SetTotal(int total) {
-    total_.store(total);
-    if (count_.load() != total) return false;
+    auto guard = mutex_.Lock();
+    total_ = total;
+    if (count_ != total_) return false;
     return DoneOnce();
   }
 
   // return true if the counter has not already been completed
-  bool Cancel() { return DoneOnce(); }
+  bool Cancel() {
+    auto guard = mutex_.Lock();
+    return DoneOnce();
+  }
 
  private:
   // ensure there is only one true return from Increment(), SetTotal(), or Cancel()
   bool DoneOnce() {

Review comment:
       Nit: Rename to `DoneOnceUnlocked`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org