[Mesa-dev] [PATCH] mesa: fix renderbuffer leak
Bartosz Tomczyk
bartosz.tomczyk86 at gmail.com
Fri Apr 7 16:25:48 UTC 2017
Patch didn't fix all leaks. There's one more still reported by ASAN:
Direct leak of 2112 byte(s) in 12 object(s) allocated from:
#0 0x7fe18d168800 in calloc (/usr/lib/clang/3.9.1/lib/linux/
libclang_rt.asan-x86_64.so+0xf6800)
#1 0x7fe181c141f6 in st_new_renderbuffer_fb
/home/bartek/Devel/mesa/src/mesa/state_tracker/st_cb_fbo.c:277:11
On Fri, Apr 7, 2017 at 6:03 PM, Brian Paul <brianp at vmware.com> wrote:
> On 04/06/2017 03:55 PM, Timothy Arceri wrote:
>
>> We don't need to call _mesa_reference_renderbuffer() for the first
>> assignment as refCount starts at 1. For swrast we work around the
>> fact we will indirectly call _mesa_reference_renderbuffer() by
>> resetting refCount to 0.
>>
>> Fixes: 32141e53d1520 (mesa: tidy up renderbuffer RefCount initialisation)
>> ---
>> src/mesa/main/fbobject.c | 2 +-
>> src/mesa/swrast/s_renderbuffer.c | 5 +++++
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index e52c1e3..e46a7ca 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -441,21 +441,21 @@ _mesa_update_texture_renderbuffer(struct
>> gl_context *ctx,
>>
>> texImage = att->Texture->Image[att->CubeMapFace][att->TextureLevel];
>>
>> rb = att->Renderbuffer;
>> if (!rb) {
>> rb = ctx->Driver.NewRenderbuffer(ctx, ~0);
>> if (!rb) {
>> _mesa_error(ctx, GL_OUT_OF_MEMORY, "glFramebufferTexture()");
>> return;
>> }
>> - _mesa_reference_renderbuffer(&att->Renderbuffer, rb);
>> + att->Renderbuffer = rb;
>>
>> /* This can't get called on a texture renderbuffer, so set it to
>> NULL
>> * for clarity compared to user renderbuffers.
>> */
>> rb->AllocStorage = NULL;
>>
>> rb->NeedsFinishRenderTexture = ctx->Driver.FinishRenderTexture
>> != NULL;
>> }
>>
>> if (!texImage)
>> diff --git a/src/mesa/swrast/s_renderbuffer.c
>> b/src/mesa/swrast/s_renderbuffer.c
>> index d8c4ed3..af09955 100644
>> --- a/src/mesa/swrast/s_renderbuffer.c
>> +++ b/src/mesa/swrast/s_renderbuffer.c
>> @@ -264,20 +264,25 @@ add_color_renderbuffers(struct gl_context *ctx,
>> struct gl_framebuffer *fb,
>> continue;
>>
>> assert(fb->Attachment[b].Renderbuffer == NULL);
>>
>> rb = ctx->Driver.NewRenderbuffer(ctx, 0);
>> if (!rb) {
>> _mesa_error(ctx, GL_OUT_OF_MEMORY, "Allocating color buffer");
>> return GL_FALSE;
>> }
>>
>> + /* Set refcount to 0 to avoid a leak since the
>> _mesa_add_renderbuffer()
>> + * call below will bump the initial refcount.
>> + */
>> + rb->RefCount = 0;
>> +
>> rb->InternalFormat = GL_RGBA;
>>
>> rb->AllocStorage = soft_renderbuffer_storage;
>> _mesa_add_renderbuffer(fb, b, rb);
>>
>
> Perhaps another way of doing this which wouldn't look quite so hacky would
> be to call _mesa_renderbuffer_reference(&rb, NULL) here, after the
> _mesa_add_renderbuffer() call. The idea is we're transferring ownership of
> the rb and the rb var is going out of scope.
>
> Something similar could be done above. A comment indicating that we're
> transferring ownership could be helpful too.
>
> -Brian
>
> }
>>
>> return GL_TRUE;
>> }
>>
>>
>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170407/41e7ff54/attachment-0001.html>
More information about the mesa-dev
mailing list