[libgee] Don't resize after deletion from hashtable in iterator, fixes #671327



commit 90830d07d0ff0fe3424a71b5c0d8d02e3d679ebb
Author: Maciej Piechotka <uzytkownik2 gmail com>
Date:   Tue Mar 6 20:52:45 2012 +0000

    Don't resize after deletion from hashtable in iterator, fixes #671327
    
    Depending on sizes of array and hash function resize might alter
    the iteration order. It meant that some elements might not be visited
    and some might be visited twice.

 gee/hashmap.vala |   49 ++++++++++++++++++++++++++++---------------------
 gee/hashset.vala |   37 ++++++++++++++++++++++---------------
 2 files changed, 50 insertions(+), 36 deletions(-)
---
diff --git a/gee/hashmap.vala b/gee/hashmap.vala
index d71d964..73fa68d 100644
--- a/gee/hashmap.vala
+++ b/gee/hashmap.vala
@@ -207,26 +207,11 @@ public class Gee.HashMap<K,V> : Gee.AbstractMap<K,V> {
 	 * { inheritDoc}
 	 */
 	public override bool unset (K key, out V? value = null) {
-		Node<K,V>** node = lookup_node (key);
-		if (*node != null) {
-			Node<K,V> next = (owned) (*node)->next;
-
-			value = (owned) (*node)->value;
-
-			(*node)->key = null;
-			(*node)->value = null;
-			delete *node;
-
-			*node = (owned) next;
-
-			_nnodes--;
-			resize ();
-			_stamp++;
-			return true;
-		} else {
-			value = null;
+		bool b = unset_helper (key, out value);
+		if(b) {
+			resize();
 		}
-		return false;
+		return b;
 	}
 
 	/**
@@ -253,7 +238,29 @@ public class Gee.HashMap<K,V> : Gee.AbstractMap<K,V> {
 		return new MapIterator<K,V> (this);
 	}
 
-	private void resize () {
+	private inline bool unset_helper (K key, out V? value = null) {
+		Node<K,V>** node = lookup_node (key);
+		if (*node != null) {
+			Node<K,V> next = (owned) (*node)->next;
+
+			value = (owned) (*node)->value;
+
+			(*node)->key = null;
+			(*node)->value = null;
+			delete *node;
+
+			*node = (owned) next;
+
+			_nnodes--;
+			_stamp++;
+			return true;
+		} else {
+			value = null;
+		}
+		return false;
+	}
+
+	private inline void resize () {
 		if ((_array_size >= 3 * _nnodes && _array_size >= MIN_SIZE) ||
 		    (3 * _array_size <= _nnodes && _array_size < MAX_SIZE)) {
 			int new_array_size = (int) SpacedPrimes.closest (_nnodes);
@@ -590,7 +597,7 @@ public class Gee.HashMap<K,V> : Gee.AbstractMap<K,V> {
 			assert (_stamp == _map._stamp);
 			assert (_node != null);
 			has_next ();
-			_map.unset (_node.key);
+			_map.unset_helper (_node.key);
 			_node = null;
 			_stamp = _map._stamp;
 		}
diff --git a/gee/hashset.vala b/gee/hashset.vala
index 3e8ae01..9272aa2 100644
--- a/gee/hashset.vala
+++ b/gee/hashset.vala
@@ -140,22 +140,11 @@ public class Gee.HashSet<G> : AbstractSet<G> {
 	 * { inheritDoc}
 	 */
 	public override bool remove (G key) {
-		Node<G>** node = lookup_node (key);
-		if (*node != null) {
-			assert (*node != null);
-			Node<G> next = (owned) (*node)->next;
-
-			(*node)->key = null;
-			delete *node;
-
-			*node = (owned) next;
-
-			_nnodes--;
+		bool b = remove_helper(key);
+		if(b) {
 			resize ();
-			_stamp++;
-			return true;
 		}
-		return false;
+		return b;
 	}
 
 	/**
@@ -174,6 +163,24 @@ public class Gee.HashSet<G> : AbstractSet<G> {
 		resize ();
 	}
 
+	private inline bool remove_helper (G key) {
+		Node<G>** node = lookup_node (key);
+		if (*node != null) {
+			assert (*node != null);
+			Node<G> next = (owned) (*node)->next;
+
+			(*node)->key = null;
+			delete *node;
+
+			*node = (owned) next;
+
+			_nnodes--;
+			_stamp++;
+			return true;
+		}
+		return false;
+	}
+
 	private void resize () {
 		if ((_array_size >= 3 * _nnodes && _array_size >= MIN_SIZE) ||
 		    (3 * _array_size <= _nnodes && _array_size < MAX_SIZE)) {
@@ -262,7 +269,7 @@ public class Gee.HashSet<G> : AbstractSet<G> {
 			assert (_stamp == _set._stamp);
 			assert (_node != null);
 			has_next ();
-			_set.remove (_node.key);
+			_set.remove_helper (_node.key);
 			_node = null;
 			_stamp = _set._stamp;
 		}



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]