util/hash_table: Don't leak hash_u64_key objects when the entry exists

When an entry exists, _mesa_hash_table_insert() updates the entry with
the new data/key pair, which causes a leak if the key has previously
been dynamically allocated, like is the case for hash_u64_key keys.

One solution to solve that is to do the insertion in two steps: first
_mesa_hash_table_search_pre_hashed(), and if the entry doesn't exist
_mesa_hash_table_insert_pre_hashed(). But approach forces us to do the
double-hashing twice.

Another approach is to extract the logic in hash_table_insert() that's
responsible for the searching and entry allocation into a separate helper
called hash_table_get_entry(), and keep the entry::{key,data} assignment
in hash_table_insert().

This way we can re-use hash_table_get_entry() from
_mesa_hash_table_u64_insert(), and lake sure we free the allocated
key if the entry was already present.

Fixes: 6649b840c340 ("mesa/util: add a hash table wrapper which support 64-bit keys")
Cc: stable
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Yonggang Luo <luoyonggang@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26423>
(cherry picked from commit 5a60fd7b14e9a3045513a4b46ebd109c422c5b2e)

diff --git a/.pick_status.json b/.pick_status.json
index 48c11dd..93584af 100644 (file)
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -24,7 +24,7 @@
         "description": "util/hash_table: Don't leak hash_u64_key objects when the entry exists",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "6649b840c34016b4753e69d4513a8d09da9febb2",
         "notes": null

diff --git a/src/util/hash_table.c b/src/util/hash_table.c
index a832d58..652c898 100644 (file)
--- a/src/util/hash_table.c
+++ b/src/util/hash_table.c
@@ -427,8 +427,7 @@ _mesa_hash_table_rehash(struct hash_table *ht, unsigned new_size_index)
 }
 
 static struct hash_entry *
-hash_table_insert(struct hash_table *ht, uint32_t hash,
-                  const void *key, void *data)
+hash_table_get_entry(struct hash_table *ht, uint32_t hash, const void *key)
 {
    struct hash_entry *available_entry = NULL;
 
@@ -469,11 +468,8 @@ hash_table_insert(struct hash_table *ht, uint32_t hash,
        */
       if (!entry_is_deleted(ht, entry) &&
           entry->hash == hash &&
-          ht->key_equals_function(key, entry->key)) {
-         entry->key = key;
-         entry->data = data;
+          ht->key_equals_function(key, entry->key))
          return entry;
-      }
 
       hash_address += double_hash;
       if (hash_address >= size)
@@ -484,8 +480,6 @@ hash_table_insert(struct hash_table *ht, uint32_t hash,
       if (entry_is_deleted(ht, available_entry))
          ht->deleted_entries--;
       available_entry->hash = hash;
-      available_entry->key = key;
-      available_entry->data = data;
       ht->entries++;
       return available_entry;
    }
@@ -496,6 +490,20 @@ hash_table_insert(struct hash_table *ht, uint32_t hash,
    return NULL;
 }
 
+static struct hash_entry *
+hash_table_insert(struct hash_table *ht, uint32_t hash,
+                  const void *key, void *data)
+{
+   struct hash_entry *entry = hash_table_get_entry(ht, hash, key);
+
+   if (entry) {
+      entry->key = key;
+      entry->data = data;
+   }
+
+   return entry;
+}
+
 /**
  * Inserts the key with the given hash into the table.
  *
@@ -847,7 +855,19 @@ _mesa_hash_table_u64_insert(struct hash_table_u64 *ht, uint64_t key,
          return;
       _key->value = key;
 
-      _mesa_hash_table_insert(ht->table, _key, data);
+      struct hash_entry *entry =
+         hash_table_get_entry(ht->table, key_u64_hash(_key), _key);
+
+      if (!entry) {
+         FREE(_key);
+         return;
+      }
+
+      entry->data = data;
+      if (!entry_is_present(ht->table, entry))
+         entry->key = _key;
+      else
+         FREE(_key);
    }
 }