commit a228cc5efaa485d66855d475ded09a543a6fd076
author Mathias Agopian <mathias@google.com> Thu Apr 29 14:16:14 2010 -0700
committer Android (Google) Code Review <android-gerrit@google.com> Thu Apr 29 14:16:14 2010 -0700
tree 2411dfd9b660f0fd40e5086a2961434a6e14ca3c
parent b219882a214a079233710a39c7c4943035d4f6dd
parent 0a8cd0689a76dbca7405004caac4dde4c0250aea

Merge "fix a race condition in undoDequeue(), where 'tail' could be computed incorrectly." into kraken

include/private/surfaceflinger/SharedBufferStack.h

diff --git a/include/private/surfaceflinger/SharedBufferStack.h b/include/private/surfaceflinger/SharedBufferStack.h
index 6ace5bc..39ef3a1 100644
--- a/include/private/surfaceflinger/SharedBufferStack.h
+++ b/include/private/surfaceflinger/SharedBufferStack.h
@@ -167,6 +167,7 @@
     SharedBufferStack* const mSharedStack;
     const int mNumBuffers;
     const int mIdentity;
+    int32_t computeTail() const;
 
     friend struct Update;
     friend struct QueueUpdate;
@@ -259,8 +260,6 @@
     friend struct Condition;
     friend struct DequeueCondition;
     friend struct LockCondition;
-    
-    int32_t computeTail() const;
 
     struct QueueUpdate : public UpdateBase {
         inline QueueUpdate(SharedBufferBase* sbb);
@@ -288,6 +287,7 @@
     };
 
     int32_t tail;
+    int32_t undoDequeueTail;
     // statistics...
     nsecs_t mDequeueTime[NUM_BUFFER_MAX];
 };

libs/surfaceflinger_client/SharedBufferStack.cpp

diff --git a/libs/surfaceflinger_client/SharedBufferStack.cpp b/libs/surfaceflinger_client/SharedBufferStack.cpp
index f1aec8a..6745704 100644
--- a/libs/surfaceflinger_client/SharedBufferStack.cpp
+++ b/libs/surfaceflinger_client/SharedBufferStack.cpp
@@ -179,7 +179,7 @@
     char buffer[SIZE];
     String8 result;
     SharedBufferStack& stack( *mSharedStack );
-    int tail = (mNumBuffers + stack.head - stack.available + 1) % mNumBuffers;
+    int tail = computeTail();
     snprintf(buffer, SIZE, 
             "%s[ head=%2d, available=%2d, queued=%2d, tail=%2d ] "
             "reallocMask=%08x, inUse=%2d, identity=%d, status=%d\n",
@@ -189,6 +189,12 @@
     return result;
 }
 
+int32_t SharedBufferBase::computeTail() const
+{
+    SharedBufferStack& stack( *mSharedStack );
+    return (mNumBuffers + stack.head - stack.available + 1) % mNumBuffers;
+}
+
 // ============================================================================
 // conditions and updates
 // ============================================================================
@@ -297,32 +303,12 @@
 
 SharedBufferClient::SharedBufferClient(SharedClient* sharedClient,
         int surface, int num, int32_t identity)
-    : SharedBufferBase(sharedClient, surface, num, identity), tail(0)
+    : SharedBufferBase(sharedClient, surface, num, identity),
+      tail(0), undoDequeueTail(0)
 {
     tail = computeTail();
 }
 
-int32_t SharedBufferClient::computeTail() const
-{
-    SharedBufferStack& stack( *mSharedStack );
-    // we need to make sure we read available and head coherently,
-    // w.r.t RetireUpdate.
-    int32_t newTail;
-    int32_t avail;
-    int32_t head;
-    do {
-        avail = stack.available;
-        head = stack.head;
-    } while (stack.available != avail);
-    newTail = head - avail + 1;
-    if (newTail < 0) {
-        newTail += mNumBuffers;
-    } else if (newTail >= mNumBuffers) {
-        newTail -= mNumBuffers;
-    }
-    return newTail;
-}
-
 ssize_t SharedBufferClient::dequeue()
 {
     SharedBufferStack& stack( *mSharedStack );
@@ -350,6 +336,7 @@
 
     int dequeued = tail;
     tail = ((tail+1 >= mNumBuffers) ? 0 : tail+1);
+    undoDequeueTail = dequeued;
     LOGD_IF(DEBUG_ATOMICS, "dequeued=%d, tail=%d, %s",
             dequeued, tail, dump("").string());
 
@@ -363,7 +350,7 @@
     UndoDequeueUpdate update(this);
     status_t err = updateCondition( update );
     if (err == NO_ERROR) {
-        tail = computeTail();
+        tail = undoDequeueTail;
     }
     return err;
 }