Avoid potential racing condition when looking up mark names in PerformanceObserver (#39535)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39535

## Changelog:
[Internal] -

When looking up a mark by name (to be used by a measure for timing), there was a potential racing condition, which would cause issues if there is another thread e.g. logging a mark at the same time.

This fixes it.

Reviewed By: dmytrorykun

Differential Revision: D49416336

fbshipit-source-id: 28e20956f9aeafd79eb07130ea9dcd269086ea63
This commit is contained in:
Ruslan Shestopalyuk
2023-09-19 09:10:40 -07:00
committed by Facebook GitHub Bot
parent 8fe2da5391
commit d4c2d9dc97
2 changed files with 19 additions and 6 deletions
@@ -111,6 +111,7 @@ void PerformanceEntryReporter::logEntry(const RawPerformanceEntry& entry) {
if (buffer.hasNameLookup) {
auto overwriteCandidate = buffer.entries.getNextOverwriteCandidate();
if (overwriteCandidate != nullptr) {
std::scoped_lock lock2(nameLookupMutex_);
auto it = buffer.nameLookup.find(overwriteCandidate);
if (it != buffer.nameLookup.end() && *it == overwriteCandidate) {
buffer.nameLookup.erase(it);
@@ -128,6 +129,7 @@ void PerformanceEntryReporter::logEntry(const RawPerformanceEntry& entry) {
}
if (buffer.hasNameLookup) {
std::scoped_lock lock2(nameLookupMutex_);
buffer.nameLookup.insert(&buffer.entries.back());
}
@@ -163,6 +165,7 @@ void PerformanceEntryReporter::clearEntries(
auto& buffer = getBuffer(entryType);
if (entryName != nullptr) {
if (buffer.hasNameLookup) {
std::scoped_lock lock2(nameLookupMutex_);
RawPerformanceEntry entry{
entryName,
static_cast<int>(entryType),
@@ -173,12 +176,20 @@ void PerformanceEntryReporter::clearEntries(
std::nullopt};
buffer.nameLookup.erase(&entry);
}
std::scoped_lock lock(entriesMutex_);
buffer.entries.clear([entryName](const RawPerformanceEntry& entry) {
return std::strcmp(entry.name.c_str(), entryName) == 0;
});
} else {
buffer.entries.clear();
buffer.nameLookup.clear();
{
std::scoped_lock lock(entriesMutex_);
buffer.entries.clear();
}
{
std::scoped_lock lock2(nameLookupMutex_);
buffer.nameLookup.clear();
}
}
}
}
@@ -193,6 +204,7 @@ void PerformanceEntryReporter::getEntries(
getEntries(static_cast<PerformanceEntryType>(i), entryName, res);
}
} else {
std::scoped_lock lock(entriesMutex_);
const auto& entries = getBuffer(entryType).entries;
if (entryName == nullptr) {
entries.getEntries(res);
@@ -251,6 +263,7 @@ double PerformanceEntryReporter::getMarkTime(
std::nullopt,
std::nullopt};
std::scoped_lock lock(nameLookupMutex_);
const auto& marksBuffer = getBuffer(PerformanceEntryType::MARK);
auto it = marksBuffer.nameLookup.find(&mark);
if (it != marksBuffer.nameLookup.end()) {
@@ -154,12 +154,10 @@ class PerformanceEntryReporter : public EventLogger {
private:
std::optional<AsyncCallback<>> callback_;
std::mutex entriesMutex_;
mutable std::mutex entriesMutex_;
std::array<PerformanceEntryBuffer, NUM_PERFORMANCE_ENTRY_TYPES> buffers_;
std::unordered_map<std::string, uint32_t> eventCounts_;
// Mark registry for "measure" lookup
PerformanceEntryRegistryType marksRegistry_;
uint32_t droppedEntryCount_{0};
struct EventEntry {
@@ -173,10 +171,12 @@ class PerformanceEntryReporter : public EventLogger {
// but since we only report discrete events, the volume is normally low,
// so a hash map should be just fine.
std::unordered_map<EventTag, EventEntry> eventsInFlight_;
std::mutex eventsInFlightMutex_;
mutable std::mutex eventsInFlightMutex_;
std::function<double()> timeStampProvider_ = nullptr;
mutable std::mutex nameLookupMutex_;
static EventTag sCurrentEventTag_;
PerformanceEntryReporter();