fix [2112575] stuck on DequeueCondition for a surface that doesn't exist anymore

this also fixes part of [2111536] Device is soft rebooted after ending the call through voice dialer

5 files changed, 65 insertions, 25 deletions

diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp
index ecb6b32..6275910 100644
--- a/libs/surfaceflinger/Layer.cpp
+++ b/libs/surfaceflinger/Layer.cpp
@@ -49,13 +49,12 @@ const char* const Layer::typeID = "Layer";
 
 Layer::Layer(SurfaceFlinger* flinger, DisplayID display, 
         const sp<Client>& c, int32_t i)
-    :   LayerBaseClient(flinger, display, c, i), lcblk(NULL),
+    :   LayerBaseClient(flinger, display, c, i),
         mSecure(false),
         mNeedsBlending(true)
 {
     // no OpenGL operation is possible here, since we might not be
     // in the OpenGL thread.
-    lcblk = new SharedBufferServer(c->ctrlblk, i, NUM_BUFFERS);
     mFrontBufferIndex = lcblk->getFrontBuffer();
 }
 
@@ -63,8 +62,14 @@ Layer::~Layer()
 {
     destroy();
     // the actual buffers will be destroyed here
-    delete lcblk;
+}
 
+// called with SurfaceFlinger::mStateLock as soon as the layer is entered
+// in the purgatory list
+void Layer::onRemoved()
+{
+    // wake up the condition
+    lcblk->setStatus(NO_INIT);
 }
 
 void Layer::destroy()
@@ -79,7 +84,9 @@ void Layer::destroy()
             eglDestroyImageKHR(dpy, mTextures[i].image);
             mTextures[i].image = EGL_NO_IMAGE_KHR;
         }
+        Mutex::Autolock _l(mLock);
         mBuffers[i].clear();
+        mWidth = mHeight = 0;
     }
     mSurface.clear();
 }
@@ -213,6 +220,16 @@ void Layer::onDraw(const Region& clip) const
 
 sp<SurfaceBuffer> Layer::requestBuffer(int index, int usage)
 {
+    sp<Buffer> buffer;
+
+    // this ensures our client doesn't go away while we're accessing
+    // the shared area.
+    sp<Client> ourClient(client.promote());
+    if (ourClient == 0) {
+        // oops, the client is already gone
+        return buffer;
+    }
+
     /*
      * This is called from the client's Surface::dequeue(). This can happen
      * at any time, especially while we're in the middle of using the
@@ -225,12 +242,21 @@ sp<SurfaceBuffer> Layer::requestBuffer(int index, int usage)
      */
     status_t err = lcblk->assertReallocate(index);
     LOGE_IF(err, "assertReallocate(%d) failed (%s)", index, strerror(-err));
+    if (err != NO_ERROR) {
+        // the surface may have died
+        return buffer;
+    }
+
+    uint32_t w, h;
+    { // scope for the lock
+        Mutex::Autolock _l(mLock);
+        w = mWidth;
+        h = mHeight;
+        buffer = mBuffers[index];
+        mBuffers[index].clear();
+    }
+
 
-    Mutex::Autolock _l(mLock);
-    uint32_t w = mWidth;
-    uint32_t h = mHeight;
-    
-    sp<Buffer>& buffer(mBuffers[index]);
     if (buffer->getStrongCount() == 1) {
         err = buffer->reallocate(w, h, mFormat, usage, mBufferFlags);
     } else {
@@ -253,8 +279,16 @@ sp<SurfaceBuffer> Layer::requestBuffer(int index, int usage)
     }
 
     if (err == NO_ERROR && buffer->handle != 0) {
-        // texture is now dirty...
-        mTextures[index].dirty = true;
+        Mutex::Autolock _l(mLock);
+        if (mWidth && mHeight) {
+            // and we have new buffer
+            mBuffers[index] = buffer;
+            // texture is now dirty...
+            mTextures[index].dirty = true;
+        } else {
+            // oops we got killed while we were allocating the buffer
+            buffer.clear();
+        }
     }
     return buffer;
 }
diff --git a/libs/surfaceflinger/Layer.h b/libs/surfaceflinger/Layer.h
index 3b4489e..2e8173d 100644
--- a/libs/surfaceflinger/Layer.h
+++ b/libs/surfaceflinger/Layer.h
@@ -51,10 +51,6 @@ public:
     static const char* const typeID;
     virtual char const* getTypeID() const { return typeID; }
     virtual uint32_t getTypeInfo() const { return typeInfo; }
-
-    
-    SharedBufferServer*     lcblk;
-
     
                  Layer(SurfaceFlinger* flinger, DisplayID display,
                          const sp<Client>& client, int32_t i);
@@ -88,6 +84,8 @@ private:
         return mBuffers[mFrontBufferIndex];
     }
  
