ADHOC/replicant-frameworks_native
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

merge various SF fixes from gingerbread to honeycomb-mr2 (DO NOT MERGE)

Browse Source 
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
This commit is contained in:
Mathias Agopian 2011-06-06 09:55:15 -07:00
parent 2768505342
commit f3d939c5e5
8 changed files with 115 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
include/utils/RefBase.h
View File

@ -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,

12
libs/utils/RefBase.cpp
View File

@ -345,6 +345,10 @@ void RefBase::incStrong(const void* id) const
const_cast<RefBase*>(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<RefBase*>(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();
}
}
}

16
services/surfaceflinger/Layer.cpp
View File

@ -77,6 +77,10 @@ Layer::~Layer()
}
}
void Layer::destroy() const {
mFlinger->destroyLayer(this);
}
status_t Layer::setToken(const sp<UserClient>& userClient,
SharedClient* sharedClient, int32_t token)
{
@ -145,18 +149,6 @@ sp<LayerBaseClient::Surface> 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)
{

2
services/surfaceflinger/Layer.h
View File

@ -82,7 +82,6 @@ public:
virtual bool isSecure() const { return mSecure; }
virtual bool isProtected() const;
virtual sp<Surface> 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:

5
services/surfaceflinger/LayerBase.cpp
View File

@ -616,10 +616,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 {

7
services/surfaceflinger/LayerBase.h
View File

@ -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:

152
services/surfaceflinger/SurfaceFlinger.cpp
View File

@ -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
}
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();
@ -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));

26
services/surfaceflinger/SurfaceFlinger.h
View File

@ -65,7 +65,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;
@ -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<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
@ -227,6 +233,7 @@ public:
status_t addLayer(const sp<LayerBase>& layer);
status_t invalidateLayerVisibility(const sp<LayerBase>& layer);
void invalidateHwcGeometry();
void destroyLayer(LayerBase const* layer);
sp<Layer> getLayer(const sp<ISurface>& sur) const;
@ -254,7 +261,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);
@ -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<LayerBase> >& ditchedLayers);
void handleTransactionLocked(uint32_t transactionFlags);
void handleDestroyLayers();
void computeVisibleRegions(
LayerVector& currentLayers,
@ -324,6 +330,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();
@ -422,6 +429,11 @@ private:
// these are thread safe
mutable Barrier mReadyToRunBarrier;
// protected by mDestroyedLayerLock;
mutable Mutex mDestroyedLayerLock;
Vector<LayerBase const *> mDestroyedLayers;
// atomic variables
enum {
eConsoleReleased = 1,