4 files changed, 31 insertions, 20 deletions

diff --git a/include/surfaceflinger/Surface.h b/include/surfaceflinger/Surface.h
index 2df8ca3755..5532052864 100644
--- a/include/surfaceflinger/Surface.h
+++ b/include/surfaceflinger/Surface.h
@@ -233,7 +233,7 @@ private:
      *  private stuff...
      */
     void init();
-    status_t validate() const;
+    status_t validate(bool inCancelBuffer = false) const;
     sp<ISurface> getISurface() const;
 
     inline const GraphicBufferMapper& getBufferMapper() const { return mBufferMapper; }
diff --git a/libs/surfaceflinger_client/Surface.cpp b/libs/surfaceflinger_client/Surface.cpp
index aa0c2e8b93..e21bab7353 100644
--- a/libs/surfaceflinger_client/Surface.cpp
+++ b/libs/surfaceflinger_client/Surface.cpp
@@ -466,7 +466,7 @@ bool Surface::isValid() {
     return mInitCheck == NO_ERROR;
 }
 
-status_t Surface::validate() const
+status_t Surface::validate(bool inCancelBuffer) const
 {
     // check that we initialized ourself properly
     if (mInitCheck != NO_ERROR) {
@@ -476,15 +476,6 @@ status_t Surface::validate() const
 
     // verify the identity of this surface
     uint32_t identity = mSharedBufferClient->getIdentity();
-
-    // this is a bit of a (temporary) special case, identity==0 means that
-    // no operation are allowed from the client (eg: dequeue/queue), this
-    // is used with PUSH_BUFFER surfaces for instance
-    if (identity == 0) {
-        LOGE("[Surface] invalid operation (identity=%u)", mIdentity);
-        return INVALID_OPERATION;
-    }
-
     if (mIdentity != identity) {
         LOGE("[Surface] using an invalid surface, "
                 "identity=%u should be %d",
@@ -492,17 +483,19 @@ status_t Surface::validate() const
         CallStack stack;
         stack.update();
         stack.dump("Surface");
-        return NO_INIT;
+        return BAD_INDEX;
     }
 
     // check the surface didn't become invalid
     status_t err = mSharedBufferClient->getStatus();
     if (err != NO_ERROR) {
-        LOGE("surface (identity=%u) is invalid, err=%d (%s)",
-                mIdentity, err, strerror(-err));
-        CallStack stack;
-        stack.update();
-        stack.dump("Surface");
+        if (!inCancelBuffer) {
+            LOGE("surface (identity=%u) is invalid, err=%d (%s)",
+                    mIdentity, err, strerror(-err));
+            CallStack stack;
+            stack.update();
+            stack.dump("Surface");
+        }
         return err;
     }
 
@@ -633,12 +626,12 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer)
 
 int Surface::cancelBuffer(android_native_buffer_t* buffer)
 {
-    status_t err = validate();
+    status_t err = validate(true);
     switch (err) {
     case NO_ERROR:
         // no error, common case
         break;
-    case INVALID_OPERATION:
+    case BAD_INDEX:
         // legitimate errors here
         return err;
     default:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 77695d73e8..154b8224c3 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1109,8 +1109,12 @@ status_t SurfaceFlinger::removeLayer_l(const sp<LayerBase>& layerBase)
 
 status_t SurfaceFlinger::purgatorizeLayer_l(const sp<LayerBase>& layerBase)
 {
-    // remove the layer from the main list (through a transaction).
+    // First add the layer to the purgatory list, which makes sure it won't
+    // go away, then remove it from the main list (through a transaction).
     ssize_t err = removeLayer_l(layerBase);
+    if (err >= 0) {
+        mLayerPurgatory.add(layerBase);
+    }
 
     layerBase->onRemoved();
 
@@ -1359,6 +1363,19 @@ status_t SurfaceFlinger::destroySurface(const sp<LayerBaseClient>& layer)
              * to use the purgatory.
              */
             status_t err = flinger->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);
+                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;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 2591123da3..eabdc64594 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -375,6 +375,7 @@ private:
     volatile    int32_t                 mTransactionFlags;
     volatile    int32_t                 mTransactionCount;
                 Condition               mTransactionCV;
+                SortedVector< sp<LayerBase> > mLayerPurgatory;
                 bool                    mResizeTransationPending;
 
                 // protected by mStateLock (but we could use another lock)