RESTRICT AUTOMERGE: Fix removal of handle from map (6a8f8af9) · Commits · e / os / platform_frameworks_native

services/surfaceflinger/Client.cpp

+8 −17

Original line number	Original line	Diff line number	Diff line
	@@ -106,16 +106,16 @@ void Client::detachLayer(const Layer* layer)
	}		}
	}		}
	}		}
	sp<Layer> Client::getLayerUser(const sp<IBinder>& handle) const
			bool Client::isAttached(const sp<IBinder>& handle) const
	{		{
	Mutex::Autolock _l(mLock);		Mutex::Autolock _l(mLock);
	sp<Layer> lbc;		sp<Layer> lbc;
	wp<Layer> layer(mLayers.valueFor(handle));		wp<Layer> layer(mLayers.valueFor(handle));
	if (layer != 0) {		if (layer != 0) {
	lbc = layer.promote();		return true;
	ALOGE_IF(lbc==0, "getLayerUser(name=%p) is dead", handle.get());
	}		}
	return lbc;		return false;
	}		}

	status_t Client::onTransact(		status_t Client::onTransact(
	@@ -150,7 +150,8 @@ status_t Client::createSurface(
	sp<IBinder>* handle,		sp<IBinder>* handle,
	sp<IGraphicBufferProducer>* gbp) {		sp<IGraphicBufferProducer>* gbp) {
	bool parentDied;		bool parentDied;
	sp<Layer> parentLayer = getParentLayer(&parentDied);		sp<Layer> parentLayer;
			if (!parentHandle) parentLayer = getParentLayer(&parentDied);
	if (parentHandle == nullptr && parentDied) {		if (parentHandle == nullptr && parentDied) {
	return NAME_NOT_FOUND;		return NAME_NOT_FOUND;
	}		}
	@@ -163,21 +164,11 @@ status_t Client::destroySurface(const sp<IBinder>& handle) {
	}		}

	status_t Client::clearLayerFrameStats(const sp<IBinder>& handle) const {		status_t Client::clearLayerFrameStats(const sp<IBinder>& handle) const {
	sp<Layer> layer = getLayerUser(handle);		return mFlinger->clearLayerFrameStats(this, handle);
	if (layer == nullptr) {
	return NAME_NOT_FOUND;
	}
	layer->clearFrameStats();
	return NO_ERROR;
	}		}

	status_t Client::getLayerFrameStats(const sp<IBinder>& handle, FrameStats* outStats) const {		status_t Client::getLayerFrameStats(const sp<IBinder>& handle, FrameStats* outStats) const {
	sp<Layer> layer = getLayerUser(handle);		return mFlinger->getLayerFrameStats(this, handle, outStats);
	if (layer == nullptr) {
	return NAME_NOT_FOUND;
	}
	layer->getFrameStats(outStats);
	return NO_ERROR;
	}		}

	// ---------------------------------------------------------------------------		// ---------------------------------------------------------------------------

services/surfaceflinger/Client.h

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -49,7 +49,7 @@ public:

	void detachLayer(const Layer* layer);		void detachLayer(const Layer* layer);

	sp<Layer> getLayerUser(const sp<IBinder>& handle) const;		bool isAttached (const sp<IBinder>& handle) const;

	void updateParent(const sp<Layer>& parentLayer);		void updateParent(const sp<Layer>& parentLayer);

services/surfaceflinger/Layer.cpp

+2 −7

Original line number	Original line	Diff line number	Diff line
	@@ -178,14 +178,9 @@ Layer::~Layer() {
	void Layer::onLayerDisplayed(const sp<Fence>& /releaseFence/) {}		void Layer::onLayerDisplayed(const sp<Fence>& /releaseFence/) {}

	void Layer::onRemovedFromCurrentState() {		void Layer::onRemovedFromCurrentState() {
	if (!mPendingRemoval) {
	// the layer is removed from SF mCurrentState to mLayersPendingRemoval		// the layer is removed from SF mCurrentState to mLayersPendingRemoval
	mPendingRemoval = true;		mPendingRemoval = true;

	// remove from sf mapping
	mFlinger->removeLayerFromMap(this);
	}

	if (mCurrentState.zOrderRelativeOf != nullptr) {		if (mCurrentState.zOrderRelativeOf != nullptr) {
	sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();		sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
	if (strongRelative != nullptr) {		if (strongRelative != nullptr) {

services/surfaceflinger/SurfaceFlinger.cpp

+39 −16

Original line number	Original line	Diff line number	Diff line
	@@ -3187,16 +3187,10 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind
	return NO_ERROR;		return NO_ERROR;
	}		}

	status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {		status_t SurfaceFlinger::removeLayerFromMap(const wp<Layer>& layer) {
	Mutex::Autolock _l(mStateLock);
	return removeLayerLocked(mStateLock, layer, topLevelOnly);
	}

	status_t SurfaceFlinger::removeLayerFromMap(Layer* layer) {
	auto it = mLayersByLocalBinderToken.begin();		auto it = mLayersByLocalBinderToken.begin();
	while (it != mLayersByLocalBinderToken.end()) {		while (it != mLayersByLocalBinderToken.end()) {
	auto strongRef = it->second.promote();		if (it->second == layer) {
	if (strongRef != nullptr && strongRef.get() == layer) {
	it = mLayersByLocalBinderToken.erase(it);		it = mLayersByLocalBinderToken.erase(it);
	break;		break;
	} else {		} else {
	@@ -3210,6 +3204,11 @@ status_t SurfaceFlinger::removeLayerFromMap(Layer* layer) {
	return NO_ERROR;		return NO_ERROR;
	}		}

			status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {
			Mutex::Autolock _l(mStateLock);
			return removeLayerLocked(mStateLock, layer, topLevelOnly);
			}

	status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,		status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,
	bool topLevelOnly) {		bool topLevelOnly) {
	if (layer->isPendingRemoval()) {		if (layer->isPendingRemoval()) {
	@@ -3455,8 +3454,8 @@ uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState
	const layer_state_t& s = composerState.state;		const layer_state_t& s = composerState.state;
	sp<Client> client(static_cast<Client*>(composerState.client.get()));		sp<Client> client(static_cast<Client*>(composerState.client.get()));

	sp<Layer> layer(client->getLayerUser(s.surface));		sp<Layer> layer = fromHandle(s.surface);
	if (layer == nullptr) {		if (layer == nullptr \|\| !(client->isAttached(s.surface))) {
	return 0;		return 0;
	}		}

	@@ -3631,8 +3630,8 @@ void SurfaceFlinger::setDestroyStateLocked(const ComposerState& composerState) {
	const layer_state_t& state = composerState.state;		const layer_state_t& state = composerState.state;
	sp<Client> client(static_cast<Client*>(composerState.client.get()));		sp<Client> client(static_cast<Client*>(composerState.client.get()));

	sp<Layer> layer(client->getLayerUser(state.surface));		sp<Layer> layer = fromHandle(state.surface);
	if (layer == nullptr) {		if (layer == nullptr \|\| !(client->isAttached(state.surface))) {
	return;		return;
	}		}

	@@ -3767,14 +3766,35 @@ status_t SurfaceFlinger::createColorLayer(const sp<Client>& client,
	return NO_ERROR;		return NO_ERROR;
	}		}

			status_t SurfaceFlinger::clearLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle) {
			Mutex::Autolock _l(mStateLock);
			sp<Layer> layer = fromHandle(handle);
			if (layer == nullptr \|\| !(client->isAttached(handle))) {
			return NAME_NOT_FOUND;
			}
			layer->clearFrameStats();
			return NO_ERROR;
			}

			status_t SurfaceFlinger::getLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle, FrameStats* outStats) {
			Mutex::Autolock _l(mStateLock);
			sp<Layer> layer = fromHandle(handle);
			if (layer == nullptr \|\| !(client->isAttached(handle))) {
			return NAME_NOT_FOUND;
			}
			layer->getFrameStats(outStats);
			return NO_ERROR;
			}

	status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)		status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)
	{		{
			Mutex::Autolock _l(mStateLock);
	// called by a client when it wants to remove a Layer		// called by a client when it wants to remove a Layer
	status_t err = NO_ERROR;		status_t err = NO_ERROR;
	sp<Layer> l(client->getLayerUser(handle));		sp<Layer> l = fromHandle(handle);
	if (l != nullptr) {		if (l != nullptr \|\| client->isAttached(handle)) {
	mInterceptor->saveSurfaceDeletion(l);		mInterceptor->saveSurfaceDeletion(l);
	err = removeLayer(l);		err = removeLayerLocked(mStateLock, l);
	ALOGE_IF(err<0 && err != NAME_NOT_FOUND,		ALOGE_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));
	}		}
	@@ -3783,15 +3803,18 @@ status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBind

	status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)		status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
	{		{
			Mutex::Autolock _l(mStateLock);
	// called by ~LayerCleaner() when all references to the IBinder (handle)		// called by ~LayerCleaner() when all references to the IBinder (handle)
	// are gone		// are gone
	sp<Layer> l = layer.promote();		sp<Layer> l = layer.promote();
	if (l == nullptr) {		if (l == nullptr) {
			removeLayerFromMap(layer);
	// The layer has already been removed, carry on		// The layer has already been removed, carry on
	return NO_ERROR;		return NO_ERROR;
	}		}
			removeLayerFromMap(layer);
	// If we have a parent, then we can continue to live as long as it does.		// If we have a parent, then we can continue to live as long as it does.
	return removeLayer(l, true);		return removeLayerLocked(mStateLock, l, true);
	}		}

	// ---------------------------------------------------------------------------		// ---------------------------------------------------------------------------

services/surfaceflinger/SurfaceFlinger.h

+7 −3

Original line number	Original line	Diff line number	Diff line
	@@ -347,6 +347,10 @@ public:

	int getPrimaryDisplayOrientation() const { return mPrimaryDisplayOrientation; }		int getPrimaryDisplayOrientation() const { return mPrimaryDisplayOrientation; }

			status_t clearLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle);

			status_t getLayerFrameStats(const sp<const Client>& client, const sp<IBinder>& handle, FrameStats* outStats);

	sp<Layer> fromHandle(const sp<IBinder>& handle) REQUIRES(mStateLock);		sp<Layer> fromHandle(const sp<IBinder>& handle) REQUIRES(mStateLock);

	private:		private:
	@@ -523,9 +527,9 @@ private:
	uint32_t setTransactionFlags(uint32_t flags, VSyncModulator::TransactionStart transactionStart);		uint32_t setTransactionFlags(uint32_t flags, VSyncModulator::TransactionStart transactionStart);
	void commitTransaction();		void commitTransaction();
	bool containsAnyInvalidClientState(const Vector<ComposerState>& states);		bool containsAnyInvalidClientState(const Vector<ComposerState>& states);
	uint32_t setClientStateLocked(const ComposerState& composerState);		uint32_t setClientStateLocked(const ComposerState& composerState) REQUIRES(mStateLock);
	uint32_t setDisplayStateLocked(const DisplayState& s);		uint32_t setDisplayStateLocked(const DisplayState& s);
	void setDestroyStateLocked(const ComposerState& composerState);		void setDestroyStateLocked(const ComposerState& composerState) REQUIRES(mStateLock);

	/* ------------------------------------------------------------------------		/* ------------------------------------------------------------------------
	* Layer management		* Layer management
	@@ -560,7 +564,7 @@ private:
	status_t removeLayerLocked(const Mutex&, const sp<Layer>& layer, bool topLevelOnly = false);		status_t removeLayerLocked(const Mutex&, const sp<Layer>& layer, bool topLevelOnly = false);

	// remove layer from mapping		// remove layer from mapping
	status_t removeLayerFromMap(Layer* layer);		status_t removeLayerFromMap(const wp<Layer>& layer);

	// add a layer to SurfaceFlinger		// add a layer to SurfaceFlinger
	status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,		status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,