Merge lp:~ken-vandine/gwibber/lp_959068 into lp:gwibber

[Ken VanDine]

Defer calling unset on the seen HashMap until after we are done iterating it,
it will resize making the iterator invalid. (LP: #959068)

This is fixed in the 0.7 series of libgee https://bugzilla.gnome.org/show_bug.cgi?id=671327

[Michael Terry (mterry)]

As discussed on IRC, the change seems harmless. Might warrant a comment in code, because otherwise that coding pattern is odd and liable to be removed accidentally in future.

review: Approve

[Ken VanDine (ken-vandine)]

Two test cases, the first one crashes the second one doesn't. Should validate the work around.

/*
 * build with:
 * valac --pkg gee-1.0 gee-test1.vala -o gee-test1
 */

static int main(string[] args)
{
  var map = new Gee.HashMap <string, string?> ();
  map["one"] = "one";
  map["two"] = "two";
  map["three"] = "three";
  map["four"] = "four";

  print ("map size is %d\n", map.size);

  foreach (var v in map.entries)
  {
    print ("Found %s\n", v.value);
    map.unset(v.key);
  }

  print ("map size is %d\n", map.size);

  return 0;
}
ken@trabajo:~/src/gwibber/lp_959068$ cat examples/gee-test2.vala
/*
 * build with:
 * valac --pkg gee-1.0 gee-test2.vala -o gee-test2
 */

static int main(string[] args)
{
  var map = new Gee.HashMap <string, string?> ();
  map["one"] = "one";
  map["two"] = "two";
  map["three"] = "three";
  map["four"] = "four";

  print ("map size is %d\n", map.size);

  var to_remove = new GLib.List <string> ();

  foreach (var v in map.entries)
  {
    print ("Found %s\n", v.value);
    to_remove.append (v.key);
  }
  foreach (var v in to_remove)
    map.unset(v);

  print ("map size is %d\n", map.size);

  return 0;
}

[Michael Terry (mterry)]

Ah, that makes sense! Since order doesn't matter here, you might want to use prepend instead of append (better performance), but no big deal.

review: Approve

[Ken VanDine (ken-vandine)]

great, will do!

=== modified file 'libgwibber/streams.vala'
--- libgwibber/streams.vala	2012-03-19 13:35:30 +0000
+++ libgwibber/streams.vala	2012-04-04 20:31:26 +0000
@@ -213,13 +213,21 @@
                 model = create_model ();
               }
               model.row_removed.connect((_m, _i) => {
+                var to_remove = new GLib.List <string> ();
                 foreach (var v in seen.entries)
                 {
                   if (v.value == _i)
                   {
-                    seen.unset(v.key);
+                    /* defer calling unset until after we are done iterating 
+                     * the HashMap, it will resize making the iterator invalid
+                     * This is fixed in the 0.7 series of libgee
+                     * https://bugzilla.gnome.org/show_bug.cgi?id=671327
+                     */
+                    to_remove.append (v.key);
                   }
                 }
+                foreach (var v in to_remove)
+                  seen.unset(v);
               });
               Idle.add(() => {
                 refresh_model_async.begin ();