Mesa (master): glsl: Fix memory leak with known glsl_type instances
Tapani Pälli
tpalli at kemper.freedesktop.org
Wed Mar 7 12:35:24 UTC 2018
Module: Mesa
Branch: master
Commit: fb5825e7ceeb16ac05f870ffe1e5a5daa09e68dd
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=fb5825e7ceeb16ac05f870ffe1e5a5daa09e68dd
Author: Simon Hausmann <simon.hausmann at qt.io>
Date: Wed Feb 14 12:51:11 2018 +0100
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 at lists.freedesktop.org
Signed-off-by: Simon Hausmann <simon.hausmann at qt.io>
Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
---
src/compiler/glsl_types.cpp | 88 ++++++++++++++++++++++-----------------------
src/compiler/glsl_types.h | 46 +++---------------------
2 files changed, 47 insertions(+), 87 deletions(-)
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 3cc5eb0495..a5abeddfc0 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 ab0b263764..c23cf200fa 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,
More information about the mesa-commit
mailing list