diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 5996c907d..1677c7687 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -108,7 +108,7 @@ Layer::Layer(SurfaceFlinger* flinger, const sp& client, void Layer::onFirstRef() { // Creates a custom BufferQueue for SurfaceFlingerConsumer to use - sp bq = new SurfaceTextureLayer(); + sp bq = new SurfaceTextureLayer(mFlinger); mSurfaceFlingerConsumer = new SurfaceFlingerConsumer(mTextureName, true, GL_TEXTURE_EXTERNAL_OES, false, bq); @@ -153,8 +153,9 @@ void Layer::onFrameAvailable() { 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 40876eced..1daadcce6 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -574,39 +574,7 @@ bool SurfaceFlinger::authenticateSurfaceTexture( const sp& bufferProducer) const { Mutex::Autolock _l(mStateLock); sp 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& 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 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& layer(mLayerPurgatory.itemAt(i)); - sp lbcBinder = layer->getBufferQueue()->asBinder(); - if (lbcBinder == surfaceTextureBinder) { - return true; - } - } - - return false; + return mGraphicBufferProducerList.indexOf(surfaceTextureBinder) >= 0; } status_t SurfaceFlinger::getDisplayInfo(const sp& display, DisplayInfo* info) { @@ -1676,6 +1644,7 @@ void SurfaceFlinger::drawWormhole(const sp& hw, void SurfaceFlinger::addClientLayer(const sp& client, const sp& handle, + const sp& gbc, const sp& lbc) { // attach this layer to the client @@ -1684,45 +1653,22 @@ void SurfaceFlinger::addClientLayer(const sp& client, // 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) { 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) -{ 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) -{ - // 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 @@ status_t SurfaceFlinger::createLayer( } if (result == NO_ERROR) { - addClientLayer(client, *handle, layer); + addClientLayer(client, *handle, *gbp, layer); setTransactionFlags(eTransactionNeeded); } return result; @@ -2010,44 +1956,25 @@ status_t SurfaceFlinger::createDimLayer(const sp& client, status_t SurfaceFlinger::onLayerRemoved(const sp& client, const sp& 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 = 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 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) { - // 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 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)); } @@ -2358,18 +2285,6 @@ void SurfaceFlinger::dumpAllLocked( layer->dump(result, buffer, SIZE); } - /* - * 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& 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 f12434792..e6734d2c1 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -128,6 +128,7 @@ private: 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 @@ private: // 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, const sp& handle); // called when all clients have released all their references to @@ -285,11 +284,10 @@ private: status_t removeLayer(const sp& layer); // add a layer to SurfaceFlinger - void addClientLayer(const sp& client, const sp& handle, - const sp& lbc); - - status_t removeLayer_l(const sp& layer); - status_t purgatorizeLayer_l(const sp& layer); + void addClientLayer(const sp& client, + const sp& handle, + const sp& gbc, + const sp& lbc); /* ------------------------------------------------------------------------ * Boot animation, on/off animations and screen capture @@ -403,10 +401,10 @@ private: State mCurrentState; volatile int32_t mTransactionFlags; Condition mTransactionCV; - SortedVector< sp > mLayerPurgatory; bool mTransactionPending; bool mAnimTransactionPending; Vector< sp > mLayersPendingRemoval; + SortedVector< wp > 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 395c8c835..d0f0daec9 100644 --- a/services/surfaceflinger/SurfaceTextureLayer.cpp +++ b/services/surfaceflinger/SurfaceTextureLayer.cpp @@ -20,17 +20,35 @@ #include +#include "SurfaceFlinger.h" #include "SurfaceTextureLayer.h" namespace android { // --------------------------------------------------------------------------- -SurfaceTextureLayer::SurfaceTextureLayer() - : BufferQueue(true) { +SurfaceTextureLayer::SurfaceTextureLayer(const sp& 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 flinger; + wp gbp; + public: + MessageCleanUpList(const sp& flinger, const wp& 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 a75ccf47d..13cff2fff 100644 --- a/services/surfaceflinger/SurfaceTextureLayer.h +++ b/services/surfaceflinger/SurfaceTextureLayer.h @@ -28,15 +28,16 @@ namespace android { // --------------------------------------------------------------------------- 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 flinger; public: - SurfaceTextureLayer(); - ~SurfaceTextureLayer(); + SurfaceTextureLayer(const sp& flinger); + virtual ~SurfaceTextureLayer(); // After calling the superclass connect(), set or clear synchronous // mode appropriately for the specified API.