This is an automated email from the ASF dual-hosted git repository. wwbmmm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push: new e289c76a Fix using butex in return keytable (#2558) e289c76a is described below commit e289c76a25209827cfd71119baf105c965d4148a Author: MJY-HUST <52315061+mjy-h...@users.noreply.github.com> AuthorDate: Mon Mar 11 10:24:55 2024 +0800 Fix using butex in return keytable (#2558) * fix_using_butex_in_return_keytable * add UT and explanatory note --- src/bthread/task_group.cpp | 7 +++--- test/bthread_key_unittest.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/bthread/task_group.cpp b/src/bthread/task_group.cpp index 45bd89ed..a675126d 100644 --- a/src/bthread/task_group.cpp +++ b/src/bthread/task_group.cpp @@ -307,9 +307,6 @@ void TaskGroup::task_runner(intptr_t skip_remained) { thread_return = e.value(); } - // Group is probably changed - g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); - // TODO: Save thread_return (void)thread_return; @@ -332,6 +329,10 @@ void TaskGroup::task_runner(intptr_t skip_remained) { m->local_storage.keytable = NULL; // optional } + // During running the function in TaskMeta and deleting the KeyTable in + // return_KeyTable, the group is probably changed. + g = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_task_group); + // Increase the version and wake up all joiners, if resulting version // is 0, change it to 1 to make bthread_t never be 0. Any access // or join to the bthread after changing version will be rejected. diff --git a/test/bthread_key_unittest.cpp b/test/bthread_key_unittest.cpp index 4b05d33d..7b6bd8d8 100644 --- a/test/bthread_key_unittest.cpp +++ b/test/bthread_key_unittest.cpp @@ -402,4 +402,56 @@ TEST(KeyTest, using_pool) { ASSERT_EQ(0, bthread_key_delete(key)); } +// NOTE: lid is short for 'lock in dtor'. +butil::atomic<size_t> lid_seq(1); +std::vector<size_t> lid_seqs; +bthread_mutex_t mu; + +static void lid_dtor(void* tls) { + bthread_mutex_lock(&mu); + lid_seqs.push_back((size_t)tls); + bthread_mutex_unlock(&mu); +} + +static void lid_worker_impl(bthread_key_t key) { + ASSERT_EQ(NULL, bthread_getspecific(key)); + ASSERT_EQ(0, bthread_setspecific(key, (void*)seq.fetch_add(1))); +} + +static void* lid_worker(void* arg) { + lid_worker_impl(*static_cast<bthread_key_t*>(arg)); + return NULL; +} + +TEST(KeyTest, use_bthread_mutex_in_dtor) { + bthread_key_t key; + + ASSERT_EQ(0, bthread_mutex_init(&mu, nullptr)); + ASSERT_EQ(0, bthread_key_create(&key, lid_dtor)); + + lid_seqs.clear(); + + bthread_t bth[8]; + for (size_t i = 0; i < arraysize(bth); ++i) { + ASSERT_EQ(0, bthread_start_urgent(&bth[i], NULL, lid_worker, &key)); + } + pthread_t th[8]; + for (size_t i = 0; i < arraysize(th); ++i) { + ASSERT_EQ(0, pthread_create(&th[i], NULL, lid_worker, &key)); + } + for (size_t i = 0; i < arraysize(bth); ++i) { + ASSERT_EQ(0, bthread_join(bth[i], NULL)); + } + for (size_t i = 0; i < arraysize(th); ++i) { + ASSERT_EQ(0, pthread_join(th[i], NULL)); + } + ASSERT_EQ(arraysize(th) + arraysize(bth), lid_seqs.size()); + std::sort(lid_seqs.begin(), lid_seqs.end()); + ASSERT_EQ(lid_seqs.end(), std::unique(lid_seqs.begin(), lid_seqs.end())); + ASSERT_EQ(arraysize(th) + arraysize(bth) - 1, + *(lid_seqs.end() - 1) - *lid_seqs.begin()); + + ASSERT_EQ(0, bthread_key_delete(key)); +} + } // namespace --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org