diff options
-rw-r--r-- | include/utils/RefBase.h | 15 | ||||
-rw-r--r-- | libs/utils/RefBase.cpp | 48 | ||||
-rw-r--r-- | services/surfaceflinger/Layer.cpp | 21 | ||||
-rw-r--r-- | services/surfaceflinger/Layer.h | 4 | ||||
-rw-r--r-- | services/surfaceflinger/LayerBase.cpp | 5 | ||||
-rw-r--r-- | services/surfaceflinger/LayerBase.h | 7 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 137 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 26 |
8 files changed, 150 insertions, 113 deletions
diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h index c24c0db..d92cfb0 100644 --- a/include/utils/RefBase.h +++ b/include/utils/RefBase.h @@ -112,10 +112,23 @@ public: getWeakRefs()->trackMe(enable, retain); } + // used to override the RefBase destruction. + class Destroyer { + friend class RefBase; + public: + virtual ~Destroyer(); + private: + virtual void destroy(RefBase const* base) = 0; + }; + + // Make sure to never acquire a strong reference from this function. The + // same restrictions than for destructors apply. + void setDestroyer(Destroyer* destroyer); + protected: RefBase(); virtual ~RefBase(); - + //! Flags for extendObjectLifetime() enum { OBJECT_LIFETIME_WEAK = 0x0001, diff --git a/libs/utils/RefBase.cpp b/libs/utils/RefBase.cpp index d28b751..545da7d 100644 --- a/libs/utils/RefBase.cpp +++ b/libs/utils/RefBase.cpp @@ -48,6 +48,11 @@ namespace android { // --------------------------------------------------------------------------- +RefBase::Destroyer::~Destroyer() { +} + +// --------------------------------------------------------------------------- + class RefBase::weakref_impl : public RefBase::weakref_type { public: @@ -55,7 +60,7 @@ public: volatile int32_t mWeak; RefBase* const mBase; volatile int32_t mFlags; - + Destroyer* mDestroyer; #if !DEBUG_REFS @@ -64,6 +69,7 @@ public: , mWeak(0) , mBase(base) , mFlags(0) + , mDestroyer(0) { } @@ -310,7 +316,11 @@ void RefBase::decStrong(const void* id) const if (c == 1) { const_cast<RefBase*>(this)->onLastStrongRef(id); if ((refs->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK) { - delete this; + if (refs->mDestroyer) { + refs->mDestroyer->destroy(this); + } else { + delete this; + } } } refs->removeWeakRef(id); @@ -345,7 +355,9 @@ int32_t RefBase::getStrongCount() const return mRefs->mStrong; } - +void RefBase::setDestroyer(RefBase::Destroyer* destroyer) { + mRefs->mDestroyer = destroyer; +} RefBase* RefBase::weakref_type::refBase() const { @@ -369,16 +381,28 @@ void RefBase::weakref_type::decWeak(const void* id) if (c != 1) return; if ((impl->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK) { - if (impl->mStrong == INITIAL_STRONG_VALUE) - delete impl->mBase; - else { -// LOGV("Freeing refs %p of old RefBase %p\n", this, impl->mBase); + if (impl->mStrong == INITIAL_STRONG_VALUE) { + if (impl->mBase) { + if (impl->mDestroyer) { + impl->mDestroyer->destroy(impl->mBase); + } else { + delete impl->mBase; + } + } + } else { + // LOGV("Freeing refs %p of old RefBase %p\n", this, impl->mBase); delete impl; } } else { impl->mBase->onLastWeakRef(id); if ((impl->mFlags&OBJECT_LIFETIME_FOREVER) != OBJECT_LIFETIME_FOREVER) { - delete impl->mBase; + if (impl->mBase) { + if (impl->mDestroyer) { + impl->mDestroyer->destroy(impl->mBase); + } else { + delete impl->mBase; + } + } } } } @@ -502,10 +526,10 @@ RefBase::RefBase() RefBase::~RefBase() { -// LOGV("Destroying RefBase %p (refs %p)\n", this, mRefs); - if (mRefs->mWeak == 0) { -// LOGV("Freeing refs %p of old RefBase %p\n", mRefs, this); - delete mRefs; + if ((mRefs->mFlags & OBJECT_LIFETIME_WEAK) == OBJECT_LIFETIME_WEAK) { + if (mRefs->mWeak == 0) { + delete mRefs; + } } } diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index dfee803..da06f61 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -60,6 +60,7 @@ Layer::Layer(SurfaceFlinger* flinger, mWidth(0), mHeight(0), mNeedsScaling(false), mFixedSize(false), mBypassState(false) { + setDestroyer(this); } Layer::~Layer() @@ -76,6 +77,10 @@ Layer::~Layer() } } +void Layer::destroy(RefBase const* base) { + mFlinger->destroyLayer(static_cast<LayerBase const*>(base)); +} + status_t Layer::setToken(const sp<UserClient>& userClient, SharedClient* sharedClient, int32_t token) { @@ -123,22 +128,6 @@ sp<LayerBaseClient::Surface> Layer::createSurface() const return mSurface; } -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(); - - EGLDisplay dpy(mFlinger->graphicPlane(0).getEGLDisplay()); - mBufferManager.destroy(dpy); - mSurface.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 7bb207a..2c4f756 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -44,7 +44,7 @@ class UserClient; // --------------------------------------------------------------------------- -class Layer : public LayerBaseClient +class Layer : public LayerBaseClient, private RefBase::Destroyer { public: Layer(SurfaceFlinger* flinger, DisplayID display, @@ -78,7 +78,6 @@ public: virtual bool needsFiltering() const; virtual bool isSecure() const { return mSecure; } virtual sp<Surface> createSurface() const; - virtual status_t ditch(); virtual void onRemoved(); virtual bool setBypass(bool enable); @@ -95,6 +94,7 @@ public: return mFreezeLock; } protected: + virtual void destroy(RefBase const* base); 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 21c36e1..3986fde 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -583,10 +583,7 @@ LayerBaseClient::Surface::~Surface() */ // destroy client resources - sp<LayerBaseClient> layer = getOwner(); - if (layer != 0) { - mFlinger->destroySurface(layer); - } + mFlinger->destroySurface(mOwner); } sp<LayerBaseClient> LayerBaseClient::Surface::getOwner() const { diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index afc5ec8..e69cb6a 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -196,10 +196,6 @@ public: */ virtual bool isSecure() 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() { }; @@ -264,7 +260,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 c08e2c9..a93d756 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -358,6 +358,9 @@ bool SurfaceFlinger::threadLoop() { waitForEvent(); + // call Layer's destructor + handleDestroyLayers(); + // check for transactions if (UNLIKELY(mConsoleSignals)) { handleConsoleEvents(); @@ -366,7 +369,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); } @@ -467,37 +470,26 @@ void SurfaceFlinger::handleConsoleEvents() void SurfaceFlinger::handleTransaction(uint32_t transactionFlags) { - Vector< sp<LayerBase> > 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; - // 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 ; i<count ; i++) { - if (ditchedLayers[i] != 0) { - //LOGD("ditching layer %p", ditchedLayers[i].get()); - ditchedLayers[i]->ditch(); - } - } + mLastTransactionTime = systemTime() - now; + mDebugInTransaction = 0; + // here the transaction has been committed } -void SurfaceFlinger::handleTransactionLocked( - uint32_t transactionFlags, Vector< sp<LayerBase> >& ditchedLayers) +void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) { const LayerVector& currentLayers(mCurrentState.layersSortedByZ); const size_t count = currentLayers.size(); @@ -569,7 +561,6 @@ void SurfaceFlinger::handleTransactionLocked( const sp<LayerBase>& layer(previousLayers[i]); if (currentLayers.indexOf( layer ) < 0) { // this layer is not visible anymore - ditchedLayers.add(layer); mDirtyRegionRemovedLayer.orSelf(layer->visibleRegionScreen); } } @@ -579,6 +570,31 @@ void SurfaceFlinger::handleTransactionLocked( commitTransaction(); } +void SurfaceFlinger::destroyLayer(LayerBase const* layer) +{ + Mutex::Autolock _l(mDestroyedLayerLock); + mDestroyedLayers.add(layer); + signalEvent(); +} + +void SurfaceFlinger::handleDestroyLayers() +{ + Vector<LayerBase const *> 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 ; i<count ; i++) { + //LOGD("destroying %s", destroyedLayers[i]->getName().string()); + delete destroyedLayers[i]; + } +} + sp<FreezeLock> SurfaceFlinger::getFreezeLock() const { return new FreezeLock(const_cast<SurfaceFlinger *>(this)); @@ -1030,15 +1046,15 @@ status_t SurfaceFlinger::addLayer_l(const sp<LayerBase>& layer) ssize_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<LayerBaseClient>& 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<LayerBase>& layer) @@ -1085,6 +1101,11 @@ status_t SurfaceFlinger::invalidateLayerVisibility(const sp<LayerBase>& 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; @@ -1314,38 +1335,18 @@ status_t SurfaceFlinger::removeSurface(const sp<Client>& client, SurfaceID sid) return err; } -status_t SurfaceFlinger::destroySurface(const sp<LayerBaseClient>& layer) +status_t SurfaceFlinger::destroySurface(const wp<LayerBaseClient>& layer) { // called by ~ISurface() when all references are gone - - class MessageDestroySurface : public MessageBase { - SurfaceFlinger* flinger; - sp<LayerBaseClient> layer; - public: - MessageDestroySurface( - SurfaceFlinger* flinger, const sp<LayerBaseClient>& layer) - : flinger(flinger), layer(layer) { } - virtual bool handler() { - sp<LayerBaseClient> 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); - 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<LayerBaseClient> 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( @@ -2253,15 +2254,17 @@ status_t Client::initCheck() const { return NO_ERROR; } -ssize_t Client::attachLayer(const sp<LayerBaseClient>& layer) +size_t Client::attachLayer(const sp<LayerBaseClient>& 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<count ; i++) { @@ -2271,9 +2274,11 @@ void Client::detachLayer(const LayerBaseClient* layer) } } } -sp<LayerBaseClient> Client::getLayerUser(int32_t i) const { +sp<LayerBaseClient> Client::getLayerUser(int32_t i) const +{ + Mutex::Autolock _l(mLock); sp<LayerBaseClient> lbc; - const wp<LayerBaseClient>& layer(mLayers.valueFor(i)); + wp<LayerBaseClient> 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 df1ca48..9fa98cf 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -66,7 +66,7 @@ public: status_t initCheck() const; // protected by SurfaceFlinger::mStateLock - ssize_t attachLayer(const sp<LayerBaseClient>& layer); + size_t attachLayer(const sp<LayerBaseClient>& layer); void detachLayer(const LayerBaseClient* layer); sp<LayerBaseClient> getLayerUser(int32_t i) const; @@ -82,9 +82,15 @@ private: virtual status_t destroySurface(SurfaceID surfaceId); virtual status_t setState(int32_t count, const layer_state_t* states); - DefaultKeyedVector< size_t, wp<LayerBaseClient> > mLayers; + // constant sp<SurfaceFlinger> mFlinger; - int32_t mNameGenerator; + + // protected by mLock + DefaultKeyedVector< size_t, wp<LayerBaseClient> > mLayers; + size_t mNameGenerator; + + // thread-safe + mutable Mutex mLock; }; class UserClient : public BnSurfaceComposerClient @@ -212,6 +218,7 @@ public: status_t removeLayer(const sp<LayerBase>& layer); status_t addLayer(const sp<LayerBase>& layer); status_t invalidateLayerVisibility(const sp<LayerBase>& layer); + void destroyLayer(LayerBase const* layer); sp<Layer> getLayer(const sp<ISurface>& sur) const; @@ -249,7 +256,7 @@ private: uint32_t w, uint32_t h, uint32_t flags); status_t removeSurface(const sp<Client>& client, SurfaceID sid); - status_t destroySurface(const sp<LayerBaseClient>& layer); + status_t destroySurface(const wp<LayerBaseClient>& layer); status_t setClientState(const sp<Client>& client, int32_t count, const layer_state_t* states); @@ -294,9 +301,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<LayerBase> >& ditchedLayers); + void handleTransactionLocked(uint32_t transactionFlags); + void handleDestroyLayers(); void computeVisibleRegions( LayerVector& currentLayers, @@ -319,6 +325,7 @@ private: status_t purgatorizeLayer_l(const sp<LayerBase>& layer); uint32_t getTransactionFlags(uint32_t flags); + uint32_t peekTransactionFlags(uint32_t flags); uint32_t setTransactionFlags(uint32_t flags); void commitTransaction(); @@ -415,6 +422,11 @@ private: // these are thread safe mutable Barrier mReadyToRunBarrier; + + // protected by mDestroyedLayerLock; + mutable Mutex mDestroyedLayerLock; + Vector<LayerBase const *> mDestroyedLayers; + // atomic variables enum { eConsoleReleased = 1, |