Merge "Fix a GraphicBuffer leak in SurfaceTexture"
diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h
index 585d288..340daaf 100644
--- a/include/gui/SurfaceTexture.h
+++ b/include/gui/SurfaceTexture.h
@@ -248,12 +248,6 @@
// allocate new GraphicBuffer objects.
sp<IGraphicBufferAlloc> mGraphicBufferAlloc;
- // mAllocdBuffers is mirror of the list of buffers that SurfaceFlinger is
- // referencing. This is kept so that gralloc implementations do not need to
- // properly handle the case where SurfaceFlinger no longer holds a reference
- // to a buffer, but other processes do.
- Vector<sp<GraphicBuffer> > mAllocdBuffers;
-
// mFrameAvailableListener is the listener object that will be called when a
// new frame becomes available. If it is not NULL it will be called from
// queueBuffer.
diff --git a/include/surfaceflinger/IGraphicBufferAlloc.h b/include/surfaceflinger/IGraphicBufferAlloc.h
index d996af7..01e4bd9 100644
--- a/include/surfaceflinger/IGraphicBufferAlloc.h
+++ b/include/surfaceflinger/IGraphicBufferAlloc.h
@@ -32,18 +32,10 @@
public:
DECLARE_META_INTERFACE(GraphicBufferAlloc);
- /* Create a new GraphicBuffer for the client to use. The server will
- * maintain a reference to the newly created GraphicBuffer until
- * freeAllGraphicBuffers is called.
+ /* Create a new GraphicBuffer for the client to use.
*/
virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
PixelFormat format, uint32_t usage) = 0;
-
- /* Free all but one of the GraphicBuffer objects that the server is
- * currently referencing. If bufIndex is not a valid index of the buffers
- * the server is referencing, then all buffers are freed.
- */
- virtual void freeAllGraphicBuffersExcept(int bufIndex) = 0;
};
// ----------------------------------------------------------------------------
diff --git a/libs/gui/IGraphicBufferAlloc.cpp b/libs/gui/IGraphicBufferAlloc.cpp
index e05da72..0cd51da 100644
--- a/libs/gui/IGraphicBufferAlloc.cpp
+++ b/libs/gui/IGraphicBufferAlloc.cpp
@@ -32,7 +32,6 @@
enum {
CREATE_GRAPHIC_BUFFER = IBinder::FIRST_CALL_TRANSACTION,
- FREE_ALL_GRAPHIC_BUFFERS_EXCEPT,
};
class BpGraphicBufferAlloc : public BpInterface<IGraphicBufferAlloc>
@@ -46,8 +45,7 @@
virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
PixelFormat format, uint32_t usage) {
Parcel data, reply;
- data.writeInterfaceToken(
- IGraphicBufferAlloc::getInterfaceDescriptor());
+ data.writeInterfaceToken(IGraphicBufferAlloc::getInterfaceDescriptor());
data.writeInt32(w);
data.writeInt32(h);
data.writeInt32(format);
@@ -58,17 +56,12 @@
if (nonNull) {
graphicBuffer = new GraphicBuffer();
reply.read(*graphicBuffer);
+ // reply.readStrongBinder();
+ // here we don't even have to read the BufferReference from
+ // the parcel, it'll die with the parcel.
}
return graphicBuffer;
}
-
- virtual void freeAllGraphicBuffersExcept(int bufIdx) {
- Parcel data, reply;
- data.writeInterfaceToken(
- IGraphicBufferAlloc::getInterfaceDescriptor());
- data.writeInt32(bufIdx);
- remote()->transact(FREE_ALL_GRAPHIC_BUFFERS_EXCEPT, data, &reply);
- }
};
IMPLEMENT_META_INTERFACE(GraphicBufferAlloc, "android.ui.IGraphicBufferAlloc");
@@ -80,6 +73,17 @@
{
// codes that don't require permission check
+ /* BufferReference just keeps a strong reference to a
+ * GraphicBuffer until it is destroyed (that is, until
+ * no local or remote process have a reference to it).
+ */
+ class BufferReference : public BBinder {
+ sp<GraphicBuffer> buffer;
+ public:
+ BufferReference(const sp<GraphicBuffer>& buffer) : buffer(buffer) { }
+ };
+
+
switch(code) {
case CREATE_GRAPHIC_BUFFER: {
CHECK_INTERFACE(IGraphicBufferAlloc, data, reply);
@@ -91,15 +95,16 @@
reply->writeInt32(result != 0);
if (result != 0) {
reply->write(*result);
+ // We add a BufferReference to this parcel to make sure the
+ // buffer stays alive until the GraphicBuffer object on
+ // the other side has been created.
+ // This is needed so that the buffer handle can be
+ // registered before the buffer is destroyed on implementations
+ // that do not use file-descriptors to track their buffers.
+ reply->writeStrongBinder( new BufferReference(result) );
}
return NO_ERROR;
} break;
- case FREE_ALL_GRAPHIC_BUFFERS_EXCEPT: {
- CHECK_INTERFACE(IGraphicBufferAlloc, data, reply);
- int bufIdx = data.readInt32();
- freeAllGraphicBuffersExcept(bufIdx);
- return NO_ERROR;
- } break;
default:
return BBinder::onTransact(code, data, reply, flags);
}
diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp
index f4e2a67..e2346f0 100644
--- a/libs/gui/SurfaceTexture.cpp
+++ b/libs/gui/SurfaceTexture.cpp
@@ -172,7 +172,6 @@
mSlots[buf].mEglImage = EGL_NO_IMAGE_KHR;
mSlots[buf].mEglDisplay = EGL_NO_DISPLAY;
}
- mAllocdBuffers.add(graphicBuffer);
}
return graphicBuffer;
}
@@ -425,19 +424,6 @@
mSlots[i].mEglDisplay = EGL_NO_DISPLAY;
}
}
-
- int exceptBuf = -1;
- for (size_t i = 0; i < mAllocdBuffers.size(); i++) {
- if (mAllocdBuffers[i] == mCurrentTextureBuf) {
- exceptBuf = i;
- break;
- }
- }
- mAllocdBuffers.clear();
- if (exceptBuf >= 0) {
- mAllocdBuffers.add(mCurrentTextureBuf);
- }
- mGraphicBufferAlloc->freeAllGraphicBuffersExcept(exceptBuf);
}
EGLImageKHR SurfaceTexture::createImage(EGLDisplay dpy,
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index ea283c606..2f3a144 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2553,22 +2553,9 @@
LOGE("createGraphicBuffer: unable to create GraphicBuffer");
return 0;
}
- Mutex::Autolock _l(mLock);
- mBuffers.add(graphicBuffer);
return graphicBuffer;
}
-void GraphicBufferAlloc::freeAllGraphicBuffersExcept(int bufIdx) {
- Mutex::Autolock _l(mLock);
- if (bufIdx >= 0 && size_t(bufIdx) < mBuffers.size()) {
- sp<GraphicBuffer> b(mBuffers[bufIdx]);
- mBuffers.clear();
- mBuffers.add(b);
- } else {
- mBuffers.clear();
- }
-}
-
// ---------------------------------------------------------------------------
GraphicPlane::GraphicPlane()
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 0964848..8d43157 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -125,14 +125,8 @@
public:
GraphicBufferAlloc();
virtual ~GraphicBufferAlloc();
-
virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t w, uint32_t h,
PixelFormat format, uint32_t usage);
- virtual void freeAllGraphicBuffersExcept(int bufIdx);
-
-private:
- Vector<sp<GraphicBuffer> > mBuffers;
- Mutex mLock;
};
// ---------------------------------------------------------------------------