[ruby-core:28189] [Bug #1535] Hash#merge! Inside Iterator Can Cause RuntimeError

Issue #1535 has been updated by Yusuke Endoh.


Hi,

>   hash = {1 => 2, 3 => 4, 5 => 6}
>   big_hash = {}
>   64.times { |k| big_hash[k.to_s] = k }
>   hash.each { hash.merge!(big_hash) }
>
> This raises a RuntimeError: "hash modified during iteration" on 1.8.6.368 and 1.8.7.72. It runs correctly on 1.9.1.129. 


It raises a RuntimeError on trunk.  I guess it is by accident
for the exception not to occur on 1.9.1.

By hashtable's nature, adding new keys to hash may cause rehash
automatically, and the automatic rehash may cause the exception
during iteration.


For compatibility reason, we cannot prohibit hash modification
during iteration because there are many programs that do so,
(e.g., rbconfig.rb), like this:

  hash.each {|k, v| hash[k] = func(v) }

But I agree with Run Paint Run Run's opinion.  It may lead to
difficult bug to indeterminately fail to add a new key.


So, I propose to permit only updating value of existing key,
and to always prohibit adding a new key:

  hash = { 1=>2, 3=>4, 5=>6 }
  hash.each {|k, v| hash[k] = func(v) }  #=> OK
  hash.each {|k, v| hash[k.to_s] = v }   #=> always exception

This does not cause compatibility problem because this just 
raises exception that has already been occurred indeterminately.
I'll commit the following patch to trunk unless anyone says an
objection.


diff --git a/hash.c b/hash.c
index d49d0ea..51537e9 100644
--- a/hash.c
+++ b/hash.c
@@ -270,6 +270,14 @@ rb_hash_modify(VALUE hash)
 }
 
 static void
+hash_update(VALUE hash, VALUE key)
+{
+    if (RHASH(hash)->iter_lev > 0 && !st_lookup(RHASH(hash)->ntbl, key, 0)) {
+	rb_raise(rb_eRuntimeError, "can't add a new key into hash during iteration");
+    }
+}
+
+static void
 default_proc_arity_check(VALUE proc)
 {
     int n = rb_proc_arity(proc);
@@ -1036,6 +1044,7 @@ VALUE
 rb_hash_aset(VALUE hash, VALUE key, VALUE val)
 {
     rb_hash_modify(hash);
+    hash_update(hash, key);
     if (hash == key) {
 	rb_raise(rb_eArgError, "recursive key for hash");
     }
@@ -1630,6 +1639,7 @@ static int
 rb_hash_update_i(VALUE key, VALUE value, VALUE hash)
 {
     if (key == Qundef) return ST_CONTINUE;
+    hash_update(hash, key);
     st_insert(RHASH(hash)->ntbl, key, value);
     return ST_CONTINUE;
 }
@@ -1641,6 +1651,7 @@ rb_hash_update_block_i(VALUE key, VALUE value, VALUE hash)
     if (rb_hash_has_key(hash, key)) {
 	value = rb_yield_values(3, key, rb_hash_aref(hash, key), value);
     }
+    hash_update(hash, key);
     st_insert(RHASH(hash)->ntbl, key, value);
     return ST_CONTINUE;
 }

-- 
Yusuke Endoh <[e-mail]>
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1535

----------------------------------------
http://redmine.ruby-lang.org