[Mesa-dev] [PATCH] mesa: tidy up renderbuffer RefCount initialisation

[Bartosz Tomczyk]

Timothy,

 I observe huge memory leak after this commit:

Direct leak of 648208 byte(s) in 3683 object(s) allocated from:
   #0 0x7f3d72729800 in calloc (/usr/lib/clang/3.9.1/lib/linux/
libclang_rt.asan-x86_64.so+0xf6800)

   #1 0x7f3d64a4d114 in st_new_renderbuffer
/home/bartek/Devel/mesa/src/mesa/state_tracker/st_cb_fbo.c:256:35
   #2 0x7f3d6373ff1a in _mesa_update_texture_renderbuffer
/home/bartek/Devel/mesa/src/mesa/main/fbobject.c:449:12
   #3 0x7f3d6376be9f in set_texture_attachment
/home/bartek/Devel/mesa/src/mesa/main/fbobject.c:517:4
   #4 0x7f3d637699e2 in _mesa_framebuffer_texture
/home/bartek/Devel/mesa/src/mesa/main/fbobject.c:3251:10
   #5 0x7f3d6376c59c in framebuffer_texture_with_dims
/home/bartek/Devel/mesa/src/mesa/main/fbobject.c:3322:4
   #6 0x7f3d6376c695 in _mesa_FramebufferTexture2D
/home/bartek/Devel/mesa/src/mesa/main/fbobject.c:3340:4
   #7 0x7f3d63db99cd in _mesa_unmarshal_FramebufferTexture2D
/home/bartek/Devel/mesa/src/mesa/main/marshal_generated.c:20078:4
   #8 0x7f3d63d50861 in _mesa_unmarshal_dispatch_cmd
/home/bartek/Devel/mesa/src/mesa/main/marshal_generated.c:40838:7
   #9 0x7f3d63cd7634 in glthread_unmarshal_batch
/home/bartek/Devel/mesa/src/mesa/main/glthread.c:64:14
   #10 0x7f3d63cd6f71 in _mesa_glthread_finish
/home/bartek/Devel/mesa/src/mesa/main/glthread.c:276:10
   #11 0x7f3d63fd3444 in _mesa_marshal_GetIntegerv
/home/bartek/Devel/mesa/src/mesa/main/marshal_generated.c:36059:4
   #12 0x53a2b2  (/usr/bin/glretrace+0x53a2b2)
   #13 0x5b8bf3  (/usr/bin/glretrace+0x5b8bf3)
   #14 0x40c48b  (/usr/bin/glretrace+0x40c48b)
   #15 0x40cbe9  (/usr/bin/glretrace+0x40cbe9)
   #16 0x40ce91  (/usr/bin/glretrace+0x40ce91)
   #17 0x7f3d720dd2e6 in start_thread (/usr/lib/libpthread.so.0+0x72e6)


On Wed, Apr 5, 2017 at 7:21 PM, Brian Paul <brianp at vmware.com> wrote:

> On 04/05/2017 05:06 AM, Timothy Arceri wrote:
>
>> 42aaa548 changed the renderbuffer initialisation of RefCount from
>> 1 to 0.
>>
>> This is inconsitent with how we use RefCount elsewhere. Also every
>> driver implementation of NewRenderbuffer() calls
>> _mesa_init_renderbuffer() so its safe to set it there.
>> ---
>>   src/mesa/drivers/common/meta.c | 2 --
>>   src/mesa/main/fbobject.c       | 1 -
>>   src/mesa/main/renderbuffer.c   | 2 +-
>>   3 files changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta.c
>> b/src/mesa/drivers/common/meta.c
>> index efbc934..f4c91ac 100644
>> --- a/src/mesa/drivers/common/meta.c
>> +++ b/src/mesa/drivers/common/meta.c
>> @@ -3037,22 +3037,20 @@ decompress_texture_image(struct gl_context *ctx,
>>                                     ctx->Texture.Unit[ctx->Texture
>> .CurrentUnit].Sampler);
>>
>>      /* Create/bind FBO/renderbuffer */
>>      if (decompress_fbo->fb == NULL) {
>>         decompress_fbo->rb = ctx->Driver.NewRenderbuffer(ctx,
>> 0xDEADBEEF);
>>         if (decompress_fbo->rb == NULL) {
>>            _mesa_meta_end(ctx);
>>            return false;
>>         }
>>
>> -      decompress_fbo->rb->RefCount = 1;
>> -
>>         decompress_fbo->fb = ctx->Driver.NewFramebuffer(ctx, 0xDEADBEEF);
>>         if (decompress_fbo->fb == NULL) {
>>            _mesa_meta_end(ctx);
>>            return false;
>>         }
>>
>>         _mesa_bind_framebuffers(ctx, decompress_fbo->fb,
>> decompress_fbo->fb);
>>         _mesa_framebuffer_renderbuffer(ctx, ctx->DrawBuffer,
>> GL_COLOR_ATTACHMENT0,
>>                                        decompress_fbo->rb);
>>      }
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 4d9a7b8..e52c1e3 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -1309,21 +1309,20 @@ allocate_renderbuffer_locked(struct gl_context
>> *ctx, GLuint renderbuffer,
>>      struct gl_renderbuffer *newRb;
>>
>>      /* create new renderbuffer object */
>>      newRb = ctx->Driver.NewRenderbuffer(ctx, renderbuffer);
>>      if (!newRb) {
>>         _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
>>         return NULL;
>>      }
>>      assert(newRb->AllocStorage);
>>      _mesa_HashInsertLocked(ctx->Shared->RenderBuffers, renderbuffer,
>> newRb);
>> -   newRb->RefCount = 1; /* referenced by hash table */
>>
>>      return newRb;
>>   }
>>
>>
>>   static void
>>   bind_renderbuffer(GLenum target, GLuint renderbuffer, bool
>> allow_user_names)
>>   {
>>      struct gl_renderbuffer *newRb;
>>      GET_CURRENT_CONTEXT(ctx);
>> diff --git a/src/mesa/main/renderbuffer.c b/src/mesa/main/renderbuffer.c
>> index 53e0b74..1d24e9c 100644
>> --- a/src/mesa/main/renderbuffer.c
>> +++ b/src/mesa/main/renderbuffer.c
>> @@ -36,21 +36,21 @@
>>   /**
>>    * Initialize the fields of a gl_renderbuffer to default values.
>>    */
>>   void
>>   _mesa_init_renderbuffer(struct gl_renderbuffer *rb, GLuint name)
>>   {
>>      GET_CURRENT_CONTEXT(ctx);
>>
>>      rb->ClassID = 0;
>>      rb->Name = name;
>> -   rb->RefCount = 0;
>> +   rb->RefCount = 1;
>>      rb->Delete = _mesa_delete_renderbuffer;
>>
>>      /* The rest of these should be set later by the caller of this
>> function or
>>       * the AllocStorage method:
>>       */
>>      rb->AllocStorage = NULL;
>>
>>      rb->Width = 0;
>>      rb->Height = 0;
>>      rb->Depth = 0;
>>
>>
> Reviewed-by: Brian Paul <brianp at vmware.com>
>
>
> _______________________________________________
> 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/20170406/486ee729/attachment-0001.html>