<div dir="ltr">Patch didn't fix all leaks. There's one more still reported by ASAN:<div> </div><div>Direct leak of 2112 byte(s) in 12 object(s) allocated from: #0 0x7fe18d168800 in calloc (/usr/lib/clang/3.9.1/lib/linux/<a href="http://libclang_rt.asan-x86_64.so">libclang_rt.asan-x86_64.so</a>+0xf6800) #1 0x7fe181c141f6 in st_new_renderbuffer_fb /home/bartek/Devel/mesa/src/mesa/state_tracker/st_cb_fbo.c:277:11 </div><div> </div></div><div class="gmail_extra"> <div class="gmail_quote">On Fri, Apr 7, 2017 at 6:03 PM, Brian Paul <<a href="mailto:brianp@vmware.com" target="_blank">brianp@vmware.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 04/06/2017 03:55 PM, Timothy Arceri wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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); </blockquote> </div></div> 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 <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> } return GL_TRUE; } </blockquote><div class="HOEnZb"><div class="h5"> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div>