You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2023/01/31 01:59:09 UTC

[doris] branch master updated: [fix](sort) fix heap-use-after-free error if sort with limit and is spilled (#16267)

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 a7b030778a [fix](sort) fix heap-use-after-free error if sort with limit and is spilled (#16267)
a7b030778a is described below

commit a7b030778a82213962faf8138d93e6be04811b27
Author: TengJianPing <18...@users.noreply.github.com>
AuthorDate: Tue Jan 31 09:59:03 2023 +0800

    [fix](sort) fix heap-use-after-free error if sort with limit and is spilled (#16267)
---
 be/src/vec/common/sort/sorter.cpp      | 19 ++++++++++++++-----
 be/src/vec/common/sort/sorter.h        |  2 ++
 be/src/vec/common/sort/topn_sorter.cpp | 25 +++++++++++++++++++------
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/be/src/vec/common/sort/sorter.cpp b/be/src/vec/common/sort/sorter.cpp
index 225deffa2b..6bfff5da1d 100644
--- a/be/src/vec/common/sort/sorter.cpp
+++ b/be/src/vec/common/sort/sorter.cpp
@@ -339,15 +339,24 @@ Status FullSorter::_do_sort() {
         // if one block totally greater the heap top of _block_priority_queue
         // we can throw the block data directly.
         if (_state->num_rows() < _limit) {
-            _block_priority_queue.emplace(
-                    _pool->add(new MergeSortCursorImpl(desc_block, _sort_description)));
             _state->add_sorted_block(desc_block);
+            // if it's spilled, sorted_block is not added into sorted block vector,
+            // so it's should not be added to _block_priority_queue, since
+            // sorted_block will be destroyed when _do_sort is finished
+            if (!_state->is_spilled()) {
+                _block_priority_queue.emplace(_pool->add(
+                        new MergeSortCursorImpl(_state->last_sorted_block(), _sort_description)));
+            }
         } else {
-            MergeSortBlockCursor block_cursor(
-                    _pool->add(new MergeSortCursorImpl(desc_block, _sort_description)));
+            auto tmp_cursor_impl =
+                    std::make_unique<MergeSortCursorImpl>(desc_block, _sort_description);
+            MergeSortBlockCursor block_cursor(tmp_cursor_impl.get());
             if (!block_cursor.totally_greater(_block_priority_queue.top())) {
                 _state->add_sorted_block(desc_block);
-                _block_priority_queue.push(block_cursor);
+                if (!_state->is_spilled()) {
+                    _block_priority_queue.emplace(_pool->add(new MergeSortCursorImpl(
+                            _state->last_sorted_block(), _sort_description)));
+                }
             }
         }
     } else {
diff --git a/be/src/vec/common/sort/sorter.h b/be/src/vec/common/sort/sorter.h
index 486e336e57..880e70f726 100644
--- a/be/src/vec/common/sort/sorter.h
+++ b/be/src/vec/common/sort/sorter.h
@@ -73,6 +73,8 @@ public:
 
     bool is_spilled() const { return is_spilled_; }
 
+    const Block& last_sorted_block() const { return sorted_blocks_.back(); }
+
     std::unique_ptr<Block> unsorted_block_;
 
 private:
diff --git a/be/src/vec/common/sort/topn_sorter.cpp b/be/src/vec/common/sort/topn_sorter.cpp
index 19d3cc4e8d..7e6427706f 100644
--- a/be/src/vec/common/sort/topn_sorter.cpp
+++ b/be/src/vec/common/sort/topn_sorter.cpp
@@ -53,15 +53,28 @@ Status TopNSorter::_do_sort(Block* block) {
         // if one block totally greater the heap top of _block_priority_queue
         // we can throw the block data directly.
         if (_state->num_rows() < _limit) {
-            _block_priority_queue.emplace(
-                    _pool->add(new MergeSortCursorImpl(sorted_block, _sort_description)));
             _state->add_sorted_block(sorted_block);
+            // if it's spilled, sorted_block is not added into sorted block vector,
+            // so it's should not be added to _block_priority_queue, since
+            // sorted_block will be destroyed when _do_sort is finished
+            if (!_state->is_spilled()) {
+                _block_priority_queue.emplace(_pool->add(
+                        new MergeSortCursorImpl(_state->last_sorted_block(), _sort_description)));
+            }
         } else {
-            MergeSortBlockCursor block_cursor(
-                    _pool->add(new MergeSortCursorImpl(sorted_block, _sort_description)));
-            if (!block_cursor.totally_greater(_block_priority_queue.top())) {
+            if (!_state->is_spilled()) {
+                auto tmp_cursor_impl =
+                        std::make_unique<MergeSortCursorImpl>(sorted_block, _sort_description);
+                MergeSortBlockCursor block_cursor(tmp_cursor_impl.get());
+                if (!block_cursor.totally_greater(_block_priority_queue.top())) {
+                    _state->add_sorted_block(sorted_block);
+                    if (!_state->is_spilled()) {
+                        _block_priority_queue.emplace(_pool->add(new MergeSortCursorImpl(
+                                _state->last_sorted_block(), _sort_description)));
+                    }
+                }
+            } else {
                 _state->add_sorted_block(sorted_block);
-                _block_priority_queue.push(block_cursor);
             }
         }
     } else {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org