get rid of purgatory and fix QueuesToWindowComposer query
the purgatory list wasn't needed anymore; in fact it had no effect as
buffer life-time management is now handled by the BufferQueue.
For QueuesToWindowComposer we keep a list of wp<> on the IBinder
for IGraphicBufferProducers we hand over to clients so we can
easily check if an IGraphicBufferProducer is ours. We clean-up the
list when our IGraphicBufferProducer are destroyed.
Bug: 8349142
Change-Id: I1aa06652ade8c72d0004a3f5e6c3d6e8a82fc2ae
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 5996c90..1677c76 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -108,7 +108,7 @@
void Layer::onFirstRef()
{
// Creates a custom BufferQueue for SurfaceFlingerConsumer to use
- sp<BufferQueue> bq = new SurfaceTextureLayer();
+ sp<BufferQueue> bq = new SurfaceTextureLayer(mFlinger);
mSurfaceFlingerConsumer = new SurfaceFlingerConsumer(mTextureName, true,
GL_TEXTURE_EXTERNAL_OES, false, bq);
@@ -153,8 +153,9 @@
mFlinger->signalLayerUpdate();
}
-// called with SurfaceFlinger::mStateLock as soon as the layer is entered
-// in the purgatory list
+// called with SurfaceFlinger::mStateLock from the drawing thread after
+// the layer has been remove from the current state list (and just before
+// it's removed from the drawing state list)
void Layer::onRemoved() {
mSurfaceFlingerConsumer->abandon();
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 98f6ded..fba6bba 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -574,39 +574,7 @@
const sp<IGraphicBufferProducer>& bufferProducer) const {
Mutex::Autolock _l(mStateLock);
sp<IBinder> surfaceTextureBinder(bufferProducer->asBinder());
-
- // We want to determine whether the IGraphicBufferProducer was created by
- // SurfaceFlinger. Check to see if we can find it in the layer list.
- const LayerVector& currentLayers = mCurrentState.layersSortedByZ;
- size_t count = currentLayers.size();
- for (size_t i=0 ; i<count ; i++) {
- const sp<Layer>& layer(currentLayers[i]);
- // Get the consumer interface of SurfaceFlingerConsumer's
- // BufferQueue. If it's the same Binder object as the graphic
- // buffer producer interface, return success.
- sp<IBinder> lbcBinder = layer->getBufferQueue()->asBinder();
- if (lbcBinder == surfaceTextureBinder) {
- return true;
- }
- }
-
- // Check the layers in the purgatory. This check is here so that if a
- // GLConsumer gets destroyed before all the clients are done using it,
- // the error will not be reported as "surface XYZ is not authenticated", but
- // will instead fail later on when the client tries to use the surface,
- // which should be reported as "surface XYZ returned an -ENODEV". The
- // purgatorized layers are no less authentic than the visible ones, so this
- // should not cause any harm.
- size_t purgatorySize = mLayerPurgatory.size();
- for (size_t i=0 ; i<purgatorySize ; i++) {
- const sp<Layer>& layer(mLayerPurgatory.itemAt(i));
- sp<IBinder> lbcBinder = layer->getBufferQueue()->asBinder();
- if (lbcBinder == surfaceTextureBinder) {
- return true;
- }
- }
-
- return false;
+ return mGraphicBufferProducerList.indexOf(surfaceTextureBinder) >= 0;
}
status_t SurfaceFlinger::getDisplayInfo(const sp<IBinder>& display, DisplayInfo* info) {
@@ -1676,6 +1644,7 @@
void SurfaceFlinger::addClientLayer(const sp<Client>& client,
const sp<IBinder>& handle,
+ const sp<IGraphicBufferProducer>& gbc,
const sp<Layer>& lbc)
{
// attach this layer to the client
@@ -1684,45 +1653,22 @@
// add this layer to the current state list
Mutex::Autolock _l(mStateLock);
mCurrentState.layersSortedByZ.add(lbc);
+ mGraphicBufferProducerList.add(gbc->asBinder());
}
status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer)
{
Mutex::Autolock _l(mStateLock);
- status_t err = purgatorizeLayer_l(layer);
- if (err == NO_ERROR)
- setTransactionFlags(eTransactionNeeded);
- return err;
-}
-
-status_t SurfaceFlinger::removeLayer_l(const sp<Layer>& layer)
-{
ssize_t index = mCurrentState.layersSortedByZ.remove(layer);
if (index >= 0) {
+ mLayersPendingRemoval.push(layer);
mLayersRemoved = true;
+ setTransactionFlags(eTransactionNeeded);
return NO_ERROR;
}
return status_t(index);
}
-status_t SurfaceFlinger::purgatorizeLayer_l(const sp<Layer>& layer)
-{
- // First add the layer to the purgatory list, which makes sure it won't
- // go away, then remove it from the main list (through a transaction).
- ssize_t err = removeLayer_l(layer);
- if (err >= 0) {
- mLayerPurgatory.add(layer);
- }
-
- mLayersPendingRemoval.push(layer);
-
- // it's possible that we don't find a layer, because it might
- // have been destroyed already -- this is not technically an error
- // from the user because there is a race between Client::destroySurface(),
- // ~Client() and ~LayerCleaner().
- return (err == NAME_NOT_FOUND) ? status_t(NO_ERROR) : err;
-}
-
uint32_t SurfaceFlinger::peekTransactionFlags(uint32_t flags)
{
return android_atomic_release_load(&mTransactionFlags);
@@ -1957,7 +1903,7 @@
}
if (result == NO_ERROR) {
- addClientLayer(client, *handle, layer);
+ addClientLayer(client, *handle, *gbp, layer);
setTransactionFlags(eTransactionNeeded);
}
return result;
@@ -2010,44 +1956,25 @@
status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)
{
- /*
- * called by the window manager, when a surface should be marked for
- * destruction.
- *
- * The surface is removed from the current and drawing lists, but placed
- * in the purgatory queue, so it's not destroyed right-away (we need
- * to wait for all client's references to go away first).
- */
-
- status_t err = NAME_NOT_FOUND;
- Mutex::Autolock _l(mStateLock);
- sp<Layer> layer = client->getLayerUser(handle);
-
- if (layer != 0) {
- err = purgatorizeLayer_l(layer);
- if (err == NO_ERROR) {
- setTransactionFlags(eTransactionNeeded);
- }
+ // called by the window manager when it wants to remove a Layer
+ status_t err = NO_ERROR;
+ sp<Layer> l(client->getLayerUser(handle));
+ if (l != NULL) {
+ err = removeLayer(l);
+ ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
+ "error removing layer=%p (%s)", l.get(), strerror(-err));
}
return err;
}
status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
{
- // called by ~LayerCleaner() when all references are gone
+ // called by ~LayerCleaner() when all references to the IBinder (handle)
+ // are gone
status_t err = NO_ERROR;
sp<Layer> l(layer.promote());
if (l != NULL) {
- Mutex::Autolock _l(mStateLock);
- err = removeLayer_l(l);
- if (err == NAME_NOT_FOUND) {
- // The surface wasn't in the current list, which means it was
- // removed already, which means it is in the purgatory,
- // and need to be removed from there.
- ssize_t idx = mLayerPurgatory.remove(l);
- ALOGE_IF(idx < 0,
- "layer=%p is not in the purgatory list", l.get());
- }
+ err = removeLayer(l);
ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
"error removing layer=%p (%s)", l.get(), strerror(-err));
}
@@ -2359,18 +2286,6 @@
}
/*
- * Dump the layers in the purgatory
- */
-
- const size_t purgatorySize = mLayerPurgatory.size();
- snprintf(buffer, SIZE, "Purgatory state (%d entries)\n", purgatorySize);
- result.append(buffer);
- for (size_t i=0 ; i<purgatorySize ; i++) {
- const sp<Layer>& layer(mLayerPurgatory.itemAt(i));
- layer->shortDump(result, buffer, SIZE);
- }
-
- /*
* Dump Display state
*/
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index f124347..e6734d2 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -128,6 +128,7 @@
friend class Client;
friend class DisplayEventConnection;
friend class Layer;
+ friend class SurfaceTextureLayer;
// We're reference counted, never destroy SurfaceFlinger directly
virtual ~SurfaceFlinger();
@@ -272,8 +273,6 @@
// called in response to the window-manager calling
// ISurfaceComposerClient::destroySurface()
- // The specified layer is first placed in a purgatory list
- // until all references from the client are released.
status_t onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle);
// called when all clients have released all their references to
@@ -285,11 +284,10 @@
status_t removeLayer(const sp<Layer>& layer);
// add a layer to SurfaceFlinger
- void addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,
- const sp<Layer>& lbc);
-
- status_t removeLayer_l(const sp<Layer>& layer);
- status_t purgatorizeLayer_l(const sp<Layer>& layer);
+ void addClientLayer(const sp<Client>& client,
+ const sp<IBinder>& handle,
+ const sp<IGraphicBufferProducer>& gbc,
+ const sp<Layer>& lbc);
/* ------------------------------------------------------------------------
* Boot animation, on/off animations and screen capture
@@ -403,10 +401,10 @@
State mCurrentState;
volatile int32_t mTransactionFlags;
Condition mTransactionCV;
- SortedVector< sp<Layer> > mLayerPurgatory;
bool mTransactionPending;
bool mAnimTransactionPending;
Vector< sp<Layer> > mLayersPendingRemoval;
+ SortedVector< wp<IBinder> > mGraphicBufferProducerList;
// protected by mStateLock (but we could use another lock)
bool mLayersRemoved;
diff --git a/services/surfaceflinger/SurfaceTextureLayer.cpp b/services/surfaceflinger/SurfaceTextureLayer.cpp
index 395c8c8..d0f0dae 100644
--- a/services/surfaceflinger/SurfaceTextureLayer.cpp
+++ b/services/surfaceflinger/SurfaceTextureLayer.cpp
@@ -20,17 +20,35 @@
#include <utils/Errors.h>
+#include "SurfaceFlinger.h"
#include "SurfaceTextureLayer.h"
namespace android {
// ---------------------------------------------------------------------------
-SurfaceTextureLayer::SurfaceTextureLayer()
- : BufferQueue(true) {
+SurfaceTextureLayer::SurfaceTextureLayer(const sp<SurfaceFlinger>& flinger)
+ : BufferQueue(true), flinger(flinger) {
}
SurfaceTextureLayer::~SurfaceTextureLayer() {
+ // remove ourselves from SurfaceFlinger's list. We do this asynchronously
+ // because we don't know where this dtor is called from, it could be
+ // called with the mStateLock held, leading to a dead-lock (it actually
+ // happens).
+ class MessageCleanUpList : public MessageBase {
+ sp<SurfaceFlinger> flinger;
+ wp<IBinder> gbp;
+ public:
+ MessageCleanUpList(const sp<SurfaceFlinger>& flinger, const wp<IBinder>& gbp)
+ : flinger(flinger), gbp(gbp) { }
+ virtual bool handler() {
+ Mutex::Autolock _l(flinger->mStateLock);
+ flinger->mGraphicBufferProducerList.remove(gbp);
+ return true;
+ }
+ };
+ flinger->postMessageAsync( new MessageCleanUpList(flinger, this) );
}
status_t SurfaceTextureLayer::connect(int api, QueueBufferOutput* output) {
diff --git a/services/surfaceflinger/SurfaceTextureLayer.h b/services/surfaceflinger/SurfaceTextureLayer.h
index a75ccf4..13cff2f 100644
--- a/services/surfaceflinger/SurfaceTextureLayer.h
+++ b/services/surfaceflinger/SurfaceTextureLayer.h
@@ -28,15 +28,16 @@
// ---------------------------------------------------------------------------
class Layer;
+class SurfaceFlinger;
/*
* This is a thin wrapper around BufferQueue, used by the Layer class.
*/
-class SurfaceTextureLayer : public BufferQueue
-{
+class SurfaceTextureLayer : public BufferQueue {
+ sp<SurfaceFlinger> flinger;
public:
- SurfaceTextureLayer();
- ~SurfaceTextureLayer();
+ SurfaceTextureLayer(const sp<SurfaceFlinger>& flinger);
+ virtual ~SurfaceTextureLayer();
// After calling the superclass connect(), set or clear synchronous
// mode appropriately for the specified API.