This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new e2a2194a50 [bugfix](fragment mgr) heap used after free in fragment
manager when query is cancelled (#23817)
e2a2194a50 is described below
commit e2a2194a500fae1b4b3c431c47f37d2cf23f9c8a
Author: yiguolei <[email protected]>
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 <[email protected]>
---
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 5036878d75..409a32a128 100644
--- a/be/src/runtime/fragment_mgr.cpp
+++ b/be/src/runtime/fragment_mgr.cpp
@@ -798,7 +798,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);
+ }
exec_state.reset(
new FragmentExecState(query_ctx->query_id,
params.params.fragment_instance_id,
diff --git a/be/src/runtime/fragment_mgr.h b/be/src/runtime/fragment_mgr.h
index 8ca58ccffa..dca60b91e7 100644
--- a/be/src/runtime/fragment_mgr.h
+++ b/be/src/runtime/fragment_mgr.h
@@ -159,6 +159,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: [email protected]
For additional commands, e-mail: [email protected]
