fix [3361121] hang in glClear() - device unresponsive, OTA fails (DO NOT MERGE)

Generally we never want to lock a buffer for write access if it is at
the "head" on the surfaceflinger side. The only exception (1) is when
the buffer is not currently in use AND there is at least one queued
buffer -- in which case, SurfaceFlinger will never use said buffer
anymore, because on the next composition around, it will be able to
retire the first queued buffer.

The logic above relies on SurfaceFlinger always retiring
and locking a buffer before composition -- unfortunately this
didn't happen during a screenshot.

This could leave us in a situation where a buffer is locked by the
application for write, and used by SurfaceFlinger for texturing,
causing a hang.

Here, we fix this issue by never assuming the exception (1), it was
intended as an optimization allowing ANativeWindow::lockBuffer() to
return sooner and was justified when most of SF composition was
done in software. The actual buffer locking is now ensured by
gralloc. We could have handled screenshots in a similar way to
a regular composition, but it could have caused glitches on screen,
essentially, taking a screenshot could cause to skip a frame.

now that we removed the notion of a "inUse" buffer in surfaceflinger
a lot of code can be simplified / removed.

noteworthy, the whole concept of "unlockClient" wrt. "compositionComplete"
is also gone.

6 files changed, 0 insertions, 43 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 3a8690e..c9dcef3 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -682,22 +682,6 @@ void Layer::unlockPageFlip(
     }
 }
 
-void Layer::finishPageFlip()
-{
-    ClientRef::Access sharedClient(mUserClientRef);
-    SharedBufferServer* lcblk(sharedClient.get());
-    if (lcblk) {
-        int buf = mBufferManager.getActiveBufferIndex();
-        if (buf >= 0) {
-            status_t err = lcblk->unlock( buf );
-            LOGE_IF(err!=NO_ERROR,
-                    "layer %p, buffer=%d wasn't locked!",
-                    this, buf);
-        }
-    }
-}
-
-
 void Layer::dump(String8& result, char* buffer, size_t SIZE) const
 {
     LayerBaseClient::dump(result, buffer, SIZE);
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index cb62558..7bb207a 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -73,7 +73,6 @@ public:
     virtual uint32_t doTransaction(uint32_t transactionFlags);
     virtual void lockPageFlip(bool& recomputeVisibleRegions);
     virtual void unlockPageFlip(const Transform& planeTransform, Region& outDirtyRegion);
-    virtual void finishPageFlip();
     virtual bool needsBlending() const      { return mNeedsBlending; }
     virtual bool needsDithering() const     { return mNeedsDithering; }
     virtual bool needsFiltering() const;
diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp
index 49b7c06..916d420 100644
--- a/services/surfaceflinger/LayerBase.cpp
+++ b/services/surfaceflinger/LayerBase.cpp
@@ -273,10 +273,6 @@ void LayerBase::unlockPageFlip(
     }
 }
 
-void LayerBase::finishPageFlip()
-{
-}
-
 void LayerBase::invalidate()
 {
     if ((android_atomic_or(1, &mInvalidate)&1) == 0) {
diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h
index aea5754..1470729 100644
--- a/services/surfaceflinger/LayerBase.h
+++ b/services/surfaceflinger/LayerBase.h
@@ -174,11 +174,6 @@ public:
     virtual void unlockPageFlip(const Transform& planeTransform, Region& outDirtyRegion);
     
     /**
-     * finishPageFlip - called after all surfaces have drawn.
-     */
-    virtual void finishPageFlip();
-    
-    /**
      * needsBlending - true if this surface needs blending
      */
     virtual bool needsBlending() const  { return false; }
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3f07d39..4876946 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -396,10 +396,6 @@ bool SurfaceFlinger::threadLoop()
         logger.log(GraphicLog::SF_COMPOSITION_COMPLETE, index);
         hw.compositionComplete();
 
-        // release the clients before we flip ('cause flip might block)
-        logger.log(GraphicLog::SF_UNLOCK_CLIENTS, index);
-        unlockClients();
-
         logger.log(GraphicLog::SF_SWAP_BUFFERS, index);
         postFramebuffer();
 
@@ -407,7 +403,6 @@ bool SurfaceFlinger::threadLoop()
     } else {
         // pretend we did the post
         hw.compositionComplete();
-        unlockClients();
         usleep(16667); // 60 fps period
     }
     return true;
@@ -895,17 +890,6 @@ void SurfaceFlinger::composeSurfaces(const Region& dirty)
     }
 }
 
-void SurfaceFlinger::unlockClients()
-{
-    const LayerVector& drawingLayers(mDrawingState.layersSortedByZ);
-    const size_t count = drawingLayers.size();
-    sp<LayerBase> const* const layers = drawingLayers.array();
-    for (size_t i=0 ; i<count ; ++i) {
-        const sp<LayerBase>& layer = layers[i];
-        layer->finishPageFlip();
-    }
-}
-
 void SurfaceFlinger::debugFlashRegions()
 {
     const DisplayHardware& hw(graphicPlane(0).displayHardware());
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 714a1b7..a023347 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -310,7 +310,6 @@ private:
             bool        handleBypassLayer();
             void        postFramebuffer();
             void        composeSurfaces(const Region& dirty);
-            void        unlockClients();
 
 
             ssize_t     addClientLayer(const sp<Client>& client,