fix the threading issue for setBuffercount()
this change introduces R/W locks in the right places.
on the server-side, it guarantees that setBufferCount()
is synchronized with "retire" and "resize".
on the client-side, it guarantees that setBufferCount()
is synchronized with "dequeue", "lockbuffer" and "queue"
diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp
index 1fe997d..621b7e3 100644
--- a/libs/surfaceflinger/Layer.cpp
+++ b/libs/surfaceflinger/Layer.cpp
@@ -60,7 +60,7 @@
// no OpenGL operation is possible here, since we might not be
// in the OpenGL thread.
lcblk = new SharedBufferServer(
- client->ctrlblk, i, mBufferManager.getBufferCount(),
+ client->ctrlblk, i, mBufferManager.getDefaultBufferCount(),
getIdentity());
mBufferManager.setActiveBufferIndex( lcblk->getFrontBuffer() );
@@ -68,7 +68,10 @@
Layer::~Layer()
{
- destroy();
+ // FIXME: must be called from the main UI thread
+ EGLDisplay dpy(mFlinger->graphicPlane(0).getEGLDisplay());
+ mBufferManager.destroy(dpy);
+
// the actual buffers will be destroyed here
delete lcblk;
}
@@ -81,17 +84,6 @@
lcblk->setStatus(NO_INIT);
}
-void Layer::destroy()
-{
- EGLDisplay dpy(mFlinger->graphicPlane(0).getEGLDisplay());
- mBufferManager.destroy(dpy);
-
- mSurface.clear();
-
- Mutex::Autolock _l(mLock);
- mWidth = mHeight = 0;
-}
-
sp<LayerBaseClient::Surface> Layer::createSurface() const
{
return mSurface;
@@ -99,9 +91,17 @@
status_t Layer::ditch()
{
+ // NOTE: Called from the main UI thread
+
// the layer is not on screen anymore. free as much resources as possible
mFreezeLock.clear();
- destroy();
+
+ EGLDisplay dpy(mFlinger->graphicPlane(0).getEGLDisplay());
+ mBufferManager.destroy(dpy);
+ mSurface.clear();
+
+ Mutex::Autolock _l(mLock);
+ mWidth = mHeight = 0;
return NO_ERROR;
}
@@ -211,7 +211,7 @@
status_t Layer::setBufferCount(int bufferCount)
{
- // this ensures our client doesn't go away while we're accessing
+ // Ensures our client doesn't go away while we're accessing
// the shared area.
sp<Client> ourClient(client.promote());
if (ourClient == 0) {
@@ -219,15 +219,10 @@
return DEAD_OBJECT;
}
- status_t err;
-
- // FIXME: resize() below is NOT thread-safe, we need to synchronize
- // the users of lcblk in our process (ie: retire), and we assume the
- // client is not mucking with the SharedStack, which is only enforced
- // by construction, therefore we need to protect ourselves against
- // buggy and malicious client (as always)
-
- err = lcblk->resize(bufferCount);
+ // NOTE: lcblk->resize() is protected by an internal lock
+ status_t err = lcblk->resize(bufferCount);
+ if (err == NO_ERROR)
+ mBufferManager.resize(bufferCount);
return err;
}
@@ -248,7 +243,7 @@
* 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
@@ -544,12 +539,20 @@
// ---------------------------------------------------------------------------
Layer::BufferManager::BufferManager(TextureManager& tm)
- : mTextureManager(tm), mActiveBuffer(0), mFailover(false)
+ : mNumBuffers(NUM_BUFFERS), mTextureManager(tm),
+ mActiveBuffer(0), mFailover(false)
{
}
-size_t Layer::BufferManager::getBufferCount() const {
- return NUM_BUFFERS;
+Layer::BufferManager::~BufferManager()
+{
+}
+
+status_t Layer::BufferManager::resize(size_t size)
+{
+ Mutex::Autolock _l(mLock);
+ mNumBuffers = size;
+ return NO_ERROR;
}
// only for debugging
@@ -568,51 +571,55 @@
}
Texture Layer::BufferManager::getActiveTexture() const {
- return mFailover ? mFailoverTexture : mBufferData[mActiveBuffer].texture;
+ Texture res;
+ if (mFailover) {
+ res = mFailoverTexture;
+ } else {
+ static_cast<Image&>(res) = mBufferData[mActiveBuffer].texture;
+ }
+ return res;
}
sp<GraphicBuffer> Layer::BufferManager::getActiveBuffer() const {
+ const size_t activeBuffer = mActiveBuffer;
+ BufferData const * const buffers = mBufferData;
Mutex::Autolock _l(mLock);
- return mBufferData[mActiveBuffer].buffer;
+ return buffers[activeBuffer].buffer;
}
sp<GraphicBuffer> Layer::BufferManager::detachBuffer(size_t index)
{
+ BufferData* const buffers = mBufferData;
sp<GraphicBuffer> buffer;
Mutex::Autolock _l(mLock);
- buffer = mBufferData[index].buffer;
- mBufferData[index].buffer = 0;
+ buffer = buffers[index].buffer;
+ buffers[index].buffer = 0;
return buffer;
}
status_t Layer::BufferManager::attachBuffer(size_t index,
const sp<GraphicBuffer>& buffer)
{
+ BufferData* const buffers = mBufferData;
Mutex::Autolock _l(mLock);
- mBufferData[index].buffer = buffer;
- mBufferData[index].texture.dirty = true;
- return NO_ERROR;
-}
-
-status_t Layer::BufferManager::destroyTexture(Texture* tex, EGLDisplay dpy)
-{
- if (tex->name != -1U) {
- glDeleteTextures(1, &tex->name);
- tex->name = -1U;
- }
- if (tex->image != EGL_NO_IMAGE_KHR) {
- eglDestroyImageKHR(dpy, tex->image);
- tex->image = EGL_NO_IMAGE_KHR;
- }
+ buffers[index].buffer = buffer;
+ buffers[index].texture.dirty = true;
return NO_ERROR;
}
status_t Layer::BufferManager::destroy(EGLDisplay dpy)
{
- Mutex::Autolock _l(mLock);
- for (size_t i=0 ; i<NUM_BUFFERS ; i++) {
- destroyTexture(&mBufferData[i].texture, dpy);
- mBufferData[i].buffer = 0;
+ BufferData* const buffers = mBufferData;
+ size_t num;
+ { // scope for the lock
+ Mutex::Autolock _l(mLock);
+ num = mNumBuffers;
+ for (size_t i=0 ; i<num ; i++) {
+ buffers[i].buffer = 0;
+ }
+ }
+ for (size_t i=0 ; i<num ; i++) {
+ destroyTexture(&buffers[i].texture, dpy);
}
destroyTexture(&mFailoverTexture, dpy);
return NO_ERROR;
@@ -622,7 +629,7 @@
const sp<GraphicBuffer>& buffer)
{
size_t index = mActiveBuffer;
- Texture& texture(mBufferData[index].texture);
+ Image& texture(mBufferData[index].texture);
status_t err = mTextureManager.initEglImage(&texture, dpy, buffer);
// if EGLImage fails, we switch to regular texture mode, and we
// free all resources associated with using EGLImages.
@@ -631,7 +638,8 @@
destroyTexture(&mFailoverTexture, dpy);
} else {
mFailover = true;
- for (size_t i=0 ; i<NUM_BUFFERS ; i++) {
+ const size_t num = mNumBuffers;
+ for (size_t i=0 ; i<num ; i++) {
destroyTexture(&mBufferData[i].texture, dpy);
}
}
@@ -644,6 +652,19 @@
return mTextureManager.loadTexture(&mFailoverTexture, dirty, t);
}
+status_t Layer::BufferManager::destroyTexture(Image* tex, EGLDisplay dpy)
+{
+ if (tex->name != -1U) {
+ glDeleteTextures(1, &tex->name);
+ tex->name = -1U;
+ }
+ if (tex->image != EGL_NO_IMAGE_KHR) {
+ eglDestroyImageKHR(dpy, tex->image);
+ tex->image = EGL_NO_IMAGE_KHR;
+ }
+ return NO_ERROR;
+}
+
// ---------------------------------------------------------------------------
Layer::SurfaceLayer::SurfaceLayer(const sp<SurfaceFlinger>& flinger,
@@ -661,6 +682,11 @@
sp<GraphicBuffer> buffer;
sp<Layer> owner(getOwner());
if (owner != 0) {
+ /*
+ * requestBuffer() cannot be called from the main thread
+ * as it could cause a dead-lock, since it may have to wait
+ * on conditions updated my the main thread.
+ */
buffer = owner->requestBuffer(index, usage);
}
return buffer;
@@ -671,6 +697,11 @@
status_t err = DEAD_OBJECT;
sp<Layer> owner(getOwner());
if (owner != 0) {
+ /*
+ * setBufferCount() cannot be called from the main thread
+ * as it could cause a dead-lock, since it may have to wait
+ * on conditions updated my the main thread.
+ */
err = owner->setBufferCount(bufferCount);
}
return err;