From 31fb2b846b1d0dfde79dfb81734f71ce0d014a47 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 14 Feb 2018 12:51:11 +0100 Subject: [PATCH] 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. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104884 Signed-off-by: Simon Hausmann --- src/compiler/glsl_types.cpp | 79 +++++++++++++++++++-------------------------- src/compiler/glsl_types.h | 44 +++---------------------- 2 files changed, 37 insertions(+), 86 deletions(-) diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp index 3cc5eb0495..2c35b42e54 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 @@ -419,37 +401,41 @@ const glsl_type *glsl_type::get_scalar_type() const void _mesa_glsl_release_types(void) { + const auto hash_free_type_function = [](struct hash_entry *entry) { + delete (glsl_type *)entry->data; + }; + /* Should only be called during atexit (either when unloading shared * object, or if process terminates), so no mutex-locking should be * necessary. */ if (glsl_type::array_types != NULL) { - _mesa_hash_table_destroy(glsl_type::array_types, NULL); + _mesa_hash_table_destroy(glsl_type::array_types, [](struct hash_entry *entry){ + ::free((void *)entry->key); + delete (glsl_type *)entry->data; + }); 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 +459,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 +957,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..90a4654790 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 * @@ -875,17 +842,12 @@ public: 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, @@ -916,6 +878,8 @@ private: /** Constructor for subroutine types */ glsl_type(const char *name); + ~glsl_type(); + /** Hash table containing the known array types. */ static struct hash_table *array_types; -- 2.14.1