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

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

commit 70391d7f0dac459b15b0f168a476ec1aa92c2586
Author: Joe McDonnell <[email protected]>
AuthorDate: Sat Mar 22 13:04:22 2025 -0700

    IMPALA-13891: Unregister MemTrackers from parents in TupleFileReader/Writer
    
    Running failure/test_failpoints.py with tuple caching causes a crash.
    The underlying cause is that the MemTrackers for TupleFileReader and
    TupleFileWriter get deleted without being unregistered from their
    parents, causing a SIGSEGV. Ordinarily, MemTrackers live from Prepare()
    to the final teardown of the query. This means that they are always
    alive past the point when something would be reading from them.
    TupleFileReader/TupleFileWriter can be deleted before the query is
    over, so this changes the code to also deregister the MemTracker
    from its parent.
    
    Testing:
     - Verified that failure/test_failpoints.py stops crashing when running
       with tuple caching
    
    Change-Id: I14e5484de35f78da74c1246d319553496793de81
    Reviewed-on: http://gerrit.cloudera.org:8080/22665
    Reviewed-by: Impala Public Jenkins <[email protected]>
    Tested-by: Impala Public Jenkins <[email protected]>
---
 be/src/exec/tuple-file-reader.cc | 6 ++++--
 be/src/exec/tuple-file-writer.cc | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/be/src/exec/tuple-file-reader.cc b/be/src/exec/tuple-file-reader.cc
index dfe638980..9659faf62 100644
--- a/be/src/exec/tuple-file-reader.cc
+++ b/be/src/exec/tuple-file-reader.cc
@@ -42,8 +42,10 @@ TupleFileReader::TupleFileReader(
         ADD_COUNTER(profile, "TupleCacheBytesRead", TUnit::BYTES) : nullptr) {}
 
 TupleFileReader::~TupleFileReader() {
-  // MemTracker expects an explicit close.
-  tracker_->Close();
+  // MemTracker expects an explicit close. Since the TupleFileReader can be 
freed
+  // before query execution ends, this must unregister the MemTracker from its 
parent
+  // before it is freed.
+  tracker_->CloseAndUnregisterFromParent();
 }
 
 Status TupleFileReader::Open(RuntimeState *state) {
diff --git a/be/src/exec/tuple-file-writer.cc b/be/src/exec/tuple-file-writer.cc
index 1eef9fbb6..46af2bbf8 100644
--- a/be/src/exec/tuple-file-writer.cc
+++ b/be/src/exec/tuple-file-writer.cc
@@ -64,9 +64,11 @@ TupleFileWriter::~TupleFileWriter() {
     DCHECK(!kudu::Env::Default()->FileExists(TempPath()));
   }
   // MemTracker expects an explicit close. Consumers need to be released first.
+  // Since the TupleFileWriter can be freed before query execution ends, this 
must
+  // unregister the MemTracker from its parent before it is freed.
   out_batch_.reset();
   allocator_.reset();
-  if (tracker_) tracker_->Close();
+  if (tracker_) tracker_->CloseAndUnregisterFromParent();
 }
 
 Status TupleFileWriter::Open(RuntimeState* state) {

Reply via email to