This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new ef2130de57 [improvement](memory) fix possible memory leak of vcollect
iterator (#16822)
ef2130de57 is described below
commit ef2130de5763a3f22edcbba7b0cb365b26fb166c
Author: TengJianPing <18241664+jackte...@users.noreply.github.com>
AuthorDate: Fri Feb 17 14:40:15 2023 +0800
[improvement](memory) fix possible memory leak of vcollect iterator (#16822)
Logic in function VCollectIterator::build_heap is not robust, which may
cause memory leak:
Level1Iterator* cumu_iter = new Level1Iterator(
cumu_children, _reader, cumu_children.size() > 1,
_is_reverse, _skip_same);
RETURN_IF_NOT_EOF_AND_OK(cumu_iter->init());
std::list<LevelIterator*> children;
children.push_back(*base_reader_child);
children.push_back(cumu_iter);
_inner_iter.reset(
new Level1Iterator(children, _reader, _merge,
_is_reverse, _skip_same));
cumu_iter will be leaked if cumu_iter->init()); is not success.
---
be/src/vec/olap/vcollect_iterator.cpp | 26 +++++++++++++++-----------
be/src/vec/olap/vcollect_iterator.h | 2 +-
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/be/src/vec/olap/vcollect_iterator.cpp
b/be/src/vec/olap/vcollect_iterator.cpp
index b7313210e4..7231ad1453 100644
--- a/be/src/vec/olap/vcollect_iterator.cpp
+++ b/be/src/vec/olap/vcollect_iterator.cpp
@@ -136,18 +136,21 @@ Status
VCollectIterator::build_heap(std::vector<RowsetReaderSharedPtr>& rs_reade
}
++i;
}
- Level1Iterator* cumu_iter = new Level1Iterator(
- cumu_children, _reader, cumu_children.size() > 1,
_is_reverse, _skip_same);
+ bool is_merge = cumu_children.size() > 1;
+ auto cumu_iter =
std::make_unique<Level1Iterator>(std::move(cumu_children), _reader,
+ is_merge,
_is_reverse, _skip_same);
RETURN_IF_NOT_EOF_AND_OK(cumu_iter->init());
std::list<LevelIterator*> children;
children.push_back(*base_reader_child);
- children.push_back(cumu_iter);
- _inner_iter.reset(
- new Level1Iterator(children, _reader, _merge, _is_reverse,
_skip_same));
+ children.push_back(cumu_iter.get());
+ auto level1_iter = new Level1Iterator(std::move(children),
_reader, _merge, _is_reverse,
+ _skip_same);
+ cumu_iter.release();
+ _inner_iter.reset(level1_iter);
} else {
// _children.size() == 1
- _inner_iter.reset(
- new Level1Iterator(_children, _reader, _merge,
_is_reverse, _skip_same));
+ _inner_iter.reset(new Level1Iterator(std::move(_children),
_reader, _merge, _is_reverse,
+ _skip_same));
}
} else {
bool have_multiple_child = false;
@@ -166,7 +169,8 @@ Status
VCollectIterator::build_heap(std::vector<RowsetReaderSharedPtr>& rs_reade
++iter;
}
}
- _inner_iter.reset(new Level1Iterator(_children, _reader, _merge,
_is_reverse, _skip_same));
+ _inner_iter.reset(
+ new Level1Iterator(std::move(_children), _reader, _merge,
_is_reverse, _skip_same));
}
RETURN_IF_NOT_EOF_AND_OK(_inner_iter->init());
// Clear _children earlier to release any related references
@@ -524,10 +528,10 @@ Status
VCollectIterator::Level0Iterator::current_block_row_locations(
}
VCollectIterator::Level1Iterator::Level1Iterator(
- const std::list<VCollectIterator::LevelIterator*>& children,
TabletReader* reader,
- bool merge, bool is_reverse, bool skip_same)
+ std::list<VCollectIterator::LevelIterator*>&& children, TabletReader*
reader, bool merge,
+ bool is_reverse, bool skip_same)
: LevelIterator(reader),
- _children(children),
+ _children(std::move(children)),
_reader(reader),
_merge(merge),
_is_reverse(is_reverse),
diff --git a/be/src/vec/olap/vcollect_iterator.h
b/be/src/vec/olap/vcollect_iterator.h
index 3018768d4a..c859fca7a5 100644
--- a/be/src/vec/olap/vcollect_iterator.h
+++ b/be/src/vec/olap/vcollect_iterator.h
@@ -244,7 +244,7 @@ private:
// Iterate from LevelIterators (maybe Level0Iterators or Level1Iterator or
mixed)
class Level1Iterator : public LevelIterator {
public:
- Level1Iterator(const std::list<LevelIterator*>& children,
TabletReader* reader, bool merge,
+ Level1Iterator(std::list<LevelIterator*>&& children, TabletReader*
reader, bool merge,
bool is_reverse, bool skip_same);
Status init(bool get_data_by_ref = false) override;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
