chenBright commented on code in PR #3689:
URL: https://github.com/apache/brpc/pull/3689#discussion_r4510131091
##########
test/bthread_id_unittest.cpp:
##########
@@ -112,6 +116,115 @@ TEST(BthreadIdTest, join_before_destroy) {
ASSERT_EQ(1UL, non_null_ret);
}
+struct JoinRaceArgs {
+ bthread_id_t id;
+ std::atomic<bool> join_butex_read;
+ std::atomic<bool> allow_wait;
+};
+
+class JoinBeforeWaitHookGuard {
+public:
+ JoinBeforeWaitHookGuard(void (*hook)(void*), void* arg,
+ std::atomic<bool>* release_waiter)
+ : _release_waiter(release_waiter) {
+ bthread::g_bthread_id_join_before_wait_arg_for_test = arg;
+ bthread::g_bthread_id_join_before_wait_for_test = hook;
+ }
+
+ ~JoinBeforeWaitHookGuard() {
+ if (_release_waiter != nullptr) {
+ _release_waiter->store(true, std::memory_order_release);
+ }
+ bthread::g_bthread_id_join_before_wait_for_test = nullptr;
+ bthread::g_bthread_id_join_before_wait_arg_for_test = nullptr;
Review Comment:
Setting the release flag here does not guarantee that the waiter has exited.
If `ASSERT_EQ` returns early after the thread is created, the stack-allocated
`args` will go out of scope once the destructor finishes; the waiter may still
be in the hook loop and may resume reading `allow_wait` or `start`. Even if it
reads true, it may continue waiting on an ID that has not yet been destroyed.
Therefore, the previous failure cleanup issue is still not fully resolved.
Please have the cleanup process manage both the ID and the waiter, ensuring
that the wait is released and `pthread_join` completes before clearing the hook
and releasing the arguments.
--
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]
