[Mesa-dev] [PATCH] mesa: fix renderbuffer leak

[Bartosz Tomczyk]

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>