+    virtual void onRemoved();
+
     void reloadTexture(const Region& dirty);
 
     sp<SurfaceBuffer> requestBuffer(int index, int usage);
diff --git a/libs/surfaceflinger/LayerBase.cpp b/libs/surfaceflinger/LayerBase.cpp
index 62e41b0..1f22488 100644
--- a/libs/surfaceflinger/LayerBase.cpp
+++ b/libs/surfaceflinger/LayerBase.cpp
@@ -642,9 +642,12 @@ int32_t LayerBaseClient::sIdentity = 0;
 
 LayerBaseClient::LayerBaseClient(SurfaceFlinger* flinger, DisplayID display,
         const sp<Client>& client, int32_t i)
-    : LayerBase(flinger, display), client(client),
+    : LayerBase(flinger, display), lcblk(NULL), client(client),
       mIndex(i), mIdentity(uint32_t(android_atomic_inc(&sIdentity)))
 {
+    lcblk = new SharedBufferServer(
+            client->ctrlblk, i, NUM_BUFFERS,
+            mIdentity);
 }
 
 void LayerBaseClient::onFirstRef()
@@ -652,8 +655,6 @@ void LayerBaseClient::onFirstRef()
     sp<Client> client(this->client.promote());
     if (client != 0) {
         client->bindLayer(this, mIndex);
-        // Initialize this layer's identity
-        client->ctrlblk->setIdentity(mIndex, mIdentity);
     }
 }
 
@@ -663,6 +664,7 @@ LayerBaseClient::~LayerBaseClient()
     if (client != 0) {
         client->free(mIndex);
     }
+    delete lcblk;
 }
 
 int32_t LayerBaseClient::serverIndex() const 
diff --git a/libs/surfaceflinger/LayerBase.h b/libs/surfaceflinger/LayerBase.h
index 78bb4bf..3a52240 100644
--- a/libs/surfaceflinger/LayerBase.h
+++ b/libs/surfaceflinger/LayerBase.h
@@ -292,13 +292,16 @@ public:
     virtual char const* getTypeID() const { return typeID; }
     virtual uint32_t getTypeInfo() const { return typeInfo; }
 
+    // lcblk is (almost) only accessed from the main SF thread, in the places
+    // where it's not, a reference to Client must be held
+    SharedBufferServer*     lcblk;
+
     LayerBaseClient(SurfaceFlinger* flinger, DisplayID display, 
             const sp<Client>& client, int32_t i);
     virtual ~LayerBaseClient();
     virtual void onFirstRef();
 
-    wp<Client>              client;
-//    SharedBufferServer*     lcblk;
+    const wp<Client>    client;
 
     inline  uint32_t    getIdentity() const { return mIdentity; }
     inline  int32_t     clientIndex() const { return mIndex; }
@@ -308,6 +311,7 @@ public:
             sp<Surface> getSurface();
     virtual sp<Surface> createSurface() const;
     
+    virtual void onRemoved() { }
 
     class Surface : public BnSurface 
     {
diff --git a/libs/surfaceflinger/SurfaceFlinger.cpp b/libs/surfaceflinger/SurfaceFlinger.cpp
index b368db6..8685f99 100644
--- a/libs/surfaceflinger/SurfaceFlinger.cpp
+++ b/libs/surfaceflinger/SurfaceFlinger.cpp
@@ -654,6 +654,7 @@ void SurfaceFlinger::handleTransactionLocked(
         // some layers might have been removed, so
         // we need to update the regions they're exposing.
         if (mLayersRemoved) {
+            mLayersRemoved = false;
             mVisibleRegionsDirty = true;
             const LayerVector& previousLayers(mDrawingState.layersSortedByZ);
             const size_t count = previousLayers.size();
@@ -1093,9 +1094,6 @@ status_t SurfaceFlinger::purgatorizeLayer_l(const sp<LayerBase>& layerBase)
 
 void SurfaceFlinger::free_resources_l()
 {
-    // Destroy layers that were removed
-    mLayersRemoved = false;
-    
     // free resources associated with disconnected clients
     Vector< sp<Client> >& disconnectedClients(mDisconnectedClients);
     const size_t count = disconnectedClients.size();
@@ -1321,11 +1319,15 @@ status_t SurfaceFlinger::removeSurface(SurfaceID index)
      * to wait for all client's references to go away first).
      */
 
+    status_t err = NAME_NOT_FOUND;
     Mutex::Autolock _l(mStateLock);
     sp<LayerBaseClient> layer = getLayerUser_l(index);
-    status_t err = purgatorizeLayer_l(layer);
-    if (err == NO_ERROR) {
-        setTransactionFlags(eTransactionNeeded);
+    if (layer != 0) {
+        err = purgatorizeLayer_l(layer);
+        if (err == NO_ERROR) {
+            layer->onRemoved();
+            setTransactionFlags(eTransactionNeeded);
+        }
     }
     return err;
 }