(impala) branch master updated: IMPALA-14994: Fix data race in destructor of ScopedGetNextEventAdder

[st...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new 9c81c3736 IMPALA-14994: Fix data race in destructor of ScopedGetNextEventAdder
9c81c3736 is described below

commit 9c81c37364a60aaaf7e806545962f7c110ec06b7
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Fri May 22 14:07:44 2026 +0800

    IMPALA-14994: Fix data race in destructor of ScopedGetNextEventAdder
    
    The last_batch_returned_ boolean flag of ExecNode is written in the
    fragment instance thread and read by the query state thread in
    constructing the status reports. This patch changes its type to
    AtomicBool to fix the data race issue.
    
    Note that it's OK if the query state thread sees last_batch_returned_ =
    false while the fragment instance thread is finishing. The query state
    thread will send a final status report which reads the final
    last_batch_returned_ value.
    
    Testing
     - Ran TSAN build
    
    Change-Id: Ia3bf17f56974e471994ca889190c7065574a3414
    Reviewed-on: http://gerrit.cloudera.org:8080/24345
    Reviewed-by: Peter Rozsa <pr...@cloudera.com>
    Reviewed-by: Zoltan Borok-Nagy <bo...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/common/atomic.h       | 2 +-
 be/src/exec/exec-node-util.h | 2 +-
 be/src/exec/exec-node.h      | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/be/src/common/atomic.h b/be/src/common/atomic.h
index 08711483c..60cf662c2 100644
--- a/be/src/common/atomic.h
+++ b/be/src/common/atomic.h
@@ -57,7 +57,7 @@ namespace internal {
 /// is performed atomically and has a specified memory-ordering semantic:
 ///
 /// Acquire: these operations ensure no later memory access by the same thread can be
-/// reordered ahead of the operation. (C++11: memory_order_relaxed)
+/// reordered ahead of the operation. (C++11: memory_order_acquire)
 ///
 /// Release: these operations ensure that no previous memory access by the same thread
 /// can be reordered after the operation (C++11: memory_order_release).
diff --git a/be/src/exec/exec-node-util.h b/be/src/exec/exec-node-util.h
index 0087a2ff9..922959988 100644
--- a/be/src/exec/exec-node-util.h
+++ b/be/src/exec/exec-node-util.h
@@ -67,7 +67,7 @@ class ScopedGetNextEventAdder {
     // e.g. if an ancestor node hits a limit.
     if (*eos_) {
       node_->events_->MarkEvent("Last Batch Returned");
-      node_->last_batch_returned_ = true;
+      node_->last_batch_returned_.Store(true);
     }
   }
 
diff --git a/be/src/exec/exec-node.h b/be/src/exec/exec-node.h
index d40ad8293..59b1b793f 100644
--- a/be/src/exec/exec-node.h
+++ b/be/src/exec/exec-node.h
@@ -354,7 +354,7 @@ class ExecNode {
   MemPool* expr_results_pool() { return expr_results_pool_.get(); }
   const TBackendResourceProfile& resource_profile() { return resource_profile_; }
   bool is_closed() const { return is_closed_; }
-  bool last_batch_returned() const { return last_batch_returned_; }
+  bool last_batch_returned() const { return last_batch_returned_.Load(); }
 
   /// Returns true if this node is inside the right-hand side plan tree of a SubplanNode.
   bool IsInSubplan() const { return plan_node_.IsInSubplan(); }
@@ -419,7 +419,7 @@ class ExecNode {
   bool first_getnext_added_ = false;
 
   /// Used to track whether the "Last Batch Returned" event was added to 'events_'.
-  bool last_batch_returned_ = false;
+  AtomicBool last_batch_returned_;
 
   /// Conjuncts and their evaluators in this node. 'conjuncts_' live in the
   /// query-state's object pool while the evaluators live in this exec node's