BUG/MINOR: dict: race condition fix when inserting dictionary entries.

When checking the result of an ebis_insert() call in an ebtree with unique keys,
if already present, in place of freeing() the old one and return the new one,
rather the correct way is to free the new one, and return the old one. For
this, the __dict_insert() function was folded into dict_insert() as this
significantly simplifies the test of duplicates.

Thanks to Olivier for having reported this bug which came with this one:
"MINOR: dict: Add dictionary new data structure".

diff --git a/src/dict.c b/src/dict.c
index c2580e1793704e95c12dfe6cc0ec9640a295d221..2da564b15f88cbc6926553d897a8a7489f366266 100644 (file)
--- a/src/dict.c
+++ b/src/dict.c
@@ -76,30 +76,13 @@ static struct dict_entry *__dict_lookup(struct dict *d, const char *s)
        return de;
 }
 
-/*
- * Insert <node> node in <root> ebtree, deleting any already existing node with
- * the same value.
- */
-static struct ebpt_node *__dict_insert(struct eb_root *root, struct ebpt_node *node)
-{
-       struct ebpt_node *n;
-
-       n = ebis_insert(root, node);
-       if (n != node) {
-               ebpt_delete(n);
-               free_dict_entry(container_of(n, struct dict_entry, value));
-               ebis_insert(root, node);
-       }
-
-       return node;
-}
-
 /*
  * Insert an entry in <d> dictionary with <s> as value. *
  */
 struct dict_entry *dict_insert(struct dict *d, char *s)
 {
        struct dict_entry *de;
+       struct ebpt_node *n;
 
        HA_RWLOCK_RDLOCK(DICT_LOCK, &d->rwlock);
        de = __dict_lookup(d, s);
@@ -112,8 +95,12 @@ struct dict_entry *dict_insert(struct dict *d, char *s)
                return NULL;
 
        HA_RWLOCK_WRLOCK(DICT_LOCK, &d->rwlock);
-       __dict_insert(&d->values, &de->value);
+       n = ebis_insert(&d->values, &de->value);
        HA_RWLOCK_WRUNLOCK(DICT_LOCK, &d->rwlock);
+       if (n != &de->value) {
+               free_dict_entry(de);
+               de = container_of(n, struct dict_entry, value);
+       }
 
        return de;
 }