am c9cd2387: Merge changes I37f0f315,I8cbf6044,Ibb598931,I5262bf11 into gingerbread
* commit 'c9cd2387b6938a6fbefc731d2177902266f2a130': 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.
This commit is contained in:
commit
be93983384
|
@ -115,7 +115,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,
|
||||
|
|
|
@ -298,6 +298,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;
|
||||
|
@ -310,7 +314,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->removeWeakRef(id);
|
||||
|
@ -370,7 +374,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;
|
||||
|
@ -378,7 +383,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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -76,6 +76,10 @@ Layer::~Layer()
|
|||
}
|
||||
}
|
||||
|
||||
void Layer::destroy() const {
|
||||
mFlinger->destroyLayer(this);
|
||||
}
|
||||
|
||||
status_t Layer::setToken(const sp<UserClient>& userClient,
|
||||
SharedClient* sharedClient, int32_t token)
|
||||
{
|
||||
|
@ -123,22 +127,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)
|
||||
{
|
||||
|
|
|
@ -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() const;
|
||||
virtual void dump(String8& result, char* scratch, size_t size) const;
|
||||
|
||||
private:
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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));
|
||||
|
|
|
@ -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,
|
||||
|
|
Loading…
Reference in New Issue