Skip to content

Commit 1c0a923

Browse files
tanx16copybara-github
authored andcommitted
Automated rollback of commit 5fc0e22.
PiperOrigin-RevId: 734287833
1 parent 7638269 commit 1c0a923

File tree

2 files changed

+32
-12
lines changed

2 files changed

+32
-12
lines changed

src/google/protobuf/descriptor.h

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2530,22 +2530,18 @@ class PROTOBUF_EXPORT DescriptorPool {
25302530
auto key = std::pair<const void*, const void*>(field, &type_key);
25312531
{
25322532
absl::ReaderMutexLock lock(&field_memo_table_mutex_);
2533-
auto it = field_memo_table_.find(key);
2534-
if (it != field_memo_table_.end()) {
2533+
auto it = field_memo_table_->find(key);
2534+
if (it != field_memo_table_->end()) {
25352535
return internal::DownCast<const MemoData<ResultT>&>(*it->second).value;
25362536
}
25372537
}
25382538
auto result = std::make_unique<MemoData<ResultT>>();
25392539
result->value = func(field);
25402540
{
25412541
absl::MutexLock lock(&field_memo_table_mutex_);
2542-
auto& res = field_memo_table_[key];
2543-
// Only initialize the first time. We don't want to invalidate old
2544-
// references.
2545-
if (res == nullptr) {
2546-
res = std::move(result);
2547-
}
2548-
return internal::DownCast<const MemoData<ResultT>&>(*res).value;
2542+
auto insert_result = field_memo_table_->insert({key, std::move(result)});
2543+
auto it = insert_result.first;
2544+
return internal::DownCast<const MemoData<ResultT>&>(*it->second).value;
25492545
}
25502546
}
25512547
// Return true if the given name is a sub-symbol of any non-package
@@ -2608,9 +2604,12 @@ class PROTOBUF_EXPORT DescriptorPool {
26082604

26092605
#ifndef SWIG
26102606
mutable absl::Mutex field_memo_table_mutex_;
2611-
mutable absl::flat_hash_map<std::pair<const void*, const void*>,
2612-
std::unique_ptr<MemoBase>>
2613-
field_memo_table_ ABSL_GUARDED_BY(field_memo_table_mutex_);
2607+
mutable std::unique_ptr<absl::flat_hash_map<
2608+
std::pair<const void*, const void*>, std::unique_ptr<MemoBase>>>
2609+
field_memo_table_ ABSL_GUARDED_BY(field_memo_table_mutex_) =
2610+
std::make_unique<
2611+
absl::flat_hash_map<std::pair<const void*, const void*>,
2612+
std::unique_ptr<MemoBase>>>();
26142613
#endif // SWIG
26152614

26162615
// If fallback_database_ is nullptr, this is nullptr. Otherwise, this is a

src/google/protobuf/descriptor_unittest.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12178,6 +12178,27 @@ TEST_F(DescriptorPoolMemoizationTest, MemoizeProjectionMultithreaded) {
1217812178
}
1217912179
}
1218012180

12181+
TEST_F(DescriptorPoolMemoizationTest, MemoizeProjectionInsertionRace) {
12182+
auto name_lambda = [](const FieldDescriptor* field) {
12183+
return field->full_name();
12184+
};
12185+
proto2_unittest::TestAllTypes message;
12186+
const Descriptor* descriptor = message.GetDescriptor();
12187+
std::vector<std::thread> threads;
12188+
for (int i = 0; i < descriptor->field_count(); ++i) {
12189+
for (int j = 0; j < 3; ++j) {
12190+
threads.emplace_back([this, name_lambda, descriptor, i]() {
12191+
auto name = DescriptorPoolMemoizationTest::MemoizeProjection(
12192+
descriptor->file()->pool(), descriptor->field(i), name_lambda);
12193+
ASSERT_THAT(name, HasSubstr("proto2_unittest.TestAllTypes"));
12194+
});
12195+
}
12196+
}
12197+
for (auto& thread : threads) {
12198+
thread.join();
12199+
}
12200+
}
12201+
1218112202

1218212203

1218312204

0 commit comments

Comments
 (0)