diff options
author | Mathias Agopian <mathias@google.com> | 2011-06-06 09:55:15 -0700 |
---|---|---|
committer | Mathias Agopian <mathias@google.com> | 2011-06-06 15:14:25 -0700 |
commit | 24855c09173a6caaec7dcedd0c2d7ce15121d39b (patch) | |
tree | 1b72c39ca375da3f4c43ba5afcf9cde70dd7a53d /services/surfaceflinger/SurfaceFlinger.cpp | |
parent | b16b020dd32147fbaaf7de47d5de16bfee10967a (diff) | |
download | frameworks_base-24855c09173a6caaec7dcedd0c2d7ce15121d39b.zip frameworks_base-24855c09173a6caaec7dcedd0c2d7ce15121d39b.tar.gz frameworks_base-24855c09173a6caaec7dcedd0c2d7ce15121d39b.tar.bz2 |
merge various SF fixes from gingerbread to honeycomb-mr2 (DO NOT MERGE)
Fix a race that could cause GL commands to be executed from the wrong thread.
RefBase subclasses can now decide how they want to be destroyed.
Fix a race in SurfaceFlinger that could cause layers to be leaked forever.
Fix a race-condtion in SurfaceFlinger that could lead to a crash.
initial cherry-pick:
resolved conflicts for merge of b9783b49 to honeycomb-plus-aosp
Change-Id: I2a335e03fff219e35c18a7b0089b3a11d636576f
Diffstat (limited to 'services/surfaceflinger/SurfaceFlinger.cpp')
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 152 |
1 files changed, 71 insertions, 81 deletions
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index a9fa1ef..9a312a7 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<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; - invalidateHwcGeometry(); - // here the transaction has been committed - } - - /* - * Clean-up all layers that went away - * (do this without the lock held) - */ + const uint32_t mask = eTransactionNeeded | eTraversalNeeded; + transactionFlags = getTransactionFlags(mask); + handleTransactionLocked(transactionFlags); - 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(); @@ -584,7 +574,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); } } @@ -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<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)); @@ -1096,15 +1110,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) @@ -1155,6 +1169,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; @@ -1362,51 +1381,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); - 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<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( @@ -2381,15 +2367,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++) { @@ -2399,9 +2387,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)); |