Copilot commented on code in PR #3522:
URL: https://github.com/apache/brpc/pull/3522#discussion_r3939820466
##########
src/bvar/detail/agent_group.h:
##########
@@ -113,10 +113,12 @@ class AgentGroup {
// We need this function to be as fast as possible.
inline static Agent* get_tls_agent(AgentId id) {
if (__builtin_expect(id >= 0, 1)) {
- if (_s_tls_blocks) {
+ std::vector<ThreadBlock*>* tls_blocks =
+ BAIDU_GET_VOLATILE_THREAD_LOCAL(_s_tls_blocks);
+ if (tls_blocks) {
const size_t block_id = (size_t)id / ELEMENTS_PER_BLOCK;
- if (block_id < _s_tls_blocks->size()) {
- ThreadBlock* const tb = (*_s_tls_blocks)[block_id];
+ if (block_id < tls_blocks->size()) {
Review Comment:
This change is meant to prevent stale TLS access after a bthread suspends
and resumes on another pthread under Clang, but the existing AgentGroup unit
test only exercises pthread usage (no bthread migration / suspend point). A
targeted regression test that forces a bthread to suspend and resume on a
different pthread (with synchronization rather than sleeps) would help ensure
this fix stays effective, especially on Clang where
BAIDU_GET_VOLATILE_THREAD_LOCAL routes through the noinline accessor.
--
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]