Commit 5e8e1f00 authored Feb 20, 2018 by Robert Carr

SurfaceFlinger: Prevent deadlock when destroying Client.

It's possible that we may destroy the Client while holding the
SurfaceFlinger lock even though the layers only hold a weak reference.
For example imagine a sp<Layer> with last reference in
SurfaceFlinger#mDrawingState. When we swap mCurrentState to mDrawingState
while holding the lock, we could thus trigger Layer::~Layer. This promotes
an sp to the Client (mClientRef) and removes the layer from it. At
this point another thread could drop the only other remaining reference
to the client. This means when we leave ~Layer, ~Client will be invoked.
~Client tries to again acquire the SurfaceFlinger lock leading to deadlock.
It seems painful to insist that sp<Client> can't be descoped while
holding the SF locked so I opted for pushing the messages from ~Client
to a handler.

Bug: 72838192
Test: Boots
Change-Id: I3fa2147f418caa0df1d86b3227eec8c56a0e5f7d

parent eb7d5676

services/surfaceflinger/Client.cpp

+11 −2

Original line number	Diff line number	Diff line
		@@ -47,12 +47,21 @@ Client::Client(const sp<SurfaceFlinger>& flinger, const sp<Layer>& parentLayer)

		Client::~Client()
		{
		// We need to post a message to remove our remaining layers rather than
		// do so directly by acquiring the SurfaceFlinger lock. If we were to
		// attempt to directly call the lock it becomes effectively impossible
		// to use sp<Client> while holding the SF lock as descoping it could
		// then trigger a dead-lock.

		const size_t count = mLayers.size();
		for (size_t i=0 ; i<count ; i++) {
		sp<Layer> l = mLayers.valueAt(i).promote();
		if (l != nullptr) {
		mFlinger->removeLayer(l);
		if (l == nullptr) {
		continue;
		}
		mFlinger->postMessageAsync(new LambdaMessage([flinger = mFlinger, l]() {
		flinger->removeLayer(l);
		}));
		}
		}