This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 876e2f05f5945bde605d16d2522a6f3d087a2b23
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Wed Sep 16 20:23:37 2026 +0800
branch-4.1: [fix](be) Fix SIGSEGV in bvar::take_sample caused by
~AgentCombiner walking freed TLS agents #67276 (#68045)
Cherry-picked from #67276
Co-authored-by: stinger1206 <[email protected]>
Co-authored-by: stinger1206 <[email protected]>
---
...c-1.4.0-fix-agent-combiner-use-after-free.patch | 86 ++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git
a/thirdparty/patches/brpc-1.4.0-fix-agent-combiner-use-after-free.patch
b/thirdparty/patches/brpc-1.4.0-fix-agent-combiner-use-after-free.patch
new file mode 100644
index 00000000000..c88455d27a1
--- /dev/null
+++ b/thirdparty/patches/brpc-1.4.0-fix-agent-combiner-use-after-free.patch
@@ -0,0 +1,86 @@
+diff --git a/src/bvar/detail/combiner.h b/src/bvar/detail/combiner.h
+index 869e2b5..2e64e31 100644
+--- a/src/bvar/detail/combiner.h
++++ b/src/bvar/detail/combiner.h
+@@ -235,7 +235,37 @@ friend class GlobalValue<self_type>;
+
+ ~AgentCombiner() {
+ if (_id >= 0) {
+- clear_all_agents();
++ // NOTE: We intentionally do NOT walk `_agents` here (e.g. via the
++ // previously existed `clear_all_agents()`).
++ //
++ // `Agent` instances live inside per-thread `ThreadBlock`s owned
by
++ // `AgentGroup` and are destroyed when their owning thread exits
++ // (via `_destroy_tls_blocks`). At that point `~Agent` calls
++ // `combiner.lock()`; if the combiner has already started its
++ // destruction the `weak_ptr` is expired and the agent will skip
++ // `commit_and_erase`, leaving its `LinkNode` linked to this
++ // combiner's `_agents`. If we tried to traverse `_agents` here we
++ // could touch agent nodes whose `ThreadBlock` was just freed by
++ // a concurrent thread-exit, causing heap-use-after-free.
++ //
++ // It is safe to leave the list "dirty" because:
++ // * `butil::LinkedList` / `butil::LinkNode` have trivial
++ // destructors and never traverse on destruction, so tearing
++ // down `_agents` here does not dereference any agent node.
++ // * After this combiner is gone, every still-alive `Agent` will
++ // observe `combiner.expired() == true` in `~Agent` and skip
++ // `commit_and_erase`, so the dangling `prev_/next_` pointers
++ // in those agents are never read.
++ // * If the freed `_id` is later reused by a new combiner and
the
++ // same TLS slot is taken, `get_or_create_tls_agent` will call
++ // `Agent::reset` and `Append` the agent into the new
++ // combiner's `_agents`. `LinkNode::InsertBefore` only writes
++ // `prev_/next_` (never reads their stale values), so the
++ // dangling pointers are safely overwritten.
++ // * `Agent::element` is destroyed together with the
`ThreadBlock`,
++ // so any non-POD resource it holds is still released; if the
++ // agent slot is reused, `Agent::reset` will overwrite the
++ // element value before it is observed again.
+ AgentGroup::destroy_agent(_id);
+ _id = -1;
+ }
+@@ -321,18 +351,30 @@ friend class GlobalValue<self_type>;
+ return agent;
+ }
+
+- void clear_all_agents() {
+- butil::AutoLock guard(_lock);
+- // reseting agents is must because the agent object may be reused.
+- // Set element to be default-constructed so that if it's non-pod,
+- // internal allocations should be released.
+- for (butil::LinkNode<Agent>* node = _agents.head(); node !=
_agents.end();) {
+- node->value()->reset(ElementTp(), NULL);
+- butil::LinkNode<Agent>* const saved_next = node->next();
+- node->RemoveFromList();
+- node = saved_next;
+- }
+- }
++ // NOTE: `clear_all_agents()` is intentionally kept but no longer called
++ // from `~AgentCombiner` (see the long comment in `~AgentCombiner`).
++ //
++ // Calling it from the destructor is unsafe: by the time the destructor
++ // runs, agent weak_ptrs have already expired and `~Agent` will skip
++ // `commit_and_erase`; a concurrent thread-exit can therefore free the
++ // `ThreadBlock` (and the agents inside it) while we are still walking
++ // `_agents` here, which is a heap-use-after-free.
++ //
++ // The body is left around (commented out) for reference / future use --
++ // do NOT re-enable it from `~AgentCombiner`.
++ //
++ // void clear_all_agents() {
++ // butil::AutoLock guard(_lock);
++ // // reseting agents is must because the agent object may be reused.
++ // // Set element to be default-constructed so that if it's non-pod,
++ // // internal allocations should be released.
++ // for (butil::LinkNode<Agent>* node = _agents.head(); node !=
_agents.end();) {
++ // node->value()->reset(ElementTp(), NULL);
++ // butil::LinkNode<Agent>* const saved_next = node->next();
++ // node->RemoveFromList();
++ // node = saved_next;
++ // }
++ // }
+
+ const BinaryOp& op() const { return _op; }
+
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
