database/doris
2
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

[improvement](memory) fix possible memory leak of vcollect iterator (#16822)

Browse Source 
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.
This commit is contained in:
TengJianPing
2023-02-17 14:40:15 +08:00
committed by GitHub
parent 30dafd6a44
commit ef2130de57
2 changed files with 16 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
be/src/vec/olap/vcollect_iterator.cpp
View File

@ -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),

2
be/src/vec/olap/vcollect_iterator.h
View File

@ -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;