From: Maciej Piechotka <uzytkownik2@gmail.com>
Date: Tue, 6 Mar 2012 20:52:45 +0000 (+0000)
Subject: Don't resize after deletion from hashtable in iterator, fixes #671327
X-Git-Tag: 0.7.3~8
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=90830d07d0ff0fe3424a71b5c0d8d02e3d679ebb;p=platform%2Fupstream%2Flibgee.git

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.
---

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;
 		}