From 2187975e391efd3e4b8078f2ce477d62856edc20 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Mon, 30 May 2011 11:33:10 +0200 Subject: [PATCH] fix subtle and bad scm_internal_hash_fold bug for weak tables * libguile/hashtab.c (scm_internal_hash_fold): Don't try to unlink deleted weak pairs. Our previous code was buggy (`prev' should have only been updated in the case of a successful traversal), but more than that, we're not in the alloc lock. Thanks very much to Michael Wells for the report, and the debugging! --- libguile/hashtab.c | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/libguile/hashtab.c b/libguile/hashtab.c index d70799380..440738c11 100644 --- a/libguile/hashtab.c +++ b/libguile/hashtab.c @@ -1377,40 +1377,20 @@ scm_internal_hash_fold (scm_t_hash_fold_fn fn, void *closure, n = SCM_SIMPLE_VECTOR_LENGTH (buckets); for (i = 0; i < n; ++i) { - SCM prev, ls; + SCM ls, handle; - for (prev = SCM_BOOL_F, ls = SCM_SIMPLE_VECTOR_REF (buckets, i); - !scm_is_null (ls); - prev = ls, ls = SCM_CDR (ls)) + for (ls = SCM_SIMPLE_VECTOR_REF (buckets, i); !scm_is_null (ls); + ls = SCM_CDR (ls)) { - SCM handle; - - if (!scm_is_pair (ls)) - SCM_WRONG_TYPE_ARG (SCM_ARG3, buckets); - handle = SCM_CAR (ls); - if (!scm_is_pair (handle)) - SCM_WRONG_TYPE_ARG (SCM_ARG3, buckets); - if (SCM_HASHTABLE_WEAK_P (table)) - { - if (SCM_WEAK_PAIR_DELETED_P (handle)) - { - /* We hit a weak pair whose car/cdr has become - unreachable: unlink it from the bucket. */ - if (scm_is_true (prev)) - SCM_SETCDR (prev, SCM_CDR (ls)); - else - SCM_SIMPLE_VECTOR_SET (buckets, i, SCM_CDR (ls)); - - /* Update the item count. */ - SCM_HASHTABLE_DECREMENT (table); - - continue; - } - } - - result = fn (closure, SCM_CAR (handle), SCM_CDR (handle), result); + if (SCM_HASHTABLE_WEAK_P (table) && SCM_WEAK_PAIR_DELETED_P (handle)) + /* Don't try to unlink this weak pair, as we're not within + the allocation lock. Instead rely on + vacuum_weak_hash_table to do its job. */ + continue; + else + result = fn (closure, SCM_CAR (handle), SCM_CDR (handle), result); } }