Fix a race that could cause GL commands to be executed from the wrong thread. (b4081bb5) · Commits · e / devices / smdk4412 / android_frameworks_native

services/surfaceflinger/Layer.cpp

+4 −16

Original line number	Original line	Diff line number	Diff line
	@@ -76,6 +76,10 @@ Layer::~Layer()
	}		}
	}		}

			void Layer::destroy() const {
			mFlinger->destroyLayer(this);
			}

	status_t Layer::setToken(const sp<UserClient>& userClient,		status_t Layer::setToken(const sp<UserClient>& userClient,
	SharedClient* sharedClient, int32_t token)		SharedClient* sharedClient, int32_t token)
	{		{
	@@ -123,22 +127,6 @@ sp<LayerBaseClient::Surface> Layer::createSurface() const
	return mSurface;		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,		status_t Layer::setBuffers( uint32_t w, uint32_t h,
	PixelFormat format, uint32_t flags)		PixelFormat format, uint32_t flags)
	{		{

services/surfaceflinger/Layer.h

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -78,7 +78,6 @@ public:
	virtual bool needsFiltering() const;		virtual bool needsFiltering() const;
	virtual bool isSecure() const { return mSecure; }		virtual bool isSecure() const { return mSecure; }
	virtual sp<Surface> createSurface() const;		virtual sp<Surface> createSurface() const;
	virtual status_t ditch();
	virtual void onRemoved();		virtual void onRemoved();
	virtual bool setBypass(bool enable);		virtual bool setBypass(bool enable);

	@@ -95,6 +94,7 @@ public:
	return mFreezeLock; }		return mFreezeLock; }

	protected:		protected:
			virtual void destroy() const;
	virtual void dump(String8& result, char* scratch, size_t size) const;		virtual void dump(String8& result, char* scratch, size_t size) const;

	private:		private:

services/surfaceflinger/LayerBase.cpp

+1 −4

Original line number	Original line	Diff line number	Diff line
	@@ -583,10 +583,7 @@ LayerBaseClient::Surface::~Surface()
	*/		*/

	// destroy client resources		// destroy client resources
	sp<LayerBaseClient> layer = getOwner();		mFlinger->destroySurface(mOwner);
	if (layer != 0) {
	mFlinger->destroySurface(layer);
	}
	}		}

	sp<LayerBaseClient> LayerBaseClient::Surface::getOwner() const {		sp<LayerBaseClient> LayerBaseClient::Surface::getOwner() const {

services/surfaceflinger/LayerBase.h

+2 −5

Original line number	Original line	Diff line number	Diff line
	@@ -196,10 +196,6 @@ public:
	*/		*/
	virtual bool isSecure() const { return false; }		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		/** called with the state lock when the surface is removed from the
	* current list */		* current list */
	virtual void onRemoved() { };		virtual void onRemoved() { };
	@@ -264,7 +260,8 @@ protected:
	volatile int32_t mInvalidate;		volatile int32_t mInvalidate;


	protected:		public:
			// called from class SurfaceFlinger
	virtual ~LayerBase();		virtual ~LayerBase();

	private:		private:

services/surfaceflinger/SurfaceFlinger.cpp

+53 −67

Original line number	Original line	Diff line number	Diff line
	@@ -358,6 +358,9 @@ bool SurfaceFlinger::threadLoop()
	{		{
	waitForEvent();		waitForEvent();

			// call Layer's destructor
			handleDestroyLayers();

	// check for transactions		// check for transactions
	if (UNLIKELY(mConsoleSignals)) {		if (UNLIKELY(mConsoleSignals)) {
	handleConsoleEvents();		handleConsoleEvents();
	@@ -467,13 +470,6 @@ void SurfaceFlinger::handleConsoleEvents()

	void SurfaceFlinger::handleTransaction(uint32_t transactionFlags)		void SurfaceFlinger::handleTransaction(uint32_t transactionFlags)
	{		{
	Vector< sp<LayerBase> > ditchedLayers;

	/*
	* Perform and commit the transaction
	*/

	{ // scope for the lock
	Mutex::Autolock _l(mStateLock);		Mutex::Autolock _l(mStateLock);
	const nsecs_t now = systemTime();		const nsecs_t now = systemTime();
	mDebugInTransaction = now;		mDebugInTransaction = now;
	@@ -482,32 +478,18 @@ void SurfaceFlinger::handleTransaction(uint32_t transactionFlags)
	// so we can call handleTransactionLocked() unconditionally.		// so we can call handleTransactionLocked() unconditionally.
	// We call getTransactionFlags(), which will also clear the flags,		// We call getTransactionFlags(), which will also clear the flags,
	// with mStateLock held to guarantee that mCurrentState won't change		// with mStateLock held to guarantee that mCurrentState won't change
	// until the transaction is commited.		// until the transaction is committed.

	const uint32_t mask = eTransactionNeeded \| eTraversalNeeded;		const uint32_t mask = eTransactionNeeded \| eTraversalNeeded;
	transactionFlags = getTransactionFlags(mask);		transactionFlags = getTransactionFlags(mask);
	handleTransactionLocked(transactionFlags, ditchedLayers);		handleTransactionLocked(transactionFlags);

	mLastTransactionTime = systemTime() - now;		mLastTransactionTime = systemTime() - now;
	mDebugInTransaction = 0;		mDebugInTransaction = 0;
	// here the transaction has been committed		// here the transaction has been committed
	}		}

	/*		void SurfaceFlinger::handleTransactionLocked(uint32_t 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();
	}
	}
	}

	void SurfaceFlinger::handleTransactionLocked(
	uint32_t transactionFlags, Vector< sp<LayerBase> >& ditchedLayers)
	{		{
	const LayerVector& currentLayers(mCurrentState.layersSortedByZ);		const LayerVector& currentLayers(mCurrentState.layersSortedByZ);
	const size_t count = currentLayers.size();		const size_t count = currentLayers.size();
	@@ -579,7 +561,6 @@ void SurfaceFlinger::handleTransactionLocked(
	const sp<LayerBase>& layer(previousLayers[i]);		const sp<LayerBase>& layer(previousLayers[i]);
	if (currentLayers.indexOf( layer ) < 0) {		if (currentLayers.indexOf( layer ) < 0) {
	// this layer is not visible anymore		// this layer is not visible anymore
	ditchedLayers.add(layer);
	mDirtyRegionRemovedLayer.orSelf(layer->visibleRegionScreen);		mDirtyRegionRemovedLayer.orSelf(layer->visibleRegionScreen);
	}		}
	}		}
	@@ -589,6 +570,31 @@ void SurfaceFlinger::handleTransactionLocked(
	commitTransaction();		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		sp<FreezeLock> SurfaceFlinger::getFreezeLock() const
	{		{
	return new FreezeLock(const_cast<SurfaceFlinger *>(this));		return new FreezeLock(const_cast<SurfaceFlinger *>(this));
	@@ -1329,38 +1335,18 @@ status_t SurfaceFlinger::removeSurface(const sp<Client>& client, SurfaceID sid)
	return err;		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		// called by ~ISurface() when all references are gone
			status_t err = NO_ERROR;
	class MessageDestroySurface : public MessageBase {		sp<LayerBaseClient> l(layer.promote());
	SurfaceFlinger* flinger;		if (l != NULL) {
	sp<LayerBaseClient> layer;		Mutex::Autolock _l(mStateLock);
	public:		err = removeLayer_l(l);
	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,		LOGE_IF(err<0 && err != NAME_NOT_FOUND,
	"error removing layer=%p (%s)", l.get(), strerror(-err));		"error removing layer=%p (%s)", l.get(), strerror(-err));
	return true;
	}		}
	};		return err;

	postMessageAsync( new MessageDestroySurface(this, layer) );
	return NO_ERROR;
	}		}

	status_t SurfaceFlinger::setClientState(		status_t SurfaceFlinger::setClientState(