Skip to content

Commit ae88617

Browse files
hadi88copybara-github
authored andcommitted
Document that the domain objects are not thread-safe.
PiperOrigin-RevId: 792730547
1 parent 5132f07 commit ae88617

File tree

2 files changed

+20
-30
lines changed

2 files changed

+20
-30
lines changed

fuzztest/internal/domains/domain.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ struct GenericPrinter {
6565
// `Domain<T>` is the type-erased domain interface.
6666
//
6767
// It can be constructed from any object derived from `DomainBase` that
68-
// implements the domain methods for the value type `T`.
68+
// implements the domain methods for the value type `T`. A Domain object is not
69+
// thread-safe. It's the domain object owner's responsibility to make sure the
70+
// domain object is not accessed concurrently by multiple threads.
6971
template <typename T>
7072
class Domain {
7173
public:

fuzztest/internal/domains/protobuf_domain_impl.h

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -349,12 +349,13 @@ class ProtoPolicy {
349349
const ProtoDescriptor* descriptor) {
350350
ABSL_CONST_INIT static absl::Mutex mutex(absl::kConstInit);
351351
static absl::NoDestructor<absl::flat_hash_map<
352-
const ProtoDescriptor*, std::vector<const FieldDescriptor*>>>
352+
const ProtoDescriptor*,
353+
std::unique_ptr<std::vector<const FieldDescriptor*>>>>
353354
descriptor_to_fields ABSL_GUARDED_BY(mutex);
354355
{
355356
absl::MutexLock l(&mutex);
356357
auto it = descriptor_to_fields->find(descriptor);
357-
if (it != descriptor_to_fields->end()) return it->second;
358+
if (it != descriptor_to_fields->end()) return *(it->second);
358359
}
359360
std::vector<const FieldDescriptor*> fields;
360361
fields.reserve(descriptor->field_count());
@@ -365,9 +366,10 @@ class ProtoPolicy {
365366
if (ShouldEnumerateExtensions(descriptor)) {
366367
descriptor->file()->pool()->FindAllExtensions(descriptor, &fields);
367368
}
368-
auto [it, _] =
369-
descriptor_to_fields->insert({descriptor, std::move(fields)});
370-
return it->second;
369+
auto [it, _] = descriptor_to_fields->insert(
370+
{descriptor, std::make_unique<std::vector<const FieldDescriptor*>>(
371+
std::move(fields))});
372+
return *(it->second);
371373
}
372374

373375
private:
@@ -408,12 +410,10 @@ class ProtoPolicy {
408410
class RecursiveFieldsCaches {
409411
public:
410412
void SetIsFieldFinitelyRecursive(const FieldDescriptor* field, bool value) {
411-
absl::MutexLock l(&field_to_is_finitely_recursive_mutex_);
412413
field_to_is_finitely_recursive_.insert({field, value});
413414
}
414415

415416
std::optional<bool> IsFieldFinitelyRecursive(const FieldDescriptor* field) {
416-
absl::ReaderMutexLock l(&field_to_is_finitely_recursive_mutex_);
417417
auto it = field_to_is_finitely_recursive_.find(field);
418418
return it != field_to_is_finitely_recursive_.end()
419419
? std::optional(it->second)
@@ -422,13 +422,11 @@ class ProtoPolicy {
422422

423423
void SetIsFieldInfinitelyRecursive(const FieldDescriptor* field,
424424
bool value) {
425-
absl::MutexLock l(&field_to_is_infinitely_recursive_mutex_);
426425
field_to_is_infinitely_recursive_.insert({field, value});
427426
}
428427

429428
std::optional<bool> IsFieldInfinitelyRecursive(
430429
const FieldDescriptor* field) {
431-
absl::ReaderMutexLock l(&field_to_is_infinitely_recursive_mutex_);
432430
auto it = field_to_is_infinitely_recursive_.find(field);
433431
return it != field_to_is_infinitely_recursive_.end()
434432
? std::optional(it->second)
@@ -437,32 +435,27 @@ class ProtoPolicy {
437435

438436
const std::vector<const FieldDescriptor*>* GetFields(
439437
const ProtoDescriptor* descriptor) {
440-
absl::ReaderMutexLock l(&proto_to_fields_mutex_);
441438
auto it = proto_to_fields_.find(descriptor);
442-
return it != proto_to_fields_.end() ? &it->second : nullptr;
439+
return it != proto_to_fields_.end() ? it->second.get() : nullptr;
443440
}
444441

445442
const std::vector<const FieldDescriptor*>& SetFields(
446443
const ProtoDescriptor* descriptor,
447444
std::vector<const FieldDescriptor*> fields) {
448-
absl::MutexLock l(&proto_to_fields_mutex_);
449-
auto [it, _] = proto_to_fields_.insert({descriptor, std::move(fields)});
450-
return it->second;
445+
auto [it, _] = proto_to_fields_.insert(
446+
{descriptor, std::make_unique<std::vector<const FieldDescriptor*>>(
447+
std::move(fields))});
448+
return *it->second;
451449
}
452450

453451
private:
454-
absl::Mutex field_to_is_finitely_recursive_mutex_;
455452
absl::flat_hash_map<const FieldDescriptor*, bool>
456-
field_to_is_finitely_recursive_
457-
ABSL_GUARDED_BY(field_to_is_finitely_recursive_mutex_);
458-
absl::Mutex field_to_is_infinitely_recursive_mutex_;
453+
field_to_is_finitely_recursive_;
459454
absl::flat_hash_map<const FieldDescriptor*, bool>
460-
field_to_is_infinitely_recursive_
461-
ABSL_GUARDED_BY(field_to_is_infinitely_recursive_mutex_);
462-
absl::Mutex proto_to_fields_mutex_;
455+
field_to_is_infinitely_recursive_;
463456
absl::flat_hash_map<const ProtoDescriptor*,
464-
std::vector<const FieldDescriptor*>>
465-
proto_to_fields_ ABSL_GUARDED_BY(proto_to_fields_mutex_);
457+
std::unique_ptr<std::vector<const FieldDescriptor*>>>
458+
proto_to_fields_;
466459
};
467460

468461
std::shared_ptr<RecursiveFieldsCaches> caches_ = nullptr;
@@ -601,7 +594,6 @@ class ProtobufDomainUntypedImpl
601594
ProtobufDomainUntypedImpl(const ProtobufDomainUntypedImpl& other)
602595
: prototype_(other.prototype_),
603596
use_lazy_initialization_(other.use_lazy_initialization_) {
604-
absl::MutexLock l(&other.mutex_);
605597
domains_ = other.domains_;
606598
policy_ = other.policy_;
607599
customized_fields_ = other.customized_fields_;
@@ -1432,7 +1424,6 @@ class ProtobufDomainUntypedImpl
14321424
"` but the field needs a message of type `",
14331425
field->message_type()->full_name(), "`.");
14341426
}
1435-
absl::MutexLock l(&self.mutex_);
14361427
auto res = self.domains_.try_emplace(field->number(),
14371428
std::in_place_type<DomainT>,
14381429
std::forward<Inner>(domain));
@@ -1694,7 +1685,6 @@ class ProtobufDomainUntypedImpl
16941685
using DomainT = decltype(GetDefaultDomainForField<T, is_repeated>(field));
16951686
// Do the operation under a lock to prevent race conditions in `const`
16961687
// methods.
1697-
absl::MutexLock l(&mutex_);
16981688
auto it = domains_.find(field->number());
16991689
if (it == domains_.end()) {
17001690
it = domains_
@@ -2019,9 +2009,7 @@ class ProtobufDomainUntypedImpl
20192009
PrototypePtr<Message> prototype_;
20202010
bool use_lazy_initialization_;
20212011

2022-
mutable absl::Mutex mutex_;
2023-
mutable absl::flat_hash_map<int, CopyableAny> domains_
2024-
ABSL_GUARDED_BY(mutex_);
2012+
mutable absl::flat_hash_map<int, CopyableAny> domains_;
20252013

20262014
ProtoPolicy<Message> policy_;
20272015
absl::flat_hash_set<int> customized_fields_;

0 commit comments

Comments
 (0)