From 4e7e8d700fbfd20ae08acaae29a940c4f31281f5 Mon Sep 17 00:00:00 2001
From: yiguolei <676222867@qq.com>
Date: Tue, 28 May 2024 12:49:58 +0800
Subject: [PATCH] [enhancement](atomicstatus) use lock to make the status
 object more stable (#35476)

1. In the past, if error code is not ok and then get status, the status
maybe ok. some dcheck maybe failed.

In this PR use std mutex to make this behavior stable.

If this is a relatively large or complex change, kick off the discussion
at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why
you chose the solution you did and what alternatives you considered,
etc...

---------

Co-authored-by: yiguolei <yiguolei@gmail.com>
---
 be/src/common/status.h | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/be/src/common/status.h b/be/src/common/status.h
index 6e5a75f796..c469239241 100644
--- a/be/src/common/status.h
+++ b/be/src/common/status.h
@@ -552,27 +552,25 @@ class AtomicStatus {
 public:
     AtomicStatus() : error_st_(Status::OK()) {}
 
-    bool ok() const { return error_code_.load() == 0; }
+    bool ok() const { return error_code_.load(std::memory_order_acquire) == 0; }
 
     bool update(const Status& new_status) {
         // If new status is normal, or the old status is abnormal, then not need update
-        if (new_status.ok() || error_code_.load() != 0) {
+        if (new_status.ok() || error_code_.load(std::memory_order_acquire) != 0) {
             return false;
         }
-        int16_t expected_error_code = 0;
-        if (error_code_.compare_exchange_strong(expected_error_code, new_status.code(),
-                                                std::memory_order_acq_rel)) {
-            // lock here for read status, to avoid core during return error_st_
-            std::lock_guard l(mutex_);
-            error_st_ = new_status;
-            return true;
-        } else {
+        std::lock_guard l(mutex_);
+        if (error_code_.load(std::memory_order_acquire) != 0) {
             return false;
         }
+        error_st_ = new_status;
+        error_code_.store(new_status.code(), std::memory_order_release);
+        return true;
     }
 
     // will copy a new status object to avoid concurrency
-    Status status() {
+    // This stauts could only be called when ok==false
+    Status status() const {
         std::lock_guard l(mutex_);
         return error_st_;
     }
@@ -580,7 +578,7 @@ public:
 private:
     std::atomic_int16_t error_code_ = 0;
     Status error_st_;
-    std::mutex mutex_;
+    mutable std::mutex mutex_;
 
     AtomicStatus(const AtomicStatus&) = delete;
     void operator=(const AtomicStatus&) = delete;