bobhan1 opened a new issue, #3520: URL: https://github.com/apache/brpc/issues/3520
**Describe the bug** We observed a `SIGSEGV` in `bvar::detail::AgentGroup<...>::get_tls_agent()` while updating a `bvar::Adder` from a bthread. The captured core and generated code indicate that the bthread suspended in a synchronous RPC, migrated from one pthread to another, and then continued to use the `_s_tls_blocks` TLS address resolved for the previous pthread. The crash was observed in a downstream Apache Doris BE process built with Clang 16 on Linux x86_64 and using brpc 1.4.0. This report is based on source, core, register, and disassembly analysis. We have not attempted a standalone reproduction. ### Relevant implementation `AgentGroup` declares its per-pthread block vector as a raw TLS static member: ```cpp static __thread std::vector<ThreadBlock*>* _s_tls_blocks; ``` The inline methods `get_tls_agent()` and `get_or_create_tls_agent()` read and write `_s_tls_blocks` directly. The same direct-access pattern is still present in [`agent_group.h` in brpc 1.17.0](https://github.com/apache/brpc/blob/1.17.0/src/bvar/detail/agent_group.h). Clang kept the address of this TLS slot alive across a call that suspended the current bthread. After the bthread resumed on another pthread, the inlined bvar path still used the address belonging to the original pthread. ### Core evidence The relevant call path was: ```text application code after a synchronous RPC -> bvar::Reducer::operator<< -> bvar::detail::AgentCombiner::get_or_create_tls_agent -> bvar::detail::AgentGroup<...>::get_tls_agent(id=124) -> SIGSEGV ``` The faulting instruction in the inlined `get_tls_agent()` path was: ```asm mov (%rcx,%rax,8), %rbx ``` At the crash: ```text id = 124 block_id = 0 rcx = 0x0 current pthread fs_base = 0x7fa6bd997700 (pthread B, LWP 2432) cached TLS slot address = 0x7fa90bd1eaf8 other pthread fs_base = 0x7fa90bd25700 (pthread A, LWP 1244) ``` The cached slot address is exactly `pthread A fs_base - 0x6c08`, where `-0x6c08` is the `_s_tls_blocks` TLS offset in this binary. It does not correspond to the currently executing pthread B. This shows that execution resumed on pthread B while retaining the `_s_tls_blocks` address from pthread A. The size check immediately before the indexed load had passed, but the vector data pointer used by the indexed load was null. This is consistent with pthread B reading pthread A's stale TLS vector while pthread A concurrently initialized or resized that vector. Even without that concurrent resize, accessing another pthread's bvar agents is incorrect. The resulting sequence is: ```text bthread runs on pthread A -> Clang resolves/caches the address of AgentGroup::_s_tls_blocks -> synchronous RPC suspends the bthread -> bthread resumes on pthread B -> inlined bvar code reuses pthread A's TLS address -> pthread A concurrently mutates its own AgentGroup vector -> inconsistent vector state is observed and get_tls_agent() crashes ``` This is not specific to the application RPC or metric. Any inlined bvar access whose TLS address is kept across a bthread suspend/migration point may be affected. **To Reproduce** We have not run a standalone reproducer. The observed trigger had the following shape: ```cpp void run_in_bthread() { // Work in this function causes Clang to resolve/retain the // AgentGroup<T>::_s_tls_blocks TLS address. synchronous_rpc_that_suspends_the_bthread(); // The bthread may now be running on a different pthread. adder << 1; // inlined AgentGroup access } ``` A deterministic regression test could use explicit handshakes to: 1. Start the bthread on pthread A and ensure the relevant TLS address has been used. 2. Suspend it at a controlled point and resume it on pthread B. 3. Concurrently initialize enough agents on pthread A to initialize or resize that `AgentGroup` specialization's TLS vector. 4. Verify that the post-migration bvar operation resolves `_s_tls_blocks` from pthread B and neither accesses pthread A's vector nor crashes. The test should use synchronization/handshakes rather than timing-based sleeps. **Expected behavior** Every `AgentGroup` operation executed after a bthread resumes should resolve `_s_tls_blocks` for the currently executing pthread. A bthread migration must not cause bvar to access another pthread's agent vector or crash. **Versions** ```text OS: Linux x86_64 (exact distribution unavailable) Compiler: Clang 16, optimized build brpc: 1.4.0 plus the downstream Apache Doris patch set protobuf: N/A to this failure ``` The affected `AgentGroup` implementation matches upstream brpc 1.4.0; the downstream patch set does not modify this code. The crash has not been reproduced against brpc 1.17.0. However, static inspection shows that 1.17.0 still directly accesses raw `_s_tls_blocks`, so the specific `AgentGroup` gap remains in the latest release source. **Additional context/screenshots** ### Existing related issues and fixes - [`#1776`](https://github.com/apache/brpc/issues/1776) describes the same class of failure: a compiler-cached `tls_bls` address remains associated with pthread A after a bthread migrates to pthread B. - [`#1860`](https://github.com/apache/brpc/issues/1860) discusses Clang/LTO retaining a TLS `errno` address across bthread context switches. - [`#845`](https://github.com/apache/brpc/issues/845) and [`#1407`](https://github.com/apache/brpc/issues/1407) are earlier `TaskGroup` failures related to compiler-optimized TLS access. - [`PR #2156`](https://github.com/apache/brpc/pull/2156) introduced noinline/asm-based volatile TLS accessors for `tls_task_group`. - [`PR #2248`](https://github.com/apache/brpc/pull/2248) enabled that protection for Clang on x86_64 as well. - [`PR #2934`](https://github.com/apache/brpc/pull/2934) applied the same mechanism to `tls_bls`. - [`LLVM issue #98479`](https://github.com/llvm/llvm-project/issues/98479) documents TLS addresses being kept alive across stackful-fiber suspension and migration to another OS thread. These fixes establish the required access pattern, but none of them changes `bvar::detail::AgentGroup::_s_tls_blocks`. An exact search for `_s_tls_blocks` in brpc issues only found unrelated memory-layout and compilation reports; no existing issue appears to cover this `AgentGroup` migration case. ### Possible fix direction brpc already provides `STATIC_MEMBER_BAIDU_VOLATILE_THREAD_LOCAL`, `BAIDU_GET_VOLATILE_THREAD_LOCAL`, and `BAIDU_SET_VOLATILE_THREAD_LOCAL` in [`butil/thread_local.h`](https://github.com/apache/brpc/blob/1.17.0/src/butil/thread_local.h), and uses this pattern for static-member TLS in [`ObjectPool`](https://github.com/apache/brpc/blob/1.17.0/src/butil/object_pool_inl.h). A possible fix is to apply the same mechanism to `AgentGroup::_s_tls_blocks` and route all reads and writes in `get_tls_agent()`, `get_or_create_tls_agent()`, and `_destroy_tls_blocks()` through the accessor functions. Each operation should first obtain the current pthread's vector through the noinline accessor and then consistently use that local pointer. An ordinary null check, value-level `volatile`, atomic operation, or memory fence is not sufficient because the problematic optimization retains the TLS address itself. ### Evidence limitations - The crash and TLS-address relationship come from the captured incident's core analysis; the original core is not publicly available. - No standalone reproduction has been attempted. - brpc 1.17.0 has only been inspected statically for this report; it has not been runtime-tested against this trigger. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
