Bug 100741

Summary: Chromium - Memory leak
Product: Mesa Reporter: Bartosz Tomczyk <bartosz.tomczyk86>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact: mesa-dev
Severity: normal    
Priority: medium CC: chadversary, gary.c.wang, lemody, robclark
Version: git   
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
i915 platform: i915 features:

Description Bartosz Tomczyk 2017-04-20 20:17:44 UTC

I observe huge memory leak in chromium browser:

Memory allocation:
==19259== 593,808 (590,328 direct, 3,480 indirect) bytes in 2,733 blocks are definitely lost in loss record 5,325 of 5,331
==19259==    at 0x4C2CF35: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19259==    by 0x9BAC22F: r600_create_surface_custom (r600_texture.c:1935)
==19259==    by 0x9BACA94: r600_create_surface (r600_texture.c:1989)
==19259==    by 0x97CE93F: st_framebuffer_validate (st_manager.c:222)
==19259==    by 0x97CFBEE: st_api_make_current (st_manager.c:808)
==19259==    by 0x9931097: dri_make_current (dri_context.c:258)
==19259==    by 0x992FD64: driBindContext (dri_util.c:555)
==19259==    by 0x7E5A19E: dri3_bind_context (dri3_glx.c:235)
==19259==    by 0x7E27AB3: MakeContextCurrent (glxcurrent.c:228)

Memory not freed in:
st_renderbuffer_delete+0x243b58: state_tracker/st_cb_fbo.c:246
_mesa_reference_renderbuffer_+0x1997d0: main/renderbuffer.c:212
_mesa_reference_renderbuffer+0x1259d1: main/renderbuffer.h:72
_mesa_free_framebuffer_data+0x1259d1: main/framebuffer.c:223
_mesa_destroy_framebuffer+0x125ad0: main/framebuffer.c:199
_mesa_reference_framebuffer_+0x125b78: main/framebuffer.c:256
_mesa_reference_framebuffer+0x8b5b7: main/framebuffer.h:63
_mesa_make_current+0x8b5b7: main/context.c:1674
st_api_make_current+0x294cba: state_tracker/st_manager.c:827
dri_unbind_context+0x3f5f7d: src/gallium/state_trackers/dri/dri_context.c:217
driUnbindContext+0x3f57ec: src/mesa/drivers/dri/common/dri_util.c:591
MakeContextCurrent+0x164: src/glx/glxcurrent.c:214

Memory is not freed in st_renderbuffer_delete because ctx is NULL.
    if (ctx) {
       struct st_context *st = st_context(ctx);
       pipe_surface_release(st->pipe, &strb->surface);

Changing above part of code to :
    pipe_surface_reference(&strb->surface, NULL);
    pipe_surface_release(strb->surface->context, &strb->surface);

fixes problem for me, but I'm not sure if it is correct way of doing it.
Could someone comment on this?
Comment 1 Bartosz Tomczyk 2017-04-23 09:37:48 UTC
I've bisected it and first bad commit is:

commit a5e733c6b52e93de3000647d075f5ca2f55fcb71
Author: Rob Clark <robdclark@gmail.com>
Date:   Wed Oct 26 16:52:52 2016 -0400

    mesa: drop current draw/read buffer when ctx is released

Another way of fixing it:

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 3570f94f5a..b0a46422f2 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1664,8 +1664,6 @@ _mesa_make_current( struct gl_context *newCtx,
    /* We used to call _glapi_check_multithread() here.  Now do it in drivers */
-   _glapi_set_context((void *) newCtx);
-   assert(_mesa_get_current_context() == newCtx);
    if (!newCtx) {
       _glapi_set_dispatch(NULL);  /* none current */
@@ -1673,8 +1671,12 @@ _mesa_make_current( struct gl_context *newCtx,
          _mesa_reference_framebuffer(&curCtx->WinSysDrawBuffer, NULL);
          _mesa_reference_framebuffer(&curCtx->WinSysReadBuffer, NULL);
+      _glapi_set_context(NULL);
+      assert(_mesa_get_current_context() == NULL);
    else {
+      _glapi_set_context((void *) newCtx);
+      assert(_mesa_get_current_context() == newCtx);
       if (drawBuffer && readBuffer) {
Comment 2 Bartosz Tomczyk 2017-04-26 19:27:14 UTC
Can someone more experienced comment, which if any of above fixes is correct?
Comment 3 Emil Velikov 2017-04-27 11:14:44 UTC
Adding the commit author - Rob.

The pipe_surface workarounds don't look right. The patch in comment 1 makes sense.
Bartosz, please send it to the list and include a Cc: Rob.. and Fixes: a5e733c6b52 ("...").

There's a gut feeling that we might need something more, but nothing concrete comes to mind.
Comment 4 Emil Velikov 2017-05-30 13:51:06 UTC
Pushed to master

commit fd6c2a3f3eb7f5f3077fb95b1441ddaa43b806fe
Author: Bartosz Tomczyk <bartosz.tomczyk86@gmail.com>
Date:   Sat Apr 29 16:37:45 2017 +0200

    mesa: Avoid leaking surface in st_renderbuffer_delete

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.