ADHOC/replicant-frameworks_native
3
0
Fork 0
Code Issues 6 Pull Requests Releases Wiki Activity

Fix a GraphicBuffer leak in SurfaceTexture

Browse Source 
This leak was intentional, it was there to deal with the fact that
some gralloc implementations don't track buffer handles with
file-descriptors so buffers needed to stay alive until there were
registered, which is not guaranteed by binder transactions.

In this new implementation, we use a small BBinder holding a
reference to the buffer, which with tuck into the parcel. This forces
the reference to stay alive until the parcel is destroyed, which
is guaranteed (by construction) to happen after the buffer is
registered.

this allows the public facing API to not expose the previous hack.

Change-Id: I1dd6cd83679a2b7457ad628169e2851acc027143
This commit is contained in:
Mathias Agopian 2011-04-08 19:10:43 -07:00
parent 626d865d41
commit 4cb18881b5
6 changed files with 23 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
include/gui/SurfaceTexture.h
View File

@ -248,12 +248,6 @@ private:
// 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.

10
include/surfaceflinger/IGraphicBufferAlloc.h
View File

@ -32,18 +32,10 @@ class IGraphicBufferAlloc : public IInterface
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;
};
// ----------------------------------------------------------------------------

39
libs/gui/IGraphicBufferAlloc.cpp
View File

@ -32,7 +32,6 @@ namespace android {
enum {
CREATE_GRAPHIC_BUFFER = IBinder::FIRST_CALL_TRANSACTION,
FREE_ALL_GRAPHIC_BUFFERS_EXCEPT,
};
class BpGraphicBufferAlloc : public BpInterface<IGraphicBufferAlloc>
@ -46,8 +45,7 @@ public:
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 @@ public:
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 @@ status_t BnGraphicBufferAlloc::onTransact(
{
// 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 @@ status_t BnGraphicBufferAlloc::onTransact(
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);
}

14
libs/gui/SurfaceTexture.cpp
View File

@ -172,7 +172,6 @@ sp<GraphicBuffer> SurfaceTexture::requestBuffer(int buf,
mSlots[buf].mEglImage = EGL_NO_IMAGE_KHR;
mSlots[buf].mEglDisplay = EGL_NO_DISPLAY;
}
mAllocdBuffers.add(graphicBuffer);
}
return graphicBuffer;
}
@ -425,19 +424,6 @@ void SurfaceTexture::freeAllBuffers() {
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,

13
services/surfaceflinger/SurfaceFlinger.cpp
View File

@ -2553,22 +2553,9 @@ sp<GraphicBuffer> GraphicBufferAlloc::createGraphicBuffer(uint32_t w, uint32_t h
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()

6
services/surfaceflinger/SurfaceFlinger.h
View File

@ -125,14 +125,8 @@ class GraphicBufferAlloc : public BnGraphicBufferAlloc
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;
};
// ---------------------------------------------------------------------------