Track where microdescs are referenced to prevent free errs

On IRC, wanoskarnet notes that if we ever do microdesc_free() on a
microdesc that's in the nodelist, we're in trouble.  Also, we're in
trouble if we free one that's still in the microdesc_cache map.

This code adds a flag to microdesc_t to note where the microdesc is
referenced from, and checks those flags from microdesc_free().  I
don't believe we have any errors here now, but if we introduce some
later, let's log and recover from them rather than introducing
heisenbugs later on.

Addresses bug 3153.

1 files changed, 40 insertions, 1 deletions

diff --git a/src/or/microdesc.c b/src/or/microdesc.c
index 717488bded..ba4532ef64 100644
--- a/src/or/microdesc.c
+++ b/src/or/microdesc.c
@@ -238,6 +238,7 @@ microdescs_add_list_to_cache(microdesc_cache_t *cache,
     md->no_save = no_save;
 
     HT_INSERT(microdesc_map, &cache->map, md);
+    md->held_in_map = 1;
     smartlist_add(added, md);
     ++cache->n_seen;
     cache->total_len_seen += md->bodylen;
@@ -266,6 +267,7 @@ microdesc_cache_clear(microdesc_cache_t *cache)
   for (entry = HT_START(microdesc_map, &cache->map); entry; entry = next) {
     microdesc_t *md = *entry;
     next = HT_NEXT_RMV(microdesc_map, &cache->map, entry);
+    md->held_in_map = 0;
     microdesc_free(md);
   }
   HT_CLEAR(microdesc_map, &cache->map);
@@ -354,6 +356,7 @@ microdesc_cache_clean(microdesc_cache_t *cache, time_t cutoff, int force)
       ++dropped;
       victim = *mdp;
       mdp = HT_NEXT_RMV(microdesc_map, &cache->map, mdp);
+      victim->held_in_map = 0;
       bytes_dropped += victim->bodylen;
       microdesc_free(victim);
     } else {
@@ -502,7 +505,43 @@ microdesc_free(microdesc_t *md)
 {
   if (!md)
     return;
-  /* Must be removed from hash table! */
+
+  /* Make sure that the microdesc was really removed from the appropriate data
+     structures. */
+  if (md->held_in_map) {
+    microdesc_cache_t *cache = get_microdesc_cache();
+    microdesc_t *md2 = HT_FIND(microdesc_map, &cache->map, md);
+    if (md2 == md) {
+      log_warn(LD_BUG, "microdesc_free() called, but md was still in "
+               "microdesc_map");
+      HT_REMOVE(microdesc_map, &cache->map, md);
+    } else {
+      log_warn(LD_BUG, "microdesc_free() called with held_in_map set, but "
+               "microdesc was not in the map.");
+    }
+    tor_fragile_assert();
+  }
+  if (md->held_by_node) {
+    int found=0;
+    const smartlist_t *nodes = nodelist_get_list();
+    SMARTLIST_FOREACH(nodes, node_t *, node, {
+        if (node->md == md) {
+          ++found;
+          node->md = NULL;
+        }
+      });
+    if (found) {
+      log_warn(LD_BUG, "microdesc_free() called, but md was still referenced "
+               "%d node(s)", found);
+    } else {
+      log_warn(LD_BUG, "microdesc_free() called with held_by_node set, but "
+               "md was not refrenced by any nodes");
+    }
+    tor_fragile_assert();
+  }
+  //tor_assert(md->held_in_map == 0);
+  //tor_assert(md->held_by_node == 0);
+
   if (md->onion_pkey)
     crypto_free_pk_env(md->onion_pkey);
   if (md->body && md->saved_location != SAVED_IN_CACHE)