RESTRICT AUTOMERGE: Fix removal of handle from map
Bug: 144667224, 137284057
Test: build, boot, SurfaceFlinger_test, libgui_test, manual
Change-Id: Ie0f111a6b8f4424375cc124ccd2683f5bfad7759
diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp
index 09d87b2..6b54a14 100644
--- a/services/surfaceflinger/Client.cpp
+++ b/services/surfaceflinger/Client.cpp
@@ -106,16 +106,16 @@
}
}
}
-sp<Layer> Client::getLayerUser(const sp<IBinder>& handle) const
+
+bool Client::isAttached(const sp<IBinder>& handle) const
{
Mutex::Autolock _l(mLock);
sp<Layer> lbc;
wp<Layer> layer(mLayers.valueFor(handle));
if (layer != 0) {
- lbc = layer.promote();
- ALOGE_IF(lbc==0, "getLayerUser(name=%p) is dead", handle.get());
+ return true;
}
- return lbc;
+ return false;
}
status_t Client::onTransact(
@@ -150,7 +150,8 @@
sp<IBinder>* handle,
sp<IGraphicBufferProducer>* gbp) {
bool parentDied;
- sp<Layer> parentLayer = getParentLayer(&parentDied);
+ sp<Layer> parentLayer;
+ if (!parentHandle) parentLayer = getParentLayer(&parentDied);
if (parentHandle == nullptr && parentDied) {
return NAME_NOT_FOUND;
}
@@ -163,21 +164,11 @@
}
status_t Client::clearLayerFrameStats(const sp<IBinder>& handle) const {
- sp<Layer> layer = getLayerUser(handle);
- if (layer == nullptr) {
- return NAME_NOT_FOUND;
- }
- layer->clearFrameStats();
- return NO_ERROR;
+ return mFlinger->clearLayerFrameStats(this, handle);
}
status_t Client::getLayerFrameStats(const sp<IBinder>& handle, FrameStats* outStats) const {
- sp<Layer> layer = getLayerUser(handle);
- if (layer == nullptr) {
- return NAME_NOT_FOUND;
- }
- layer->getFrameStats(outStats);
- return NO_ERROR;
+ return mFlinger->getLayerFrameStats(this, handle, outStats);
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/Client.h b/services/surfaceflinger/Client.h
index 36b685d..c8da528 100644
--- a/services/surfaceflinger/Client.h
+++ b/services/surfaceflinger/Client.h
@@ -49,7 +49,7 @@
void detachLayer(const Layer* layer);
- sp<Layer> getLayerUser(const sp<IBinder>& handle) const;
+ bool isAttached (const sp<IBinder>& handle) const;
void updateParent(const sp<Layer>& parentLayer);
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 13b12e2..a062aa1 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -178,13 +178,8 @@
void Layer::onLayerDisplayed(const sp<Fence>& /*releaseFence*/) {}
void Layer::onRemovedFromCurrentState() {
- if (!mPendingRemoval) {
- // the layer is removed from SF mCurrentState to mLayersPendingRemoval
- mPendingRemoval = true;
-
- // remove from sf mapping
- mFlinger->removeLayerFromMap(this);
- }
+ // the layer is removed from SF mCurrentState to mLayersPendingRemoval
+ mPendingRemoval = true;
if (mCurrentState.zOrderRelativeOf != nullptr) {
sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 6ce2cb8..2c7f8c4 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3187,16 +3187,10 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {
- Mutex::Autolock _l(mStateLock);
- return removeLayerLocked(mStateLock, layer, topLevelOnly);
-}
-
-status_t SurfaceFlinger::removeLayerFromMap(Layer* layer) {
+status_t SurfaceFlinger::removeLayerFromMap(const wp<Layer>& layer) {
auto it = mLayersByLocalBinderToken.begin();
while (it != mLayersByLocalBinderToken.end()) {
- auto strongRef = it->second.promote();
- if (strongRef != nullptr && strongRef.get() == layer) {
+ if (it->second == layer) {
it = mLayersByLocalBinderToken.erase(it);
break;
} else {
@@ -3210,6 +3204,11 @@
return NO_ERROR;
}
+status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {
+ Mutex::Autolock _l(mStateLock);
+ return removeLayerLocked(mStateLock, layer, topLevelOnly);
+}
+
status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,
bool topLevelOnly) {
if (layer->isPendingRemoval()) {
@@ -3455,8 +3454,8 @@
const layer_state_t& s = composerState.state;
sp<Client> client(static_cast<Client*>(composerState.client.get()));
- sp<Layer> layer(client->getLayerUser(s.surface));
- if (layer == nullptr) {
+ sp<Layer> layer = fromHandle(s.surface);
+ if (layer == nullptr || !(client->isAttached(s.surface))) {
return 0;
}
@@ -3631,8 +3630,8 @@
const layer_state_t& state = composerState.state;
sp<Client> client(static_cast<Client*>(composerState.client.get()));
- sp<Layer> layer(client->getLayerUser(state.surface));
- if (layer == nullptr) {
+ sp<Layer> layer = fromHandle(state.surface);
+ if (layer == nullptr || !(client->isAttached(state.surface))) {
return;
}
@@ -3767,14 +3766,35 @@
return NO_ERROR;
}
+status_t SurfaceFlinger::clearLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle) {
+ Mutex::Autolock _l(mStateLock);
+ sp<Layer> layer = fromHandle(handle);
+ if (layer == nullptr || !(client->isAttached(handle))) {
+ return NAME_NOT_FOUND;
+ }
+ layer->clearFrameStats();
+ return NO_ERROR;
+}
+
+status_t SurfaceFlinger::getLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle, FrameStats* outStats) {
+ Mutex::Autolock _l(mStateLock);
+ sp<Layer> layer = fromHandle(handle);
+ if (layer == nullptr || !(client->isAttached(handle))) {
+ return NAME_NOT_FOUND;
+ }
+ layer->getFrameStats(outStats);
+ return NO_ERROR;
+}
+
status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)
{
+ Mutex::Autolock _l(mStateLock);
// called by a client when it wants to remove a Layer
status_t err = NO_ERROR;
- sp<Layer> l(client->getLayerUser(handle));
- if (l != nullptr) {
+ sp<Layer> l = fromHandle(handle);
+ if (l != nullptr || client->isAttached(handle)) {
mInterceptor->saveSurfaceDeletion(l);
- err = removeLayer(l);
+ err = removeLayerLocked(mStateLock, l);
ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
"error removing layer=%p (%s)", l.get(), strerror(-err));
}
@@ -3783,15 +3803,18 @@
status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
{
+ Mutex::Autolock _l(mStateLock);
// called by ~LayerCleaner() when all references to the IBinder (handle)
// are gone
sp<Layer> l = layer.promote();
if (l == nullptr) {
+ removeLayerFromMap(layer);
// The layer has already been removed, carry on
return NO_ERROR;
}
+ removeLayerFromMap(layer);
// If we have a parent, then we can continue to live as long as it does.
- return removeLayer(l, true);
+ return removeLayerLocked(mStateLock, l, true);
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 11cdad1..1f8c205 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -347,6 +347,10 @@
int getPrimaryDisplayOrientation() const { return mPrimaryDisplayOrientation; }
+ status_t clearLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle);
+
+ status_t getLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle, FrameStats* outStats);
+
sp<Layer> fromHandle(const sp<IBinder>& handle) REQUIRES(mStateLock);
private:
@@ -523,9 +527,9 @@
uint32_t setTransactionFlags(uint32_t flags, VSyncModulator::TransactionStart transactionStart);
void commitTransaction();
bool containsAnyInvalidClientState(const Vector<ComposerState>& states);
- uint32_t setClientStateLocked(const ComposerState& composerState);
+ uint32_t setClientStateLocked(const ComposerState& composerState) REQUIRES(mStateLock);
uint32_t setDisplayStateLocked(const DisplayState& s);
- void setDestroyStateLocked(const ComposerState& composerState);
+ void setDestroyStateLocked(const ComposerState& composerState) REQUIRES(mStateLock);
/* ------------------------------------------------------------------------
* Layer management
@@ -560,7 +564,7 @@
status_t removeLayerLocked(const Mutex&, const sp<Layer>& layer, bool topLevelOnly = false);
// remove layer from mapping
- status_t removeLayerFromMap(Layer* layer);
+ status_t removeLayerFromMap(const wp<Layer>& layer);
// add a layer to SurfaceFlinger
status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,
