[PATCH] Test for weak pairs in hash-for-each

[PATCH] Test for weak pairs in hash-for-each

[David Thompson]

[-- Attachment #1: Type: text/plain, Size: 131 bytes --]

Hello all,

This patch fixes a segfault that occurs when iterating over a weak hash
table with deleted weak pairs.

WDYT?

- Dave


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Test-for-deleted-weak-pairs-in-hash-for-each.patch --]
[-- Type: text/x-diff, Size: 1660 bytes --]

From 531c773dae023f15e0719d76a4352064e3681a7b Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Sat, 8 Mar 2014 17:15:52 -0500
Subject: [PATCH] Test for deleted weak pairs in hash-for-each.

* libguile/hashtab.c (hash-for-each): Test for deleted weak pairs.
* test-suite/tests/hash.test: Add test case.
---
 libguile/hashtab.c         |  4 +++-
 test-suite/tests/hash.test | 12 ++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/libguile/hashtab.c b/libguile/hashtab.c
index 9107ce5..c5277c1 100644
--- a/libguile/hashtab.c
+++ b/libguile/hashtab.c
@@ -1464,7 +1464,9 @@ scm_internal_hash_for_each_handle (scm_t_hash_handle_fn fn, void *closure,
 	  handle = SCM_CAR (ls);
 	  if (!scm_is_pair (handle))
 	    SCM_WRONG_TYPE_ARG (SCM_ARG3, buckets);
-	  fn (closure, handle);
+          if (!SCM_HASHTABLE_WEAK_P (table) ||
+              !SCM_WEAK_PAIR_DELETED_P (handle))
+            fn (closure, handle);
 	  ls = SCM_CDR (ls);
 	}
     }
diff --git a/test-suite/tests/hash.test b/test-suite/tests/hash.test
index 64d10bb..4c21d71 100644
--- a/test-suite/tests/hash.test
+++ b/test-suite/tests/hash.test
@@ -347,3 +347,15 @@
 
     (pass-if (equal? 2 (hash-count (lambda (k v)
                                      (string? v)) table)))))
+
+;;;
+;;; weak key hash table
+;;;
+
+(with-test-prefix "weak key hash table"
+  (pass-if "hash-for-each after gc"
+    (let ((table (make-weak-key-hash-table)))
+      (hashq-set! table (list 'foo) 'bar)
+      (gc)
+      ;; Iterate over deleted weak ref without crashing.
+      (unspecified? (hash-for-each (lambda (key value) key) table)))))
-- 
1.8.5.3

Re: [PATCH] Test for weak pairs in hash-for-each

[David Thompson]

[-- Attachment #1: Type: text/plain, Size: 123 bytes --]

David Thompson <dthompson2@worcester.edu> writes:

Hello,

Updated patch attached that fixes a small style issue.

- Dave


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Test-for-deleted-weak-pairs-in-hash-for-each.patch --]
[-- Type: text/x-diff, Size: 1660 bytes --]

From 1bbf073905bc12f80b0a32fc6311163a0b0ab849 Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Sat, 8 Mar 2014 17:15:52 -0500
Subject: [PATCH] Test for deleted weak pairs in hash-for-each.

* libguile/hashtab.c (hash-for-each): Test for deleted weak pairs.
* test-suite/tests/hash.test: Add test case.
---
 libguile/hashtab.c         |  4 +++-
 test-suite/tests/hash.test | 12 ++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/libguile/hashtab.c b/libguile/hashtab.c
index 9107ce5..44db051 100644
--- a/libguile/hashtab.c
+++ b/libguile/hashtab.c
@@ -1464,7 +1464,9 @@ scm_internal_hash_for_each_handle (scm_t_hash_handle_fn fn, void *closure,
 	  handle = SCM_CAR (ls);
 	  if (!scm_is_pair (handle))
 	    SCM_WRONG_TYPE_ARG (SCM_ARG3, buckets);
-	  fn (closure, handle);
+          if (!SCM_HASHTABLE_WEAK_P (table)
+              || !SCM_WEAK_PAIR_DELETED_P (handle))
+            fn (closure, handle);
 	  ls = SCM_CDR (ls);
 	}
     }
diff --git a/test-suite/tests/hash.test b/test-suite/tests/hash.test
index 64d10bb..4c21d71 100644
--- a/test-suite/tests/hash.test
+++ b/test-suite/tests/hash.test
@@ -347,3 +347,15 @@
 
     (pass-if (equal? 2 (hash-count (lambda (k v)
                                      (string? v)) table)))))
+
+;;;
+;;; weak key hash table
+;;;
+
+(with-test-prefix "weak key hash table"
+  (pass-if "hash-for-each after gc"
+    (let ((table (make-weak-key-hash-table)))
+      (hashq-set! table (list 'foo) 'bar)
+      (gc)
+      ;; Iterate over deleted weak ref without crashing.
+      (unspecified? (hash-for-each (lambda (key value) key) table)))))
-- 
1.8.5.3

Re: [PATCH] Test for weak pairs in hash-for-each

[Mark H Weaver]

David Thompson <dthompson2@worcester.edu> writes:

> * libguile/hashtab.c (hash-for-each): Test for deleted weak pairs.

Instead of (hash-for-each), it should be
(scm_internal_hash_for_each_handle).

I went ahead and pushed it, with that change.

   Thanks!
     Mark

Re: [PATCH] Test for weak pairs in hash-for-each

[Thompson, David]

On Wed, Mar 12, 2014 at 2:45 AM, Mark H Weaver <mhw@netris.org> wrote:
> David Thompson <dthompson2@worcester.edu> writes:
>
>> * libguile/hashtab.c (hash-for-each): Test for deleted weak pairs.
>
> Instead of (hash-for-each), it should be
> (scm_internal_hash_for_each_handle).
>
> I went ahead and pushed it, with that change.
>
>    Thanks!
>      Mark

Ah, sorry.  I always mess up something like that.

Thanks for taking care of it.

- Dave