Fix a race that could cause GL commands to be executed from the wrong thread. (ca4d3602) · Commits · e / os / platform_frameworks_native

services/surfaceflinger/Layer.cpp

+4 −12

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

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

		status_t Layer::setToken(const sp<UserClient>& userClient,
		SharedClient* sharedClient, int32_t token)
		{
		@@ -147,18 +151,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)
		{

services/surfaceflinger/Layer.h

+1 −1

Original line number	Diff line number	Diff line
		@@ -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:

services/surfaceflinger/LayerBase.cpp

+1 −4

Original line number	Diff line number	Diff line
		@@ -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 {

services/surfaceflinger/LayerBase.h

+2 −5

Original line number	Diff line number	Diff line
		@@ -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:

services/surfaceflinger/SurfaceFlinger.cpp

+62 −82

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

		// call Layer's destructor
		handleDestroyLayers();

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

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

		/*
		* Perform and commit the transaction
		*/

		{ // scope for the lock
		Mutex::Autolock _l(mStateLock);
		const nsecs_t now = systemTime();
		mDebugInTransaction = now;
		@@ -495,11 +491,11 @@ void SurfaceFlinger::handleTransaction(uint32_t transactionFlags)
		// 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 commited.
		// until the transaction is committed.

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

		mLastTransactionTime = systemTime() - now;
		mDebugInTransaction = 0;
		@@ -507,22 +503,7 @@ void SurfaceFlinger::handleTransaction(uint32_t transactionFlags)
		// here the transaction has been committed
		}

		/*
		* 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)
		void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
		{
		const LayerVector& currentLayers(mCurrentState.layersSortedByZ);
		const size_t count = currentLayers.size();
		@@ -594,7 +575,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);
		}
		}
		@@ -604,6 +584,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));
		@@ -1377,51 +1382,26 @@ 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);
		status_t err = NO_ERROR;
		sp<LayerBaseClient> l(layer.promote());
		if (l != NULL) {
		Mutex::Autolock _l(mStateLock);
		err = 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);
		ssize_t idx = 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;
		return err;
		}

		status_t SurfaceFlinger::setClientState(