IMPALA-8299: Fix crash in GroupingAggregator on uninited hash table
If HashTable::Init() fails, in some cases we may try to call
HashTable::Close() on the table in GroupingAggregator::Close(), which
can cause a crash.
The solution is to set the hash table to a nullptr if Init() fails.
I also verified that the other use of HashTable, PhjBuilder, handles
this case correctly.
Testing:
- Manually verified that the crash no longer occurs if HashTable::Init
fails.
Change-Id: I381657c43f93eb4871b93d54532cb886198fd0b2
Reviewed-on: http://gerrit.cloudera.org:8080/12746
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
diff --git a/be/src/exec/grouping-aggregator-partition.cc b/be/src/exec/grouping-aggregator-partition.cc
index 8fe08f4..8be932d 100644
--- a/be/src/exec/grouping-aggregator-partition.cc
+++ b/be/src/exec/grouping-aggregator-partition.cc
@@ -89,7 +89,12 @@
1L << (32 - NUM_PARTITIONING_BITS), PAGG_DEFAULT_HASH_TABLE_SZ));
// Please update the error message in CreateHashPartitions() if initial size of
// hash table changes.
- return hash_tbl->Init(got_memory);
+ Status status = hash_tbl->Init(got_memory);
+ if (!status.ok() || !(*got_memory)) {
+ hash_tbl->Close();
+ hash_tbl.reset();
+ }
+ return status;
}
Status GroupingAggregator::Partition::SerializeStreamForSpilling() {
diff --git a/be/src/exec/grouping-aggregator.h b/be/src/exec/grouping-aggregator.h
index 405d4ab..258ed6e 100644
--- a/be/src/exec/grouping-aggregator.h
+++ b/be/src/exec/grouping-aggregator.h
@@ -346,6 +346,8 @@
/// Initializes the hash table. 'aggregated_row_stream' must be non-NULL.
/// Sets 'got_memory' to true if the hash table was initialised or false on OOM.
+ /// After returning, 'hash_tbl' will be non-null iff 'got_memory' is true and the
+ /// returned status is OK.
Status InitHashTable(bool* got_memory) WARN_UNUSED_RESULT;
/// Called in case we need to serialize aggregated rows. This step effectively does
