[doris] branch master updated: [bugfix](fragment mgr) heap used after free in fragment manager when query is cancelled (#23817)

[yiguolei]

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 0179e5c2ba [bugfix](fragment mgr) heap used after free in fragment 
manager when query is cancelled (#23817)
0179e5c2ba is described below

commit 0179e5c2ba550bb9ed9a5eaa92f5ea6d1cc60cd7
Author: yiguolei <676222...@qq.com>
AuthorDate: Mon Sep 4 12:20:16 2023 +0800

    [bugfix](fragment mgr) heap used after free in fragment manager when query 
is cancelled (#23817)
    
    Co-authored-by: yiguolei <yiguo...@gmail.com>
---
 be/src/runtime/fragment_mgr.cpp | 8 +++++++-
 be/src/runtime/fragment_mgr.h   | 5 +++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/be/src/runtime/fragment_mgr.cpp b/be/src/runtime/fragment_mgr.cpp
index b0ec7bbc3b..b56d92fc83 100644
--- a/be/src/runtime/fragment_mgr.cpp
+++ b/be/src/runtime/fragment_mgr.cpp
@@ -617,7 +617,13 @@ Status FragmentMgr::exec_plan_fragment(const 
TExecPlanFragmentParams& params,
                                    params.query_options.enable_pipeline_engine;
     RETURN_IF_ERROR(
             _get_query_ctx(params, params.params.query_id, 
pipeline_engine_enabled, query_ctx));
-    query_ctx->fragment_ids.push_back(fragment_instance_id);
+    {
+        // Need lock here, because it will modify fragment ids and std::vector 
may resize and reallocate
+        // memory, but query_is_canncelled will traverse the vector, it will 
core.
+        // query_is_cancelled is called in allocator, we has to avoid dead 
lock.
+        std::lock_guard<std::mutex> lock(_lock);
+        query_ctx->fragment_ids.push_back(fragment_instance_id);
+    }
 
     auto fragment_executor = std::make_shared<PlanFragmentExecutor>(
             _exec_env, query_ctx, params.params.fragment_instance_id, -1, 
params.backend_num,
diff --git a/be/src/runtime/fragment_mgr.h b/be/src/runtime/fragment_mgr.h
index 8548d19d78..5691e08e31 100644
--- a/be/src/runtime/fragment_mgr.h
+++ b/be/src/runtime/fragment_mgr.h
@@ -169,6 +169,11 @@ private:
     // This is input params
     ExecEnv* _exec_env;
 
+    // The lock should only be used to protect the structures in fragment 
manager. Has to be
+    // used in a very small scope because it may dead lock. For example, if 
the _lock is used
+    // in prepare stage, the call path is  prepare --> expr prepare --> may 
call allocator
+    // when allocate failed, allocator may call query_is_cancelled, query is 
callced will also
+    // call _lock, so that there is dead lock.
     std::mutex _lock;
 
     std::condition_variable _cv;


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