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 <simon.hausmann@qt.io>
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 3cc5eb0495c8317b4e0275a11e8bcc681dac9ebb..a5abeddfc0f81cb058419b3248c174fc8957750f 100644 (file)
--- 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 ab0b26376492d408dd9daf9d3ba9260f3ab959c7..c23cf200fa1e7c343bdda9532046f92d54def09b 100644 (file)
--- 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,