diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h index f35508771..e88cb29b7 100644 --- a/include/utils/RefBase.h +++ b/include/utils/RefBase.h @@ -120,7 +120,14 @@ public: protected: RefBase(); virtual ~RefBase(); - + + // called when the last reference goes away. this is responsible for + // calling the destructor. The default implementation just does + // "delete this;". + // Make sure to never acquire a strong reference from this function. The + // same restrictions than for destructors apply. + virtual void destroy() const; + //! Flags for extendObjectLifetime() enum { OBJECT_LIFETIME_WEAK = 0x0001, diff --git a/libs/utils/RefBase.cpp b/libs/utils/RefBase.cpp index bb6c1255f..7de263326 100644 --- a/libs/utils/RefBase.cpp +++ b/libs/utils/RefBase.cpp @@ -345,6 +345,10 @@ void RefBase::incStrong(const void* id) const const_cast(this)->onFirstRef(); } +void RefBase::destroy() const { + delete this; +} + void RefBase::decStrong(const void* id) const { weakref_impl* const refs = mRefs; @@ -357,7 +361,7 @@ void RefBase::decStrong(const void* id) const if (c == 1) { const_cast(this)->onLastStrongRef(id); if ((refs->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK) { - delete this; + destroy(); } } refs->decWeak(id); @@ -415,7 +419,8 @@ void RefBase::weakref_type::decWeak(const void* id) if ((impl->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK) { if (impl->mStrong == INITIAL_STRONG_VALUE) - delete impl->mBase; + if (impl->mBase) + impl->mBase->destroy(); else { // LOGV("Freeing refs %p of old RefBase %p\n", this, impl->mBase); delete impl; @@ -423,7 +428,8 @@ void RefBase::weakref_type::decWeak(const void* id) } else { impl->mBase->onLastWeakRef(id); if ((impl->mFlags&OBJECT_LIFETIME_FOREVER) != OBJECT_LIFETIME_FOREVER) { - delete impl->mBase; + if (impl->mBase) + impl->mBase->destroy(); } } } diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 517c33592..1d75a7b55 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -77,6 +77,10 @@ Layer::~Layer() } } +void Layer::destroy() const { + mFlinger->destroyLayer(this); +} + status_t Layer::setToken(const sp& userClient, SharedClient* sharedClient, int32_t token) { @@ -145,18 +149,6 @@ sp Layer::createSurface() const return sur; } -status_t Layer::ditch() -{ - // NOTE: Called from the main UI thread - - // the layer is not on screen anymore. free as much resources as possible - mFreezeLock.clear(); - - Mutex::Autolock _l(mLock); - mWidth = mHeight = 0; - return NO_ERROR; -} - status_t Layer::setBuffers( uint32_t w, uint32_t h, PixelFormat format, uint32_t flags) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 128f93d84..278d64e97 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -82,7 +82,6 @@ public: virtual bool isSecure() const { return mSecure; } virtual bool isProtected() const; virtual sp createSurface() const; - virtual status_t ditch(); virtual void onRemoved(); // only for debugging @@ -93,6 +92,7 @@ public: return mFreezeLock; } protected: + virtual void destroy() const; virtual void dump(String8& result, char* scratch, size_t size) const; private: diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index 6025ed492..022f25145 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -616,10 +616,7 @@ LayerBaseClient::Surface::~Surface() */ // destroy client resources - sp layer = getOwner(); - if (layer != 0) { - mFlinger->destroySurface(layer); - } + mFlinger->destroySurface(mOwner); } sp LayerBaseClient::Surface::getOwner() const { diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index 7162e4727..6c49a1972 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -202,10 +202,6 @@ public: */ virtual bool isProtected() const { return false; } - /** Called from the main thread, when the surface is removed from the - * draw list */ - virtual status_t ditch() { return NO_ERROR; } - /** called with the state lock when the surface is removed from the * current list */ virtual void onRemoved() { }; @@ -271,7 +267,8 @@ protected: volatile int32_t mInvalidate; -protected: +public: + // called from class SurfaceFlinger virtual ~LayerBase(); private: diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index a9fa1ef42..9a312a736 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -387,6 +387,9 @@ bool SurfaceFlinger::threadLoop() { waitForEvent(); + // call Layer's destructor + handleDestroyLayers(); + // check for transactions if (UNLIKELY(mConsoleSignals)) { handleConsoleEvents(); @@ -395,7 +398,7 @@ bool SurfaceFlinger::threadLoop() if (LIKELY(mTransactionCount == 0)) { // if we're in a global transaction, don't do anything. const uint32_t mask = eTransactionNeeded | eTraversalNeeded; - uint32_t transactionFlags = getTransactionFlags(mask); + uint32_t transactionFlags = peekTransactionFlags(mask); if (LIKELY(transactionFlags)) { handleTransaction(transactionFlags); } @@ -480,39 +483,26 @@ void SurfaceFlinger::handleConsoleEvents() void SurfaceFlinger::handleTransaction(uint32_t transactionFlags) { - Vector< sp > ditchedLayers; + Mutex::Autolock _l(mStateLock); + const nsecs_t now = systemTime(); + mDebugInTransaction = now; - /* - * Perform and commit the transaction - */ + // Here we're guaranteed that some transaction flags are set + // so we can call handleTransactionLocked() unconditionally. + // We call getTransactionFlags(), which will also clear the flags, + // with mStateLock held to guarantee that mCurrentState won't change + // until the transaction is committed. - { // scope for the lock - Mutex::Autolock _l(mStateLock); - const nsecs_t now = systemTime(); - mDebugInTransaction = now; - handleTransactionLocked(transactionFlags, ditchedLayers); - mLastTransactionTime = systemTime() - now; - mDebugInTransaction = 0; - invalidateHwcGeometry(); - // here the transaction has been committed - } + const uint32_t mask = eTransactionNeeded | eTraversalNeeded; + transactionFlags = getTransactionFlags(mask); + handleTransactionLocked(transactionFlags); - /* - * Clean-up all layers that went away - * (do this without the lock held) - */ - - const size_t count = ditchedLayers.size(); - for (size_t i=0 ; iditch(); - } - } + mLastTransactionTime = systemTime() - now; + mDebugInTransaction = 0; + // here the transaction has been committed } -void SurfaceFlinger::handleTransactionLocked( - uint32_t transactionFlags, Vector< sp >& ditchedLayers) +void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) { const LayerVector& currentLayers(mCurrentState.layersSortedByZ); const size_t count = currentLayers.size(); @@ -584,7 +574,6 @@ void SurfaceFlinger::handleTransactionLocked( const sp& layer(previousLayers[i]); if (currentLayers.indexOf( layer ) < 0) { // this layer is not visible anymore - ditchedLayers.add(layer); mDirtyRegionRemovedLayer.orSelf(layer->visibleRegionScreen); } } @@ -594,6 +583,31 @@ void SurfaceFlinger::handleTransactionLocked( commitTransaction(); } +void SurfaceFlinger::destroyLayer(LayerBase const* layer) +{ + Mutex::Autolock _l(mDestroyedLayerLock); + mDestroyedLayers.add(layer); + signalEvent(); +} + +void SurfaceFlinger::handleDestroyLayers() +{ + Vector destroyedLayers; + + { // scope for the lock + Mutex::Autolock _l(mDestroyedLayerLock); + destroyedLayers = mDestroyedLayers; + mDestroyedLayers.clear(); + } + + // call destructors without a lock held + const size_t count = destroyedLayers.size(); + for (size_t i=0 ; igetName().string()); + delete destroyedLayers[i]; + } +} + sp SurfaceFlinger::getFreezeLock() const { return new FreezeLock(const_cast(this)); @@ -1096,15 +1110,15 @@ status_t SurfaceFlinger::addLayer_l(const sp& layer) ssize_t SurfaceFlinger::addClientLayer(const sp& client, const sp& lbc) { - Mutex::Autolock _l(mStateLock); - // attach this layer to the client - ssize_t name = client->attachLayer(lbc); + size_t name = client->attachLayer(lbc); + + Mutex::Autolock _l(mStateLock); // add this layer to the current state list addLayer_l(lbc); - return name; + return ssize_t(name); } status_t SurfaceFlinger::removeLayer(const sp& layer) @@ -1155,6 +1169,11 @@ status_t SurfaceFlinger::invalidateLayerVisibility(const sp& layer) return NO_ERROR; } +uint32_t SurfaceFlinger::peekTransactionFlags(uint32_t flags) +{ + return android_atomic_release_load(&mTransactionFlags); +} + uint32_t SurfaceFlinger::getTransactionFlags(uint32_t flags) { return android_atomic_and(~flags, &mTransactionFlags) & flags; @@ -1362,51 +1381,18 @@ status_t SurfaceFlinger::removeSurface(const sp& client, SurfaceID sid) return err; } -status_t SurfaceFlinger::destroySurface(const sp& layer) +status_t SurfaceFlinger::destroySurface(const wp& layer) { // called by ~ISurface() when all references are gone - - class MessageDestroySurface : public MessageBase { - SurfaceFlinger* flinger; - sp layer; - public: - MessageDestroySurface( - SurfaceFlinger* flinger, const sp& layer) - : flinger(flinger), layer(layer) { } - virtual bool handler() { - sp l(layer); - layer.clear(); // clear it outside of the lock; - Mutex::Autolock _l(flinger->mStateLock); - /* - * remove the layer from the current list -- chances are that it's - * not in the list anyway, because it should have been removed - * already upon request of the client (eg: window manager). - * However, a buggy client could have not done that. - * Since we know we don't have any more clients, we don't need - * to use the purgatory. - */ - status_t err = flinger->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. - // This needs to happen from the main thread since its dtor - // must run from there (b/c of OpenGL ES). Additionally, we - // can't really acquire our internal lock from - // destroySurface() -- see postMessage() below. - ssize_t idx = flinger->mLayerPurgatory.remove(l); - LOGE_IF(idx < 0, - "layer=%p is not in the purgatory list", l.get()); - } - - LOGE_IF(err<0 && err != NAME_NOT_FOUND, - "error removing layer=%p (%s)", l.get(), strerror(-err)); - return true; - } - }; - - postMessageAsync( new MessageDestroySurface(this, layer) ); - return NO_ERROR; + status_t err = NO_ERROR; + sp l(layer.promote()); + if (l != NULL) { + Mutex::Autolock _l(mStateLock); + err = removeLayer_l(l); + LOGE_IF(err<0 && err != NAME_NOT_FOUND, + "error removing layer=%p (%s)", l.get(), strerror(-err)); + } + return err; } status_t SurfaceFlinger::setClientState( @@ -2381,15 +2367,17 @@ status_t Client::initCheck() const { return NO_ERROR; } -ssize_t Client::attachLayer(const sp& layer) +size_t Client::attachLayer(const sp& layer) { - int32_t name = android_atomic_inc(&mNameGenerator); + Mutex::Autolock _l(mLock); + size_t name = mNameGenerator++; mLayers.add(name, layer); return name; } void Client::detachLayer(const LayerBaseClient* layer) { + Mutex::Autolock _l(mLock); // we do a linear search here, because this doesn't happen often const size_t count = mLayers.size(); for (size_t i=0 ; i Client::getLayerUser(int32_t i) const { +sp Client::getLayerUser(int32_t i) const +{ + Mutex::Autolock _l(mLock); sp lbc; - const wp& layer(mLayers.valueFor(i)); + wp layer(mLayers.valueFor(i)); if (layer != 0) { lbc = layer.promote(); LOGE_IF(lbc==0, "getLayerUser(name=%d) is dead", int(i)); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 95668190f..f81b074ea 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -65,7 +65,7 @@ public: status_t initCheck() const; // protected by SurfaceFlinger::mStateLock - ssize_t attachLayer(const sp& layer); + size_t attachLayer(const sp& layer); void detachLayer(const LayerBaseClient* layer); sp getLayerUser(int32_t i) const; @@ -81,9 +81,15 @@ private: virtual status_t destroySurface(SurfaceID surfaceId); virtual status_t setState(int32_t count, const layer_state_t* states); - DefaultKeyedVector< size_t, wp > mLayers; + // constant sp mFlinger; - int32_t mNameGenerator; + + // protected by mLock + DefaultKeyedVector< size_t, wp > mLayers; + size_t mNameGenerator; + + // thread-safe + mutable Mutex mLock; }; class UserClient : public BnSurfaceComposerClient @@ -227,6 +233,7 @@ public: status_t addLayer(const sp& layer); status_t invalidateLayerVisibility(const sp& layer); void invalidateHwcGeometry(); + void destroyLayer(LayerBase const* layer); sp getLayer(const sp& sur) const; @@ -254,7 +261,7 @@ private: uint32_t w, uint32_t h, uint32_t flags); status_t removeSurface(const sp& client, SurfaceID sid); - status_t destroySurface(const sp& layer); + status_t destroySurface(const wp& layer); status_t setClientState(const sp& client, int32_t count, const layer_state_t* states); @@ -299,9 +306,8 @@ public: // hack to work around gcc 4.0.3 bug private: void handleConsoleEvents(); void handleTransaction(uint32_t transactionFlags); - void handleTransactionLocked( - uint32_t transactionFlags, - Vector< sp >& ditchedLayers); + void handleTransactionLocked(uint32_t transactionFlags); + void handleDestroyLayers(); void computeVisibleRegions( LayerVector& currentLayers, @@ -324,6 +330,7 @@ private: status_t purgatorizeLayer_l(const sp& layer); uint32_t getTransactionFlags(uint32_t flags); + uint32_t peekTransactionFlags(uint32_t flags); uint32_t setTransactionFlags(uint32_t flags); void commitTransaction(); @@ -422,6 +429,11 @@ private: // these are thread safe mutable Barrier mReadyToRunBarrier; + + // protected by mDestroyedLayerLock; + mutable Mutex mDestroyedLayerLock; + Vector mDestroyedLayers; + // atomic variables enum { eConsoleReleased = 1,