[libgee] Fix use-after-frees caused by weak pointer issues

[Maciej Marcin Piechotka <mpiechotka src gnome org>]

commit 707456e37bded83b28d8bb8e21bab80c02359611
Author: Ole André Vadla Ravnås <oleavr gmail com>
Date:   Tue Oct 4 17:11:00 2016 +0000

    Fix use-after-frees caused by weak pointer issues
    
    Same issue in HashMap and TreeMap:
    
    ```
    ==3251==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000000870 at pc 0x000108be666b bp 
0x7fff571e62b0 sp 0x7fff571e62a8
    WRITE of size 8 at 0x604000000870 thread T0
        #0 0x108be666a in g_nullify_pointer gutils.c:2051
        #1 0x108b8c906 in weak_refs_notify gobject.c:2638
        #2 0x108bbb17c in g_data_set_internal gdataset.c:407
        #3 0x108b887db in g_object_unref gobject.c:3148
        #4 0x108a4b0ec in map_tests_test_entry_weak_pointer_lifetime testmap.c:1358
    
    0x604000000870 is located 32 bytes inside of 40-byte region [0x604000000850,0x604000000878)
    freed by thread T0 here:
        #0 0x1090f0e29 in wrap_free (libclang_rt.asan_osx_dynamic.dylib+0x4ae29)
        #1 0x108ace566 in gee_hash_map_unset_helper hashmap.c:1692
        #2 0x108acc534 in gee_hash_map_real_unset hashmap.c:1520
        #3 0x108a4b0df in map_tests_test_entry_weak_pointer_lifetime testmap.c:1357
    
    previously allocated by thread T0 here:
        #0 0x1090f0c60 in wrap_malloc (libclang_rt.asan_osx_dynamic.dylib+0x4ac60)
        #1 0x108bce848 in g_malloc gmem.c:95
        #2 0x108bd6585 in g_slice_alloc gslice.c:1012
        #3 0x108bd6bee in g_slice_alloc0 gslice.c:1038
        #4 0x108acdc27 in gee_hash_map_node_new hashmap.c:2084
        #5 0x108acc277 in gee_hash_map_real_set hashmap.c:1494
        #6 0x108a4b032 in map_tests_test_entry_weak_pointer_lifetime testmap.c:1311
    
    https://bugzilla.gnome.org/show_bug.cgi?id=772418

 gee/hashmap.vala   |    6 ++++++
 gee/treemap.vala   |    6 ++++++
 tests/testmap.vala |   16 +++++++++++++++-
 3 files changed, 27 insertions(+), 1 deletions(-)
---
diff --git a/gee/hashmap.vala b/gee/hashmap.vala
index b1767e3..83243a7 100644
--- a/gee/hashmap.vala
+++ b/gee/hashmap.vala
@@ -341,6 +341,12 @@ public class Gee.HashMap<K,V> : Gee.AbstractMap<K,V> {
                        key_hash = hash;
                         entry = null;
                }
+
+               ~Node () {
+                       if (entry != null) {
+                               entry.remove_weak_pointer ((void**) (&entry));
+                       }
+               }
        }
 
        private class Entry<K,V> : Map.Entry<K,V> {
diff --git a/gee/treemap.vala b/gee/treemap.vala
index d151d27..5c1c4cd 100644
--- a/gee/treemap.vala
+++ b/gee/treemap.vala
@@ -507,6 +507,12 @@ public class Gee.TreeMap<K,V> : Gee.AbstractBidirSortedMap<K,V> {
                        }
                }
 
+               ~Node () {
+                       if (entry != null) {
+                               entry.remove_weak_pointer ((void**) (&entry));
+                       }
+               }
+
                public void flip () {
                        color = color.flip ();
                        if (left != null) {
diff --git a/tests/testmap.vala b/tests/testmap.vala
index 5a63cf8..468d477 100644
--- a/tests/testmap.vala
+++ b/tests/testmap.vala
@@ -35,6 +35,7 @@ public abstract class MapTests : Gee.TestCase {
                add_test ("[Map] keys", test_keys);
                add_test ("[Map] values", test_values);
                add_test ("[Map] entries", test_entries);
+               add_test ("[Map] entry weak pointer lifetime", test_entry_weak_pointer_lifetime);
                add_test ("[Map] set all", test_set_all);
                add_test ("[Map] unset all", test_unset_all);
                add_test ("[Map] has all", test_has_all);
@@ -283,7 +284,20 @@ public abstract class MapTests : Gee.TestCase {
                entries = test_map.entries;
                assert (entries.size == 0);
        }
-       
+
+       private void test_entry_weak_pointer_lifetime () {
+               // Issue was reproducible with AddressSanitizer and G_SLICE=always-malloc
+
+               test_map["1337"] = "Badger";
+
+               foreach (var entry in test_map.entries) {
+                       if (entry.value == "Badger") {
+                               test_map.unset (entry.key);
+                               break;
+                       }
+               }
+       }
+
        public void test_clear () {
                test_map.set ("one", "value_of_one");
                test_map.set ("two", "value_of_two");