Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix using butex in return keytable #2558

Merged
merged 2 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/bthread/task_group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
Expand Down
52 changes: 52 additions & 0 deletions test/bthread_key_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading