From: "h.shirosaki (Hiroshi Shirosaki)" <h.shirosaki@...>
Date: 2012-11-11T21:41:08+09:00
Subject: [ruby-core:49220] [ruby-trunk - Bug #7330][Open] TestRequire#test_race_exception sometimes fails


Issue #7330 has been reported by h.shirosaki (Hiroshi Shirosaki).

----------------------------------------
Bug #7330: TestRequire#test_race_exception sometimes fails
https://bugs.ruby-lang.org/issues/7330

Author: h.shirosaki (Hiroshi Shirosaki)
Status: Open
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: 
Target version: 2.0.0
ruby -v: ruby 2.0.0dev (2012-11-11 trunk 37614) [x86_64-darwin12.2.0]


=begin

I noticed TestRequire#test_race_exception sometimes fails on RubyCI recently.

http://www.rubyist.net/~akr/chkbuild/debian/ruby-trunk/log/20121111T095100Z.log.html.gz

I was able to reproduce that on my local.

With this patch:

 diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb
 index 36d7c78..5defdf5 100644
 --- a/test/ruby/test_require.rb
 +++ b/test/ruby/test_require.rb
 @@ -412,6 +412,9 @@ class TestRequire < Test::Unit::TestCase
      assert_nothing_raised(ThreadError, bug5754) {t1.join}
      assert_nothing_raised(ThreadError, bug5754) {t2.join}
  
 +    p "t1:#{t1_res} t2:#{t2_res}"
 +    p scratch
 +    p $"[-2, 2]
      assert_equal(true, (t1_res ^ t2_res), bug5754 + " t1:#{t1_res} t2:#{t2_res}")
      assert_equal([:pre, :post], scratch, bug5754)
    ensure


Run make test-all repeatedly. After a while, I got a failure.

 $ echo; while [ $? = 0 ]; do LANG=C make test-all TESTS='-q rubygems/test_gem.rb ruby/test_require.rb' RUBYOPT=-w; done
 (snip)
 [ 93/108] TestRequire#test_race_exception/Users/hiroshi/src/ruby/lib/rubygems/custom_require.rb:55: warning: loading in progress, circular require considered harmful - /var/folders/75/_4hkzm9d33l7gglj71f1m_p00000gn/T/bug575420121111-3699-18fa4nd.rb
 	from /Users/hiroshi/src/ruby/test/ruby/test_require.rb:403:in `block in test_race_exception'
 	from /Users/hiroshi/src/ruby/lib/rubygems/custom_require.rb:55:in `require' 
 	from /Users/hiroshi/src/ruby/lib/rubygems/custom_require.rb:55:in `require'
 "t1:true t2:true"
 [:pre, :post, :post]
 ["\x00\x00\x00\x00\x00\x00\x00\xC0\x00\x00\x00\x00\x00\x00\x00\xC0\x06\x00hkzm9d33l7gglj71f1m_p00000gn/T/bug575420121111-3699-18fa4nd.rb", 
 "/var/folders/75/_4hkzm9d33l7gglj71f1m_p00000gn/T/bug575420121111-3699-18fa4nd.rb"]
 = 0.01 s
 1) Failure:
 test_race_exception(TestRequire) [/Users/hiroshi/src/ruby/test/ruby/test_require.rb:418]:
 [ruby-core:41618] t1:true t2:true.
 <true> expected but was
 <false>.


$LOADED_FEATURES has a broken string as the following.

"\x00\x00\x00\x00\x00\x00\x00\xC0\x00\x00\x00\x00\x00\x00\x00\xC0\x06\x00hkzm9d33l7gglj71f1m_p00000gn/T/bug575420121111-3699-18fa4nd.rb"

It seems that `ftptr` which equals to `RSTRING_PTR(path)` while loading is freed by xfree() in load_unlock(). It causes memory corruption.


load.c:L897 load_unlock(ftptr, !state);

https://github.com/ruby/ruby/blob/trunk/load.c#L897

load.c:L675 xfree((char *)*key);

https://github.com/ruby/ruby/blob/trunk/load.c#L675


Should st_update() pass the key stored in `st_table` to the callback function instead of the key argument of st_update()?

Here is a patch. And this fixes above failure on my local.

 diff --git a/st.c b/st.c
 index 91fcb7b..c5550bc 100644
 --- a/st.c
 +++ b/st.c
 @@ -843,6 +843,7 @@ st_update(st_table *table, st_data_t key, st_update_callback_func *func, st_data
      if (table->entries_packed) {
  	st_index_t i = find_packed_index(table, hash_val, key);
  	if (i < table->real_entries) {
 +	    key = PKEY(table, i);
  	    value = PVAL(table, i);
  	    existing = 1;
  	}
 @@ -871,6 +872,7 @@ st_update(st_table *table, st_data_t key, st_update_callback_func *func, st_data
      FIND_ENTRY(table, ptr, hash_val, bin_pos);
  
      if (ptr != 0) {
 +	key = ptr->key;
  	value = ptr->record;
  	existing = 1;
      }

=end


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