am 9f2a91bc: Merge "fix [2873058] Surface::dequeueBuffer blocks on last buffer, i.e. cannot dequeue all allocated buffers at once." into gingerbread

Merge commit '9f2a91bc2b01c2cc60f73a0bae673225fdddec09' into gingerbread-plus-aosp

* commit '9f2a91bc2b01c2cc60f73a0bae673225fdddec09':
  fix [2873058] Surface::dequeueBuffer blocks on last buffer, i.e. cannot dequeue all allocated buffers at once.

include/private/surfaceflinger/SharedBufferStack.h

@@ -273,7 +273,6 @@ public:
     void setStatus(status_t status);
     status_t reallocateAll();
     status_t reallocateAllExcept(int buffer);
-    status_t assertReallocate(int buffer);
     int32_t getQueuedCount() const;
     Region getDirtyRegion(int buffer) const;
 
@@ -356,13 +355,6 @@ private:
         inline StatusUpdate(SharedBufferBase* sbb, status_t status);
         inline ssize_t operator()();
     };
-
-    struct ReallocateCondition : public ConditionBase {
-        int buf;
-        inline ReallocateCondition(SharedBufferBase* sbb, int buf);
-        inline bool operator()() const;
-        inline const char* name() const { return "ReallocateCondition"; }
-    };
 };
 
 // ===========================================================================

libs/surfaceflinger_client/SharedBufferStack.cpp

@@ -243,21 +243,6 @@ bool SharedBufferClient::LockCondition::operator()() const {
             (stack.queued > 0 && stack.inUse != buf));
 }
 
-SharedBufferServer::ReallocateCondition::ReallocateCondition(
-        SharedBufferBase* sbb, int buf) : ConditionBase(sbb), buf(buf) { 
-}
-bool SharedBufferServer::ReallocateCondition::operator()() const {
-    int32_t head = stack.head;
-    if (uint32_t(head) >= SharedBufferStack::NUM_BUFFER_MAX) {
-        // if stack.head is messed up, we cannot allow the server to
-        // crash (since stack.head is mapped on the client side)
-        stack.status = BAD_VALUE;
-        return false;
-    }
-    // TODO: we should also check that buf has been dequeued
-    return (buf != stack.index[head]);
-}
-
 // ----------------------------------------------------------------------------
 
 SharedBufferClient::QueueUpdate::QueueUpdate(SharedBufferBase* sbb)
@@ -558,21 +543,6 @@ int32_t SharedBufferServer::getQueuedCount() const
     return stack.queued;
 }
 
-status_t SharedBufferServer::assertReallocate(int buf)
-{
-    /*
-     * NOTE: it's safe to hold mLock for read while waiting for
-     * the ReallocateCondition because that condition is not updated
-     * by the thread that holds mLock for write.
-     */
-    RWLock::AutoRLock _l(mLock);
-
-    // TODO: need to validate "buf"
-    ReallocateCondition condition(this, buf);
-    status_t err = waitForCondition(condition);
-    return err;
-}
-
 Region SharedBufferServer::getDirtyRegion(int buf) const
 {
     SharedBufferStack& stack( *mSharedStack );

services/surfaceflinger/Layer.cpp

@@ -294,19 +294,9 @@ sp<GraphicBuffer> Layer::requestBuffer(int index,
      * 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
      * buffer 'index' as our front buffer.
-     *
-     * Make sure the buffer we're resizing is not the front buffer and has been
-     * dequeued. Once this condition is asserted, we are guaranteed that this
-     * buffer cannot become the front buffer under our feet, since we're called
-     * from Surface::dequeue()
      */
-    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;
-    }
 
+    status_t err = NO_ERROR;
     uint32_t w, h, f;
     { // scope for the lock
         Mutex::Autolock _l(mLock);
@@ -319,23 +309,17 @@ sp<GraphicBuffer> Layer::requestBuffer(int index,
         w = reqWidth  ? reqWidth  : mWidth;
         h = reqHeight ? reqHeight : mHeight;
         f = reqFormat ? reqFormat : mFormat;
-        buffer = mBufferManager.detachBuffer(index);
         if (fixedSizeChanged || formatChanged) {
             lcblk->reallocateAllExcept(index);
         }
     }
 
+    // here we have to reallocate a new buffer because the buffer could be
+    // used as the front buffer, or by a client in our process
+    // (eg: status bar), and we can't release the handle under its feet.
     const uint32_t effectiveUsage = getEffectiveUsage(usage);
-    if (buffer!=0 && buffer->getStrongCount() == 1) {
+    buffer = new GraphicBuffer(w, h, f, effectiveUsage);
-        err = buffer->reallocate(w, h, f, effectiveUsage);
+    err = buffer->initCheck();
-    } else {
-        // here we have to reallocate a new buffer because we could have a
-        // client in our process with a reference to it (eg: status bar),
-        // and we can't release the handle under its feet.
-        buffer.clear();
-        buffer = new GraphicBuffer(w, h, f, effectiveUsage);
-        err = buffer->initCheck();
-    }
 
     if (err || buffer->handle == 0) {
         LOGE_IF(err || buffer->handle == 0,

services/surfaceflinger/tests/surface/Android.mk

@@ -0,0 +1,18 @@
+LOCAL_PATH:= $(call my-dir)
+include $(CLEAR_VARS)
+
+LOCAL_SRC_FILES:= \
+	surface.cpp
+
+LOCAL_SHARED_LIBRARIES := \
+	libcutils \
+	libutils \
+	libbinder \
+    libui \
+    libsurfaceflinger_client
+
+LOCAL_MODULE:= test-surface
+
+LOCAL_MODULE_TAGS := tests
+
+include $(BUILD_EXECUTABLE)

services/surfaceflinger/tests/surface/surface.cpp

@@ -0,0 +1,54 @@
+#include <cutils/memory.h>
+
+#include <utils/Log.h>
+
+#include <binder/IPCThreadState.h>
+#include <binder/ProcessState.h>
+#include <binder/IServiceManager.h>
+
+#include <surfaceflinger/Surface.h>
+#include <surfaceflinger/ISurface.h>
+#include <surfaceflinger/SurfaceComposerClient.h>
+
+#include <ui/Overlay.h>
+
+using namespace android;
+
+int main(int argc, char** argv)
+{
+    // set up the thread-pool
+    sp<ProcessState> proc(ProcessState::self());
+    ProcessState::self()->startThreadPool();
+
+    // create a client to surfaceflinger
+    sp<SurfaceComposerClient> client = new SurfaceComposerClient();
+    
+    // create pushbuffer surface
+    sp<SurfaceControl> surfaceControl = client->createSurface(
+            getpid(), 0, 160, 240, PIXEL_FORMAT_RGB_565);
+    client->openTransaction();
+    surfaceControl->setLayer(100000);
+    client->closeTransaction();
+
+    // pretend it went cross-process
+    Parcel parcel;
+    SurfaceControl::writeSurfaceToParcel(surfaceControl, &parcel);
+    parcel.setDataPosition(0);
+    sp<Surface> surface = Surface::readFromParcel(parcel);
+    ANativeWindow* window = surface.get();
+
+    printf("window=%p\n", window);
+
+    int err = native_window_set_buffer_count(window, 8);
+    android_native_buffer_t* buffer;
+
+    for (int i=0 ; i<8 ; i++) {
+        window->dequeueBuffer(window, &buffer);
+        printf("buffer %d: %p\n", i, buffer);
+    }
+
+    printf("test complete. CTRL+C to finish.\n");
+
+    IPCThreadState::self()->joinThreadPool();
+    return 0;
+}