[Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
Tapani Pälli
tapani.palli at intel.com
Mon Feb 26 11:50:00 UTC 2018
ping ..
On 02/15/2018 11:12 AM, Tapani Pälli 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(-)
>
> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
> index 3cc5eb0495..81ff97b1f7 100644
> --- a/src/compiler/glsl_types.cpp
> +++ b/src/compiler/glsl_types.cpp
> @@ -28,23 +28,12 @@
> #include "util/hash_table.h"
>
>
> -mtx_t glsl_type::mem_mutex = _MTX_INITIALIZER_NP;
> mtx_t glsl_type::hash_mutex = _MTX_INITIALIZER_NP;
> hash_table *glsl_type::array_types = NULL;
> hash_table *glsl_type::record_types = NULL;
> hash_table *glsl_type::interface_types = NULL;
> hash_table *glsl_type::function_types = NULL;
> hash_table *glsl_type::subroutine_types = NULL;
> -void *glsl_type::mem_ctx = NULL;
> -
> -void
> -glsl_type::init_ralloc_type_ctx(void)
> -{
> - if (glsl_type::mem_ctx == NULL) {
> - glsl_type::mem_ctx = ralloc_context(NULL);
> - assert(glsl_type::mem_ctx != NULL);
> - }
> -}
>
> glsl_type::glsl_type(GLenum gl_type,
> glsl_base_type base_type, unsigned vector_elements,
> @@ -63,14 +52,12 @@ glsl_type::glsl_type(GLenum gl_type,
> STATIC_ASSERT((unsigned(GLSL_TYPE_INT) & 3) == unsigned(GLSL_TYPE_INT));
> STATIC_ASSERT((unsigned(GLSL_TYPE_FLOAT) & 3) == unsigned(GLSL_TYPE_FLOAT));
>
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
>
> - mtx_unlock(&glsl_type::mem_mutex);
> -
> /* Neither dimension is zero or both dimensions are zero.
> */
> assert((vector_elements == 0) == (matrix_columns == 0));
> @@ -86,14 +73,12 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type base_type,
> sampler_array(array), interface_packing(0),
> interface_row_major(0), length(0)
> {
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
>
> - mtx_unlock(&glsl_type::mem_mutex);
> -
> memset(& fields, 0, sizeof(fields));
>
> matrix_columns = vector_elements = 1;
> @@ -110,9 +95,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> {
> unsigned int i;
>
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
> this->fields.structure = ralloc_array(this->mem_ctx,
> @@ -123,8 +108,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
> fields[i].name);
> }
> -
> - mtx_unlock(&glsl_type::mem_mutex);
> }
>
> glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> @@ -140,9 +123,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> {
> unsigned int i;
>
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
> this->fields.structure = rzalloc_array(this->mem_ctx,
> @@ -152,8 +135,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
> fields[i].name);
> }
> -
> - mtx_unlock(&glsl_type::mem_mutex);
> }
>
> glsl_type::glsl_type(const glsl_type *return_type,
> @@ -167,9 +148,8 @@ glsl_type::glsl_type(const glsl_type *return_type,
> {
> unsigned int i;
>
> - mtx_lock(&glsl_type::mem_mutex);
> -
> - init_ralloc_type_ctx();
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> this->fields.parameters = rzalloc_array(this->mem_ctx,
> glsl_function_param, num_params + 1);
> @@ -185,8 +165,6 @@ glsl_type::glsl_type(const glsl_type *return_type,
> this->fields.parameters[i + 1].in = params[i].in;
> this->fields.parameters[i + 1].out = params[i].out;
> }
> -
> - mtx_unlock(&glsl_type::mem_mutex);
> }
>
> glsl_type::glsl_type(const char *subroutine_name) :
> @@ -197,12 +175,16 @@ glsl_type::glsl_type(const char *subroutine_name) :
> vector_elements(1), matrix_columns(1),
> length(0)
> {
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(subroutine_name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, subroutine_name);
> - mtx_unlock(&glsl_type::mem_mutex);
> +}
> +
> +glsl_type::~glsl_type()
> +{
> + ralloc_free(this->mem_ctx);
> }
>
> bool
> @@ -416,6 +398,17 @@ const glsl_type *glsl_type::get_scalar_type() const
> }
>
>
> +static void
> +hash_free_type_function(struct hash_entry *entry)
> +{
> + glsl_type *type = (glsl_type *) entry->data;
> +
> + if (type->is_array())
> + free((void*)entry->key);
> +
> + delete type;
> +}
> +
> void
> _mesa_glsl_release_types(void)
> {
> @@ -424,32 +417,29 @@ _mesa_glsl_release_types(void)
> * necessary.
> */
> if (glsl_type::array_types != NULL) {
> - _mesa_hash_table_destroy(glsl_type::array_types, NULL);
> + _mesa_hash_table_destroy(glsl_type::array_types, hash_free_type_function);
> glsl_type::array_types = NULL;
> }
>
> if (glsl_type::record_types != NULL) {
> - _mesa_hash_table_destroy(glsl_type::record_types, NULL);
> + _mesa_hash_table_destroy(glsl_type::record_types, hash_free_type_function);
> glsl_type::record_types = NULL;
> }
>
> if (glsl_type::interface_types != NULL) {
> - _mesa_hash_table_destroy(glsl_type::interface_types, NULL);
> + _mesa_hash_table_destroy(glsl_type::interface_types, hash_free_type_function);
> glsl_type::interface_types = NULL;
> }
>
> if (glsl_type::function_types != NULL) {
> - _mesa_hash_table_destroy(glsl_type::function_types, NULL);
> + _mesa_hash_table_destroy(glsl_type::function_types, hash_free_type_function);
> glsl_type::function_types = NULL;
> }
>
> if (glsl_type::subroutine_types != NULL) {
> - _mesa_hash_table_destroy(glsl_type::subroutine_types, NULL);
> + _mesa_hash_table_destroy(glsl_type::subroutine_types, hash_free_type_function);
> glsl_type::subroutine_types = NULL;
> }
> -
> - ralloc_free(glsl_type::mem_ctx);
> - glsl_type::mem_ctx = NULL;
> }
>
>
> @@ -473,9 +463,10 @@ glsl_type::glsl_type(const glsl_type *array, unsigned length) :
> */
> const unsigned name_length = strlen(array->name) + 10 + 3;
>
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
> +
> char *const n = (char *) ralloc_size(this->mem_ctx, name_length);
> - mtx_unlock(&glsl_type::mem_mutex);
>
> if (length == 0)
> snprintf(n, name_length, "%s[]", array->name);
> @@ -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);
> }
>
> diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
> index ab0b263764..e016835695 100644
> --- a/src/compiler/glsl_types.h
> +++ b/src/compiler/glsl_types.h
> @@ -169,39 +169,6 @@ private:
> }
>
> public:
> - /* Callers of this ralloc-based new need not call delete. It's
> - * easier to just ralloc_free 'mem_ctx' (or any of its ancestors). */
> - static void* operator new(size_t size)
> - {
> - ASSERT_BITFIELD_SIZE(glsl_type, base_type, GLSL_TYPE_ERROR);
> - ASSERT_BITFIELD_SIZE(glsl_type, sampled_type, GLSL_TYPE_ERROR);
> - ASSERT_BITFIELD_SIZE(glsl_type, sampler_dimensionality,
> - GLSL_SAMPLER_DIM_SUBPASS_MS);
> -
> - mtx_lock(&glsl_type::mem_mutex);
> -
> - /* mem_ctx should have been created by the static members */
> - assert(glsl_type::mem_ctx != NULL);
> -
> - void *type;
> -
> - type = ralloc_size(glsl_type::mem_ctx, size);
> - assert(type != NULL);
> -
> - mtx_unlock(&glsl_type::mem_mutex);
> -
> - return type;
> - }
> -
> - /* If the user *does* call delete, that's OK, we will just
> - * ralloc_free in that case. */
> - static void operator delete(void *type)
> - {
> - mtx_lock(&glsl_type::mem_mutex);
> - ralloc_free(type);
> - mtx_unlock(&glsl_type::mem_mutex);
> - }
> -
> /**
> * \name Vector and matrix element counts
> *
> @@ -873,19 +840,16 @@ public:
> return (bool) interface_row_major;
> }
>
> + ~glsl_type();
> +
> private:
>
> - static mtx_t mem_mutex;
> static mtx_t hash_mutex;
>
> /**
> - * ralloc context for all glsl_type allocations
> - *
> - * Set on the first call to \c glsl_type::new.
> + * ralloc context for the type itself.
> */
> - static void *mem_ctx;
> -
> - void init_ralloc_type_ctx(void);
> + void *mem_ctx;
>
> /** Constructor for vector and matrix types */
> glsl_type(GLenum gl_type,
>
More information about the mesa-dev
mailing list