Bug #7330

TestRequire#test_race_exception sometimes fails

Added by Hiroshi Shirosaki over 2 years ago. Updated over 2 years ago.

[ruby-core:49220]
Status:Closed
Priority:Normal
Assignee:Nobuyoshi Nakada
ruby -v:ruby 2.0.0dev (2012-11-11 trunk 37614) [x86_64-darwin12.2.0] Backport:

Description

=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]:
t1:true t2:true.
expected but was
.

$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

Associated revisions

Revision 37696
Added by shirosaki over 2 years ago

st_update passes the key in st_table

  • st.c (st_update): pass the key in st_table so that we can free
    memory of the key in st_table when deleting.
    [Bug #7330]

  • test/-ext-/st/test_update.rb
    (Bug::StTable#test_pass_objects_in_st_table): add a test.

Revision 37696
Added by shirosaki over 2 years ago

st_update passes the key in st_table

  • st.c (st_update): pass the key in st_table so that we can free
    memory of the key in st_table when deleting.
    [Bug #7330]

  • test/-ext-/st/test_update.rb
    (Bug::StTable#test_pass_objects_in_st_table): add a test.

History

#1 Updated by Anonymous over 2 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r37696.
Hiroshi, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


st_update passes the key in st_table

  • st.c (st_update): pass the key in st_table so that we can free
    memory of the key in st_table when deleting.
    [Bug #7330]

  • test/-ext-/st/test_update.rb
    (Bug::StTable#test_pass_objects_in_st_table): add a test.

Also available in: Atom PDF