Yida Wu has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/19087 )

Change subject: IMPALA-11631 Fix impala crashes in 
impala::TopNNode::Heap::Close()
......................................................................

IMPALA-11631 Fix impala crashes in impala::TopNNode::Heap::Close()

The bug is introduced by IMPALA-11631, if RematerializeTuples()
fails in ReclaimTuplePool(), it returns immediately with an error,
however, some Heap unique_ptr in the partition_heaps_ could be
already moved to the rematerialized_heaps, and set to nullptr,
while the Close() of the TopNNode will try to call Close() on all
the Heap unique_ptr in the partition_heaps_, which leads to a
crash.

There could be two issues, the calling on a nullptr as described
above and a potential memory leaking. Because Heap doesn't call
Close() in the destructor, the unique_ptr may not release all
the resources properly if we don't call the Close() explicitly
for the Heap.

The patch changes the logic of moving each Heap object after
one rematerialize process succeeds, instead, we will move all the
Heap objects in the partition_heaps_ only when all the rematerialize
processes succeed. Therefore, there will be no half released
partition_heaps_, all the Heap should be called Close() during the
TopNNode closing. Also, added checking for nullptr Heaps in the
Close() process of TopNNode.

Because it could be difficult for a testcase to inject an error
for this case to create a crash. I did some hacking in the
code to inject a memory allocation failure in certain cases,
reproduced the issue, and proved the patch can solve the issue
manually.

Tests:
Ran core tests.
Manual test.

Change-Id: Iaf45b6ef777f68e1843c076a935e4189acc6990b
---
M be/src/exec/topn-node.cc
1 file changed, 9 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/19087/6
--
To view, visit http://gerrit.cloudera.org:8080/19087
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaf45b6ef777f68e1843c076a935e4189acc6990b
Gerrit-Change-Number: 19087
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <wydbaggio...@gmail.com>
Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wydbaggio...@gmail.com>

Reply via email to