hash: bugfix: spurious retention in weak processing.

The algorithm for weak processing is not correct. In
hash_mark, we must must simply not mark any of the entries,
keys or values, of a weak table regardless of what type of
weak table it is. If we do that, we cause spurious retention
in situations that the keys and values have some kind of link
together other than through the table. For instance, suppose
keys are weak, but values happen to have references to keys.
If we mark the values, we mark the keys and nothing will
expire from the table.

Such a situation happens in stream_parser_hash, which
associates streams with parsers, and has weak keys. Parsers
have references to streams.  So entries in the hash never
expire. Any stream that gets a parser is retained forever.

The weak hashes used for binding in eval.c (top_vb, ...) are
also affected, because the key is some symbol <sym> and the
value is (<sym> . <val>). The key is weak, but the value
references the sym. So these hashes also will not expire the
keys: unreachable variable bindings will stick around.

* hash.c (hash_mark): If a hash table has weak keys,
values, or both, then only mark its vector if the count is
zero. If it has one or more entries, we just add it to the
reachable_weak_hashes list to be processed in do_weak_tables.

1 files changed, 10 insertions, 32 deletions

diff --git a/hash.c b/hash.c
index 8a298de2..8c2aee2d 100644
--- a/hash.c
+++ b/hash.c
@@ -591,7 +591,6 @@ static void hash_print_op(val hash, val out, val pretty, struct strm_ctx *ctx)
 static void hash_mark(val hash)
 {
   struct hash *h = coerce(struct hash *, hash->co.handle);
-  cnum i;
 
   gc_mark(h->userdata);
 
@@ -606,39 +605,18 @@ static void hash_mark(val hash)
     gc_mark(h->table);
     break;
   case hash_weak_keys:
-    /* Keys are weak: mark the values only. */
-    for (i = 0; i < h->modulus; i++) {
-      val chain = h->table->v.vec[i];
-      val iter;
-
-      for (iter = chain; iter != nil; iter = us_cdr(iter)) {
-        val entry = us_car(iter);
-        gc_mark(us_cdr(entry));
-      }
-    }
-    h->next = reachable_weak_hashes;
-    reachable_weak_hashes = h;
-    break;
   case hash_weak_vals:
-    /* Values are weak: mark the keys only. */
-
-    for (i = 0; i < h->modulus; i++) {
-      val chain = h->table->v.vec[i];
-      val iter;
-
-      for (iter = chain; iter != nil; iter = us_cdr(iter)) {
-        val entry = us_car(iter);
-        gc_mark(us_car(entry));
-      }
-    }
-    h->next = reachable_weak_hashes;
-    reachable_weak_hashes = h;
-    break;
   case hash_weak_both:
-    /* Values and keys are weak: don't mark anything. */
-    h->next = reachable_weak_hashes;
-    reachable_weak_hashes = h;
-    break;
+    /* If the hash is weak, we don't touch it at this time,
+       but add it to the list of reachable weak hashes,
+       unless it is empty. */
+    if (h->count > 0) {
+      h->next = reachable_weak_hashes;
+      reachable_weak_hashes = h;
+    } else {
+      gc_mark(h->table);
+      break;
+    }
   }
 }