SurfaceFlinger: fix layer removal race condition

Layer::lockPageFlip() and layer::onRemove() could be called on
different threads and race such that lockPageFlip() successfully
called mSurfaceTexture->updateTexImage() but then gets NULL back from
mSurfaceTexture->getCurrentBuffer(), leading to a crash.

This change moves Layer::onRemove() calls to
SurfaceFlinger::commitTransaction() so they happen after the Layer is
done being drawn from and only happen on the main surfaceflinger
thread.

Change-Id: I4b550caadff4cc1878d7c3bca6129193fb0c713e

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index f38e948..24bd2a6 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -710,6 +710,14 @@ void SurfaceFlinger::computeVisibleRegions(
 
 void SurfaceFlinger::commitTransaction()
 {
+    if (!mLayersPendingRemoval.isEmpty()) {
+        // Notify removed layers now that they can't be drawn from
+        for (size_t i = 0; i < mLayersPendingRemoval.size(); i++) {
+            mLayersPendingRemoval[i]->onRemoved();
+        }
+        mLayersPendingRemoval.clear();
+    }
+
     mDrawingState = mCurrentState;
     mTransationPending = false;
     mTransactionCV.broadcast();
@@ -1162,7 +1170,7 @@ status_t SurfaceFlinger::purgatorizeLayer_l(const sp<LayerBase>& layerBase)
         mLayerPurgatory.add(layerBase);
     }
 
-    layerBase->onRemoved();
+    mLayersPendingRemoval.push(layerBase);
 
     // it's possible that we don't find a layer, because it might
     // have been destroyed already -- this is not technically an error

diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 17028db..17b80a6 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -345,6 +345,7 @@ private:
                 Condition               mTransactionCV;
                 SortedVector< sp<LayerBase> > mLayerPurgatory;
                 bool                    mTransationPending;
+                Vector< sp<LayerBase> > mLayersPendingRemoval;
 
                 // protected by mStateLock (but we could use another lock)
                 GraphicPlane                mGraphicPlanes[1];