Copilot commented on code in PR #3380:
URL: https://github.com/apache/brpc/pull/3380#discussion_r3566273024
##########
src/bthread/mutex.cpp:
##########
@@ -576,7 +579,8 @@ struct TLSPthreadContentionSites {
uint64_t cp_version;
MutexAndContentionSite list[TLS_MAX_COUNT];
};
-static __thread TLSPthreadContentionSites tls_csites = {0,0,{}};
+BAIDU_VOLATILE_THREAD_LOCAL(TLSPthreadContentionSites, tls_csites,
+ TLSPthreadContentionSites());
Review Comment:
Initializing a __thread variable via `TLSPthreadContentionSites()` risks a
non-constant TLS initializer on some toolchains; this code previously used an
aggregate zero-initializer. Using `{0}` keeps constant/zero initialization and
avoids potential build issues while preserving the intended zeroed state.
##########
src/bthread/bthread.cpp:
##########
@@ -283,13 +282,14 @@ start_from_non_worker(bthread_t* __restrict tid,
// 1. NOSIGNAL is often for creating many bthreads in batch,
// inserting into the same TaskGroup maximizes the batch.
// 2. bthread_flush() needs to know which TaskGroup to flush.
- TaskGroup* g = tls_task_group_nosignal;
- if (NULL == g) {
+ auto g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal);
+ if (NULL == g || g->tag() != tag) {
g = c->choose_one_group(tag);
- tls_task_group_nosignal = g;
+ BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal, g);
}
return g->start_background<true>(tid, attr, fn, arg);
}
+ BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_task_group_nosignal, NULL);
return c->choose_one_group(tag)->start_background<true>(tid, attr, fn,
arg);
Review Comment:
Clearing tls_task_group_nosignal in the non-NOSIGNAL path can break
bthread_flush() for a non-worker thread that previously created NOSIGNAL
bthreads: the cached TaskGroup pointer is the only way bthread_flush() knows
which group to signal, so resetting it here can leave queued NOSIGNAL tasks
unsignaled.
--
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]
