SF TimeStats: fix a hazard issue
The setAcquireFence and setDesiredTime could happen in between the
splited setLayerName and setPostTime, where setLayerName inserts an
entry but setPostTime actually initializes the structure and waitData
position. This change combine setLayerName and setPostTime again to
eliminate the hazard.
Test: build
Bug: b/118770127
Change-Id: Ia1961cb688bbabf23d88305750e69c9fa41da8dd
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 5df8ade..73098bf 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -470,8 +470,7 @@
}
// TODO(marissaw): properly support mTimeStats
- mTimeStats.setLayerName(layerID, getName().c_str());
- mTimeStats.setPostTime(layerID, getFrameNumber(), latchTime);
+ mTimeStats.setPostTime(layerID, getFrameNumber(), getName().c_str(), latchTime);
mTimeStats.setAcquireFence(layerID, getFrameNumber(), getCurrentFenceTime());
mTimeStats.setLatchTime(layerID, getFrameNumber(), latchTime);
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 88c3c8a..2e564e7 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1476,9 +1476,8 @@
void Layer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
FrameEventHistoryDelta* outDelta) {
if (newTimestamps) {
- const int32_t layerID = getSequence();
- mTimeStats.setLayerName(layerID, getName().c_str());
- mTimeStats.setPostTime(layerID, newTimestamps->frameNumber, newTimestamps->postedTime);
+ mTimeStats.setPostTime(getSequence(), newTimestamps->frameNumber, getName().c_str(),
+ newTimestamps->postedTime);
}
Mutex::Autolock lock(mFrameEventHistoryMutex);
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index c219afd..ace7c1b 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -241,25 +241,18 @@
return std::regex_match(layerName.begin(), layerName.end(), layerNameRegex);
}
-void TimeStats::setLayerName(int32_t layerID, const std::string& layerName) {
+void TimeStats::setPostTime(int32_t layerID, uint64_t frameNumber, const std::string& layerName,
+ nsecs_t postTime) {
if (!mEnabled.load()) return;
ATRACE_CALL();
- ALOGV("[%d]-[%s]", layerID, layerName.c_str());
+ ALOGV("[%d]-[%" PRIu64 "]-[%s]-PostTime[%" PRId64 "]", layerID, frameNumber, layerName.c_str(),
+ postTime);
std::lock_guard<std::mutex> lock(mMutex);
if (!mTimeStatsTracker.count(layerID) && layerNameIsValid(layerName)) {
mTimeStatsTracker[layerID].layerName = layerName;
}
-}
-
-void TimeStats::setPostTime(int32_t layerID, uint64_t frameNumber, nsecs_t postTime) {
- if (!mEnabled.load()) return;
-
- ATRACE_CALL();
- ALOGV("[%d]-[%" PRIu64 "]-PostTime[%" PRId64 "]", layerID, frameNumber, postTime);
-
- std::lock_guard<std::mutex> lock(mMutex);
if (!mTimeStatsTracker.count(layerID)) return;
LayerRecord& layerRecord = mTimeStatsTracker[layerID];
if (layerRecord.timeRecords.size() == MAX_NUM_TIME_RECORDS) {
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index d1e554c..184bf40 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -86,8 +86,8 @@
void incrementMissedFrames();
void incrementClientCompositionFrames();
- void setLayerName(int32_t layerID, const std::string& layerName);
- void setPostTime(int32_t layerID, uint64_t frameNumber, nsecs_t postTime);
+ void setPostTime(int32_t layerID, uint64_t frameNumber, const std::string& layerName,
+ nsecs_t postTime);
void setLatchTime(int32_t layerID, uint64_t frameNumber, nsecs_t latchTime);
void setDesiredTime(int32_t layerID, uint64_t frameNumber, nsecs_t desiredTime);
void setAcquireTime(int32_t layerID, uint64_t frameNumber, nsecs_t acquireTime);