From: Simon Hausmann Date: Wed, 14 Feb 2018 11:51:11 +0000 (+0100) Subject: glsl: Fix memory leak with known glsl_type instances X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=fb5825e7ceeb16ac05f870ffe1e5a5daa09e68dd;p=mesa.git glsl: Fix memory leak with known glsl_type instances 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) (+keep ASSERT_BITFIELD_SIZE, modify dummy ctor to initialize mem_ctx) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104884 Cc: mesa-stable@lists.freedesktop.org Signed-off-by: Simon Hausmann Signed-off-by: Tapani Pälli Reviewed-by: Emil Velikov Reviewed-by: Kenneth Graunke --- diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp index 3cc5eb0495c..a5abeddfc0f 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,17 @@ 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); + 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); + + 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 +78,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 +100,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 +113,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 +128,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 +140,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 +153,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 +170,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 +180,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 +403,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 +422,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 +468,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 +966,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 ab0b2637649..c23cf200fa1 100644 --- a/src/compiler/glsl_types.h +++ b/src/compiler/glsl_types.h @@ -163,45 +163,12 @@ struct glsl_type { unsigned interface_row_major:1; private: - glsl_type() + glsl_type() : mem_ctx(NULL) { // Dummy constructor, just for the sake of ASSERT_BITFIELD_SIZE. } 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,