Copilot commented on code in PR #3545:
URL: https://github.com/apache/brpc/pull/3545#discussion_r4089303564
##########
src/bthread/butex.cpp:
##########
@@ -731,7 +731,13 @@ static int butex_wait_from_pthread(TaskGroup* g, Butex* b,
int expected_value,
30/*nops before sched_yield*/);
if (task->interrupted) {
task->interrupted = false;
- if (rc == 0) {
+ // If interrupted after enqueueing but before futex_wait_private,
+ // pw.sig is already signalled and futex_wait_private may report
+ // EWOULDBLOCK. This is an interruption, not a value mismatch on
+ // the user's butex. Preserve other errors (notably ETIMEDOUT).
+ if (rc == 0 || (errno == EWOULDBLOCK &&
+ pw.sig.load(butil::memory_order_acquire) ==
+ PTHREAD_SIGNALLED)) {
Review Comment:
The new classification still reads and clears `task->interrupted` outside
`task->version_lock`, while `TaskGroup::interrupt()` updates that field under
the same lock. An interruption racing this epilogue is therefore a data race,
and the clear can also consume an interruption being published concurrently.
Hold `task->version_lock` across the check/clear and the `pw.sig`
classification, as is already done during waiter publication above.
##########
test/bthread_butex_unittest.cpp:
##########
@@ -149,27 +163,27 @@ void* waiter(void* arg) {
TEST(ButexTest, sanity) {
const size_t N = 5;
WaiterArg args[N * 4];
- pthread_t t1, t2;
- butil::atomic<int>* b1 =
- bthread::butex_create_checked<butil::atomic<int> >();
+ pthread_t pthreads[2 * N];
+ bthread_t bthreads[2 * N];
+ butil::atomic<int>* b1 = bthread::butex_create_checked<butil::atomic<int>
>();
ASSERT_TRUE(b1);
bthread::butex_destroy(b1);
Review Comment:
`b1` is returned to the butex object pool here, but the test immediately
dereferences it and passes it to 20 waiters below. That is a use-after-destroy
and can make the new waiter setup corrupt or hang; keep the butex alive until
all joins complete, then destroy it at the end of the test.
--
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]
