Ensure onHandleDestroyed drops reference before releasing lock.

onHandleDestroyed copies the layer pointer and references it in
mLayersPendingRemoval, so that the final reference will be dropped
from the main thread. However the current implementation is flawed.
It is possible for the main thread to process mLayersPendingRemoval
after we release the lock, but BEFORE we finish executing the
~LayerCleaner destructor. This would mean the last reference would
again be LayerCleaner::mLayer and we would incorrectly run the d'tor
on the main thread. Scheduling priority boosts to the main thread
make this more likely than it may seem.

Test: Boots. Existing tests pass.
Change-Id: Ife247b530b67907b28b4f29f6766c2056fe49bfa

2 files changed, 3 insertions, 2 deletions

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 20e7f708ca..4537d23175 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3902,10 +3902,11 @@ void SurfaceFlinger::markLayerPendingRemovalLocked(const Mutex&, const sp<Layer>
     setTransactionFlags(eTransactionNeeded);
 }
 
-void SurfaceFlinger::onHandleDestroyed(const sp<Layer>& layer)
+void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer)
 {
     Mutex::Autolock lock(mStateLock);
     markLayerPendingRemovalLocked(mStateLock, layer);
+    layer.clear();
 }
 
 // ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index d3f0ecef16..b1bfb3a08d 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -601,7 +601,7 @@ private:
     // called when all clients have released all their references to
     // this layer meaning it is entirely safe to destroy all
     // resources associated to this layer.
-    void onHandleDestroyed(const sp<Layer>& layer);
+    void onHandleDestroyed(sp<Layer>& layer);
 
     // remove a layer from SurfaceFlinger immediately
     status_t removeLayer(const sp<Layer>& layer);