SurfaceFlinger: protect state members in Layer
Add proper locking to protect state members in Layer.
These members are accessed by both the main thread and binder.
Bug: 119481871
Test: SurfaceFlinger unit tests
Test: go/wm-smoke
Change-Id: I12d47711992e09c0677b77f7e1b36c1254b63a1b
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index efc2c9f..8091b94 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -70,30 +70,31 @@
}
bool BufferStateLayer::willPresentCurrentTransaction() const {
+ Mutex::Autolock lock(mStateMutex);
// Returns true if the most recent Transaction applied to CurrentState will be presented.
return getSidebandStreamChanged() || getAutoRefresh() ||
- (mCurrentState.modified && mCurrentState.buffer != nullptr);
+ (mState.current.modified && mState.current.buffer != nullptr);
}
-bool BufferStateLayer::getTransformToDisplayInverse() const {
- return mCurrentState.transformToDisplayInverse;
+bool BufferStateLayer::getTransformToDisplayInverseLocked() const {
+ return mState.current.transformToDisplayInverse;
}
-void BufferStateLayer::pushPendingState() {
- if (!mCurrentState.modified) {
+void BufferStateLayer::pushPendingStateLocked() {
+ if (!mState.current.modified) {
return;
}
- mPendingStates.push_back(mCurrentState);
- ATRACE_INT(mTransactionName.string(), mPendingStates.size());
+ mState.pending.push_back(mState.current);
+ ATRACE_INT(mTransactionName.string(), mState.pending.size());
}
bool BufferStateLayer::applyPendingStates(Layer::State* stateToCommit) {
- const bool stateUpdateAvailable = !mPendingStates.empty();
- while (!mPendingStates.empty()) {
+ const bool stateUpdateAvailable = !mState.pending.empty();
+ while (!mState.pending.empty()) {
popPendingState(stateToCommit);
}
- mCurrentStateModified = stateUpdateAvailable && mCurrentState.modified;
- mCurrentState.modified = false;
+ mCurrentStateModified = stateUpdateAvailable && mState.current.modified;
+ mState.current.modified = false;
return stateUpdateAvailable;
}
@@ -103,28 +104,31 @@
}
bool BufferStateLayer::setTransform(uint32_t transform) {
- if (mCurrentState.transform == transform) return false;
- mCurrentState.sequence++;
- mCurrentState.transform = transform;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.transform == transform) return false;
+ mState.current.sequence++;
+ mState.current.transform = transform;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setTransformToDisplayInverse(bool transformToDisplayInverse) {
- if (mCurrentState.transformToDisplayInverse == transformToDisplayInverse) return false;
- mCurrentState.sequence++;
- mCurrentState.transformToDisplayInverse = transformToDisplayInverse;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.transformToDisplayInverse == transformToDisplayInverse) return false;
+ mState.current.sequence++;
+ mState.current.transformToDisplayInverse = transformToDisplayInverse;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setCrop(const Rect& crop) {
- if (mCurrentState.crop == crop) return false;
- mCurrentState.sequence++;
- mCurrentState.crop = crop;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.crop == crop) return false;
+ mState.current.sequence++;
+ mState.current.crop = crop;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
@@ -135,86 +139,94 @@
int w = frame.getWidth();
int h = frame.getHeight();
- if (mCurrentState.active.transform.tx() == x && mCurrentState.active.transform.ty() == y &&
- mCurrentState.active.w == w && mCurrentState.active.h == h) {
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.active.transform.tx() == x && mState.current.active.transform.ty() == y &&
+ mState.current.active.w == w && mState.current.active.h == h) {
return false;
}
if (!frame.isValid()) {
x = y = w = h = 0;
}
- mCurrentState.active.transform.set(x, y);
- mCurrentState.active.w = w;
- mCurrentState.active.h = h;
+ mState.current.active.transform.set(x, y);
+ mState.current.active.w = w;
+ mState.current.active.h = h;
- mCurrentState.sequence++;
- mCurrentState.modified = true;
+ mState.current.sequence++;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setBuffer(const sp<GraphicBuffer>& buffer) {
- if (mCurrentState.buffer) {
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.buffer) {
mReleasePreviousBuffer = true;
}
- mCurrentState.sequence++;
- mCurrentState.buffer = buffer;
- mCurrentState.modified = true;
+ mState.current.sequence++;
+ mState.current.buffer = buffer;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setAcquireFence(const sp<Fence>& fence) {
+ Mutex::Autolock lock(mStateMutex);
// The acquire fences of BufferStateLayers have already signaled before they are set
mCallbackHandleAcquireTime = fence->getSignalTime();
- mCurrentState.acquireFence = fence;
- mCurrentState.modified = true;
+ mState.current.acquireFence = fence;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setDataspace(ui::Dataspace dataspace) {
- if (mCurrentState.dataspace == dataspace) return false;
- mCurrentState.sequence++;
- mCurrentState.dataspace = dataspace;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.dataspace == dataspace) return false;
+ mState.current.sequence++;
+ mState.current.dataspace = dataspace;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setHdrMetadata(const HdrMetadata& hdrMetadata) {
- if (mCurrentState.hdrMetadata == hdrMetadata) return false;
- mCurrentState.sequence++;
- mCurrentState.hdrMetadata = hdrMetadata;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.hdrMetadata == hdrMetadata) return false;
+ mState.current.sequence++;
+ mState.current.hdrMetadata = hdrMetadata;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setSurfaceDamageRegion(const Region& surfaceDamage) {
- mCurrentState.sequence++;
- mCurrentState.surfaceDamageRegion = surfaceDamage;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ mState.current.sequence++;
+ mState.current.surfaceDamageRegion = surfaceDamage;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setApi(int32_t api) {
- if (mCurrentState.api == api) return false;
- mCurrentState.sequence++;
- mCurrentState.api = api;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.api == api) return false;
+ mState.current.sequence++;
+ mState.current.api = api;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool BufferStateLayer::setSidebandStream(const sp<NativeHandle>& sidebandStream) {
- if (mCurrentState.sidebandStream == sidebandStream) return false;
- mCurrentState.sequence++;
- mCurrentState.sidebandStream = sidebandStream;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ if (mState.current.sidebandStream == sidebandStream) return false;
+ mState.current.sequence++;
+ mState.current.sidebandStream = sidebandStream;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
if (!mSidebandStreamChanged.exchange(true)) {
@@ -248,7 +260,10 @@
mFlinger->getTransactionCompletedThread().registerPendingLatchedCallbackHandle(handle);
// Store so latched time and release fence can be set
- mCurrentState.callbackHandles.push_back(handle);
+ {
+ Mutex::Autolock lock(mStateMutex);
+ mState.current.callbackHandles.push_back(handle);
+ }
} else { // If this layer will NOT need to be relatched and presented this frame
// Notify the transaction completed thread this handle is done
@@ -263,8 +278,9 @@
}
bool BufferStateLayer::setTransparentRegionHint(const Region& transparent) {
- mCurrentState.transparentRegionHint = transparent;
- mCurrentState.modified = true;
+ Mutex::Autolock lock(mStateMutex);
+ mState.current.transparentRegionHint = transparent;
+ mState.current.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
@@ -300,6 +316,7 @@
return true;
}
+ Mutex::Autolock lock(mStateMutex);
return getDrawingState().acquireFence->getStatus() == Fence::Status::Signaled;
}
@@ -308,7 +325,7 @@
return 0;
}
-std::shared_ptr<FenceTime> BufferStateLayer::getCurrentFenceTime() const {
+std::shared_ptr<FenceTime> BufferStateLayer::getCurrentFenceTimeLocked() const {
return std::make_shared<FenceTime>(getDrawingState().acquireFence);
}
@@ -339,14 +356,17 @@
}
Region BufferStateLayer::getDrawingSurfaceDamage() const {
+ Mutex::Autolock lock(mStateMutex);
return getDrawingState().surfaceDamageRegion;
}
const HdrMetadata& BufferStateLayer::getDrawingHdrMetadata() const {
+ Mutex::Autolock lock(mStateMutex);
return getDrawingState().hdrMetadata;
}
int BufferStateLayer::getDrawingApi() const {
+ Mutex::Autolock lock(mStateMutex);
return getDrawingState().api;
}
@@ -371,6 +391,7 @@
}
std::optional<Region> BufferStateLayer::latchSidebandStream(bool& recomputeVisibleRegions) {
+ Mutex::Autolock lock(mStateMutex);
if (mSidebandStreamChanged.exchange(false)) {
const State& s(getDrawingState());
// mSidebandStreamChanged was true
@@ -382,12 +403,12 @@
}
recomputeVisibleRegions = true;
- return getTransform().transform(Region(Rect(s.active.w, s.active.h)));
+ return getTransformLocked().transform(Region(Rect(s.active.w, s.active.h)));
}
return {};
}
-bool BufferStateLayer::hasFrameUpdate() const {
+bool BufferStateLayer::hasFrameUpdateLocked() const {
return mCurrentStateModified && getCurrentState().buffer != nullptr;
}
@@ -397,6 +418,10 @@
}
status_t BufferStateLayer::bindTextureImage() {
+ Mutex::Autolock lock(mStateMutex);
+ return bindTextureImageLocked();
+}
+status_t BufferStateLayer::bindTextureImageLocked() {
const State& s(getDrawingState());
auto& engine(mFlinger->getRenderEngine());
@@ -501,7 +526,7 @@
auto incomingStatus = releaseFence->getStatus();
if (incomingStatus == Fence::Status::Invalid) {
ALOGE("New fence has invalid state");
- mDrawingState.acquireFence = releaseFence;
+ mState.drawing.acquireFence = releaseFence;
mFlinger->mTimeStats->onDestroy(layerID);
return BAD_VALUE;
}
@@ -512,16 +537,16 @@
char fenceName[32] = {};
snprintf(fenceName, 32, "%.28s:%d", mName.string(), mFrameNumber);
sp<Fence> mergedFence =
- Fence::merge(fenceName, mDrawingState.acquireFence, releaseFence);
+ Fence::merge(fenceName, mState.drawing.acquireFence, releaseFence);
if (!mergedFence.get()) {
ALOGE("failed to merge release fences");
// synchronization is broken, the best we can do is hope fences
// signal in order so the new fence will act like a union
- mDrawingState.acquireFence = releaseFence;
+ mState.drawing.acquireFence = releaseFence;
mFlinger->mTimeStats->onDestroy(layerID);
return BAD_VALUE;
}
- mDrawingState.acquireFence = mergedFence;
+ mState.drawing.acquireFence = mergedFence;
} else if (incomingStatus == Fence::Status::Unsignaled) {
// If one fence has signaled and the other hasn't, the unsignaled
// fence will approximately correspond with the correct timestamp.
@@ -530,7 +555,7 @@
// by this point, they will have both signaled and only the timestamp
// will be slightly off; any dependencies after this point will
// already have been met.
- mDrawingState.acquireFence = releaseFence;
+ mState.drawing.acquireFence = releaseFence;
}
} else {
// Bind the new buffer to the GL texture.
@@ -539,7 +564,7 @@
// by glEGLImageTargetTexture2DOES, which this method calls. Newer
// devices will either call this in Layer::onDraw, or (if it's not
// a GL-composited layer) not at all.
- status_t err = bindTextureImage();
+ status_t err = bindTextureImageLocked();
if (err != NO_ERROR) {
mFlinger->mTimeStats->onDestroy(layerID);
return BAD_VALUE;
@@ -548,7 +573,7 @@
// TODO(marissaw): properly support mTimeStats
mFlinger->mTimeStats->setPostTime(layerID, getFrameNumber(), getName().c_str(), latchTime);
- mFlinger->mTimeStats->setAcquireFence(layerID, getFrameNumber(), getCurrentFenceTime());
+ mFlinger->mTimeStats->setAcquireFence(layerID, getFrameNumber(), getCurrentFenceTimeLocked());
mFlinger->mTimeStats->setLatchTime(layerID, getFrameNumber(), latchTime);
return NO_ERROR;
@@ -575,6 +600,7 @@
}
void BufferStateLayer::setHwcLayerBuffer(DisplayId displayId) {
+ Mutex::Autolock lock(mStateMutex);
auto& hwcInfo = getBE().mHwcLayers[displayId];
auto& hwcLayer = hwcInfo.layer;