[Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
Tapani Pälli
tapani.palli at intel.com
Tue Feb 27 05:43:06 UTC 2018
On 02/26/2018 07:55 PM, Emil Velikov wrote:
> 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.
Sure, can add this.
>
>> @@ -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.
>
Yes I believe we do need it, see comment about base type name some rows
before. It is being freed during _mesa_glsl_release_types() by the
hash_free_type_function when array_types hash is iterated.
// Tapani
More information about the mesa-dev
mailing list