[Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances

[Emil Velikov]

On 15 February 2018 at 09:12, Tapani Pälli <tapani.palli at intel.com> wrote:
> From: Simon Hausmann <simon.hausmann at qt.io>
>
> When looking up known glsl_type instances in the various hash tables, we
> end up leaking the key instances used for the lookup, as the glsl_type
> constructor allocates memory on the global mem_ctx. This patch changes
> glsl_type to manage its own memory, which fixes the leak and also allows
> getting rid of the global mem_ctx and its mutex.
>
> v2: remove lambda usage (Tapani)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104884
> Signed-off-by: Simon Hausmann <simon.hausmann at qt.io>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/compiler/glsl_types.cpp | 83 ++++++++++++++++++++-------------------------
>  src/compiler/glsl_types.h   | 44 +++---------------------
>  2 files changed, 41 insertions(+), 86 deletions(-)
>
Great overall result, there's a small question below.

Can you include the mesa-stable tag and/or the commit that introduced
this shared memory pool.
... Gut feeling says it was before the file started using ralloc.


> @@ -970,7 +961,7 @@ glsl_type::get_array_instance(const glsl_type *base, unsigned array_size)
>        const glsl_type *t = new glsl_type(base, array_size);
>
>        entry = _mesa_hash_table_insert(array_types,
> -                                      ralloc_strdup(mem_ctx, key),
> +                                      strdup(key),
>                                        (void *) t);
>     }
>

Are you sure we need the strdup here? AFAICT nothing clears it so it
will be leaked.

-Emil