Merge "Ensure that /data/anr/traces.txt is world-writable"
diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h
index a808216..57b9f8a 100644
--- a/include/gui/BufferQueue.h
+++ b/include/gui/BufferQueue.h
@@ -42,6 +42,7 @@
enum { NUM_BUFFER_SLOTS = 32 };
enum { NO_CONNECTED_API = 0 };
enum { INVALID_BUFFER_SLOT = -1 };
+ enum { STALE_BUFFER_SLOT = 1 };
// ConsumerListener is the interface through which the BufferQueue notifies
// the consumer of events that the consumer may wish to react to. Because
@@ -297,7 +298,8 @@
mTimestamp(0),
mFrameNumber(0),
mFence(EGL_NO_SYNC_KHR),
- mAcquireCalled(false) {
+ mAcquireCalled(false),
+ mNeedsCleanupOnRelease(false) {
mCrop.makeInvalid();
}
@@ -376,6 +378,9 @@
// Indicates whether this buffer has been seen by a consumer yet
bool mAcquireCalled;
+
+ // Indicates whether this buffer needs to be cleaned up by consumer
+ bool mNeedsCleanupOnRelease;
};
// mSlots is the array of buffer slots that must be mirrored on the client
diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h
index 496cfba..3f5e4df 100644
--- a/include/gui/SurfaceTexture.h
+++ b/include/gui/SurfaceTexture.h
@@ -185,7 +185,6 @@
status_t setConsumerUsageBits(uint32_t usage);
status_t setTransformHint(uint32_t hint);
virtual status_t setSynchronousMode(bool enabled);
- virtual status_t setBufferCount(int bufferCount);
virtual status_t connect(int api,
uint32_t* outWidth, uint32_t* outHeight, uint32_t* outTransform);
diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index 1762a4a..2d042c8 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -163,48 +163,58 @@
status_t BufferQueue::setBufferCount(int bufferCount) {
ST_LOGV("setBufferCount: count=%d", bufferCount);
- Mutex::Autolock lock(mMutex);
- if (mAbandoned) {
- ST_LOGE("setBufferCount: SurfaceTexture has been abandoned!");
- return NO_INIT;
- }
- if (bufferCount > NUM_BUFFER_SLOTS) {
- ST_LOGE("setBufferCount: bufferCount larger than slots available");
- return BAD_VALUE;
- }
+ sp<ConsumerListener> listener;
+ {
+ Mutex::Autolock lock(mMutex);
- // Error out if the user has dequeued buffers
- for (int i=0 ; i<mBufferCount ; i++) {
- if (mSlots[i].mBufferState == BufferSlot::DEQUEUED) {
- ST_LOGE("setBufferCount: client owns some buffers");
- return -EINVAL;
+ if (mAbandoned) {
+ ST_LOGE("setBufferCount: SurfaceTexture has been abandoned!");
+ return NO_INIT;
}
+ if (bufferCount > NUM_BUFFER_SLOTS) {
+ ST_LOGE("setBufferCount: bufferCount larger than slots available");
+ return BAD_VALUE;
+ }
+
+ // Error out if the user has dequeued buffers
+ for (int i=0 ; i<mBufferCount ; i++) {
+ if (mSlots[i].mBufferState == BufferSlot::DEQUEUED) {
+ ST_LOGE("setBufferCount: client owns some buffers");
+ return -EINVAL;
+ }
+ }
+
+ const int minBufferSlots = mSynchronousMode ?
+ MIN_SYNC_BUFFER_SLOTS : MIN_ASYNC_BUFFER_SLOTS;
+ if (bufferCount == 0) {
+ mClientBufferCount = 0;
+ bufferCount = (mServerBufferCount >= minBufferSlots) ?
+ mServerBufferCount : minBufferSlots;
+ return setBufferCountServerLocked(bufferCount);
+ }
+
+ if (bufferCount < minBufferSlots) {
+ ST_LOGE("setBufferCount: requested buffer count (%d) is less than "
+ "minimum (%d)", bufferCount, minBufferSlots);
+ return BAD_VALUE;
+ }
+
+ // here we're guaranteed that the client doesn't have dequeued buffers
+ // and will release all of its buffer references.
+ freeAllBuffersLocked();
+ mBufferCount = bufferCount;
+ mClientBufferCount = bufferCount;
+ mBufferHasBeenQueued = false;
+ mQueue.clear();
+ mDequeueCondition.broadcast();
+ listener = mConsumerListener;
+ } // scope for lock
+
+ if (listener != NULL) {
+ listener->onBuffersReleased();
}
- const int minBufferSlots = mSynchronousMode ?
- MIN_SYNC_BUFFER_SLOTS : MIN_ASYNC_BUFFER_SLOTS;
- if (bufferCount == 0) {
- mClientBufferCount = 0;
- bufferCount = (mServerBufferCount >= minBufferSlots) ?
- mServerBufferCount : minBufferSlots;
- return setBufferCountServerLocked(bufferCount);
- }
-
- if (bufferCount < minBufferSlots) {
- ST_LOGE("setBufferCount: requested buffer count (%d) is less than "
- "minimum (%d)", bufferCount, minBufferSlots);
- return BAD_VALUE;
- }
-
- // here we're guaranteed that the client doesn't have dequeued buffers
- // and will release all of its buffer references.
- freeAllBuffersLocked();
- mBufferCount = bufferCount;
- mClientBufferCount = bufferCount;
- mBufferHasBeenQueued = false;
- mQueue.clear();
- mDequeueCondition.broadcast();
return OK;
}
@@ -780,6 +790,9 @@
void BufferQueue::freeBufferLocked(int i) {
mSlots[i].mGraphicBuffer = 0;
+ if (mSlots[i].mBufferState == BufferSlot::ACQUIRED) {
+ mSlots[i].mNeedsCleanupOnRelease = true;
+ }
mSlots[i].mBufferState = BufferSlot::FREE;
mSlots[i].mFrameNumber = 0;
mSlots[i].mAcquireCalled = false;
@@ -853,15 +866,19 @@
mSlots[buf].mEglDisplay = display;
mSlots[buf].mFence = fence;
- // The current buffer becomes FREE if it was still in the queued
- // state. If it has already been given to the client
- // (synchronous mode), then it stays in DEQUEUED state.
- if (mSlots[buf].mBufferState == BufferSlot::QUEUED
- || mSlots[buf].mBufferState == BufferSlot::ACQUIRED) {
+ // The buffer can now only be released if its in the acquired state
+ if (mSlots[buf].mBufferState == BufferSlot::ACQUIRED) {
mSlots[buf].mBufferState = BufferSlot::FREE;
+ } else if (mSlots[buf].mNeedsCleanupOnRelease) {
+ ST_LOGV("releasing a stale buf %d its state was %d", buf, mSlots[buf].mBufferState);
+ mSlots[buf].mNeedsCleanupOnRelease = false;
+ return STALE_BUFFER_SLOT;
+ } else {
+ ST_LOGE("attempted to release buf %d but its state was %d", buf, mSlots[buf].mBufferState);
+ return -EINVAL;
}
- mDequeueCondition.broadcast();
+ mDequeueCondition.broadcast();
return OK;
}
diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp
index 18c86fa..dd96f84 100644
--- a/libs/gui/SurfaceTexture.cpp
+++ b/libs/gui/SurfaceTexture.cpp
@@ -176,6 +176,8 @@
ST_LOGV("updateTexImage");
Mutex::Autolock lock(mMutex);
+ status_t err = NO_ERROR;
+
if (mAbandoned) {
ST_LOGE("updateTexImage: SurfaceTexture is abandoned!");
return NO_INIT;
@@ -269,11 +271,17 @@
mCurrentTextureBuf != NULL ? mCurrentTextureBuf->handle : 0,
buf, item.mGraphicBuffer != NULL ? item.mGraphicBuffer->handle : 0);
- // Release the old buffer
+ // release old buffer
if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
- mBufferQueue->releaseBuffer(mCurrentTexture, dpy,
- mEGLSlots[mCurrentTexture].mFence);
+ status_t status = mBufferQueue->releaseBuffer(mCurrentTexture, dpy, mEGLSlots[mCurrentTexture].mFence);
+
mEGLSlots[mCurrentTexture].mFence = EGL_NO_SYNC_KHR;
+ if (status == BufferQueue::STALE_BUFFER_SLOT) {
+ freeBufferLocked(mCurrentTexture);
+ } else if (status != OK) {
+ ST_LOGE("updateTexImage: released invalid buffer");
+ err = status;
+ }
}
// Update the SurfaceTexture state.
@@ -289,7 +297,7 @@
glBindTexture(mTexTarget, mTexName);
}
- return OK;
+ return err;
}
status_t SurfaceTexture::detachFromContext() {
@@ -630,6 +638,9 @@
void SurfaceTexture::freeBufferLocked(int slotIndex) {
ST_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);
mEGLSlots[slotIndex].mGraphicBuffer = 0;
+ if (slotIndex == mCurrentTexture) {
+ mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT;
+ }
if (mEGLSlots[slotIndex].mEglImage != EGL_NO_IMAGE_KHR) {
EGLImageKHR img = mEGLSlots[slotIndex].mEglImage;
if (img != EGL_NO_IMAGE_KHR) {
@@ -693,12 +704,6 @@
}
// Used for refactoring, should not be in final interface
-status_t SurfaceTexture::setBufferCount(int bufferCount) {
- Mutex::Autolock lock(mMutex);
- return mBufferQueue->setBufferCount(bufferCount);
-}
-
-// Used for refactoring, should not be in final interface
status_t SurfaceTexture::connect(int api,
uint32_t* outWidth, uint32_t* outHeight, uint32_t* outTransform) {
Mutex::Autolock lock(mMutex);
@@ -737,8 +742,6 @@
freeBufferLocked(i);
}
}
-
- mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT;
}
void SurfaceTexture::dump(String8& result) const
diff --git a/libs/gui/tests/SurfaceTexture_test.cpp b/libs/gui/tests/SurfaceTexture_test.cpp
index bf347c6..3009093 100644
--- a/libs/gui/tests/SurfaceTexture_test.cpp
+++ b/libs/gui/tests/SurfaceTexture_test.cpp
@@ -132,7 +132,7 @@
}
virtual void TearDown() {
- // Display the result
+ // Display the result
if (mDisplaySecs > 0 && mEglSurface != EGL_NO_SURFACE) {
eglSwapBuffers(mEglDisplay, mEglSurface);
sleep(mDisplaySecs);
@@ -486,6 +486,55 @@
Condition mCondition;
};
+ // Note that SurfaceTexture will lose the notifications
+ // onBuffersReleased and onFrameAvailable as there is currently
+ // no way to forward the events. This DisconnectWaiter will not let the
+ // disconnect finish until finishDisconnect() is called. It will
+ // also block until a disconnect is called
+ class DisconnectWaiter : public BufferQueue::ConsumerListener {
+ public:
+ DisconnectWaiter () :
+ mWaitForDisconnect(false),
+ mPendingFrames(0) {
+ }
+
+ void waitForFrame() {
+ Mutex::Autolock lock(mMutex);
+ while (mPendingFrames == 0) {
+ mFrameCondition.wait(mMutex);
+ }
+ mPendingFrames--;
+ }
+
+ virtual void onFrameAvailable() {
+ Mutex::Autolock lock(mMutex);
+ mPendingFrames++;
+ mFrameCondition.signal();
+ }
+
+ virtual void onBuffersReleased() {
+ Mutex::Autolock lock(mMutex);
+ while (!mWaitForDisconnect) {
+ mDisconnectCondition.wait(mMutex);
+ }
+ }
+
+ void finishDisconnect() {
+ Mutex::Autolock lock(mMutex);
+ mWaitForDisconnect = true;
+ mDisconnectCondition.signal();
+ }
+
+ private:
+ Mutex mMutex;
+
+ bool mWaitForDisconnect;
+ Condition mDisconnectCondition;
+
+ int mPendingFrames;
+ Condition mFrameCondition;
+ };
+
sp<SurfaceTexture> mST;
sp<SurfaceTextureClient> mSTC;
sp<ANativeWindow> mANW;
@@ -984,6 +1033,102 @@
EXPECT_TRUE(checkPixel( 3, 52, 35, 231, 35, 35));
}
+// Tests if SurfaceTexture and BufferQueue are robust enough
+// to handle a special case where updateTexImage is called
+// in the middle of disconnect. This ordering is enforced
+// by blocking in the disconnect callback.
+TEST_F(SurfaceTextureGLTest, DisconnectStressTest) {
+
+ class ProducerThread : public Thread {
+ public:
+ ProducerThread(const sp<ANativeWindow>& anw):
+ mANW(anw) {
+ }
+
+ virtual ~ProducerThread() {
+ }
+
+ virtual bool threadLoop() {
+ ANativeWindowBuffer* anb;
+
+ native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL);
+
+ for (int numFrames =0 ; numFrames < 2; numFrames ++) {
+
+ if (mANW->dequeueBuffer(mANW.get(), &anb) != NO_ERROR) {
+ return false;
+ }
+ if (anb == NULL) {
+ return false;
+ }
+ if (mANW->queueBuffer(mANW.get(), anb)
+ != NO_ERROR) {
+ return false;
+ }
+ }
+
+ native_window_api_disconnect(mANW.get(), NATIVE_WINDOW_API_EGL);
+
+ return false;
+ }
+
+ private:
+ sp<ANativeWindow> mANW;
+ };
+
+ ASSERT_EQ(OK, mST->setSynchronousMode(true));
+
+ sp<DisconnectWaiter> dw(new DisconnectWaiter());
+ mST->getBufferQueue()->consumerConnect(dw);
+
+
+ sp<Thread> pt(new ProducerThread(mANW));
+ pt->run();
+
+ // eat a frame so SurfaceTexture will own an at least one slot
+ dw->waitForFrame();
+ EXPECT_EQ(OK,mST->updateTexImage());
+
+ dw->waitForFrame();
+ // Could fail here as SurfaceTexture thinks it still owns the slot
+ // but bufferQueue has released all slots
+ EXPECT_EQ(OK,mST->updateTexImage());
+
+ dw->finishDisconnect();
+}
+
+
+// This test ensures that the SurfaceTexture clears the mCurrentTexture
+// when it is disconnected and reconnected. Otherwise it will
+// attempt to release a buffer that it does not owned
+TEST_F(SurfaceTextureGLTest, DisconnectClearsCurrentTexture) {
+ ASSERT_EQ(OK, mST->setSynchronousMode(true));
+
+ native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL);
+
+ ANativeWindowBuffer *anb;
+
+ EXPECT_EQ (OK, mANW->dequeueBuffer(mANW.get(), &anb));
+ EXPECT_EQ(OK, mANW->queueBuffer(mANW.get(), anb));
+
+ EXPECT_EQ (OK, mANW->dequeueBuffer(mANW.get(), &anb));
+ EXPECT_EQ(OK, mANW->queueBuffer(mANW.get(), anb));
+
+ EXPECT_EQ(OK,mST->updateTexImage());
+ EXPECT_EQ(OK,mST->updateTexImage());
+
+ native_window_api_disconnect(mANW.get(), NATIVE_WINDOW_API_EGL);
+ native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL);
+
+ ASSERT_EQ(OK, mST->setSynchronousMode(true));
+
+ EXPECT_EQ (OK, mANW->dequeueBuffer(mANW.get(), &anb));
+ EXPECT_EQ(OK, mANW->queueBuffer(mANW.get(), anb));
+
+ // Will fail here if mCurrentTexture is not cleared properly
+ EXPECT_EQ(OK,mST->updateTexImage());
+}
+
TEST_F(SurfaceTextureGLTest, AbandonUnblocksDequeueBuffer) {
class ProducerThread : public Thread {
public: