Issue #5768 has been updated by Yui NARUSE.
追いかけた結果、どうも lock に用いている rb_barrier_release/rb_barrier_destroy や
rb_mutex_lock のマルチスレッド対応がいまいちだったようです。
前者は rb_mutex_unlock を外した直後から、待っていた他のスレッドが動き始めるのでもはや mutex->cond_waiting
は古い情報となってしまう点、
後者は GVL_UNLOCK_BEGIN() した直後から、メインのスレッドが動いてしまう可能性があるのがダメですね。
diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb
index 9186a6f…f1d8d12 100644
— a/test/ruby/test_require.rb
+++ b/test/ruby/test_require.rb
@@ -350,9 +350,18 @@ class TestRequire < Test::Unit::TestCase
path = tmp.path
tmp.print <<-EOS
TestRequire.scratch << :pre
-Thread.pass until t2 = TestRequire.scratch[1]
+TestRequire.scratch << Thread.current
+Thread.pass until t2 = TestRequire.scratch[2]
Thread.pass until t2.stop?
-open(FILE, “w”) {|f| f.puts “TestRequire.scratch << :post”}
+open(FILE, “w”) do |f|
- f.puts “t1, t2 = TestRequire.scratch[1, 2]”
- f.puts “if Thread.current == t2”
- f.puts " TestRequire.scratch << :post"
- f.puts " until t1.stop?"
- f.puts " Thread.pass"
- f.puts " end"
- f.puts “end”
+end
raise “con1”
EOS
tmp.close
@@ -368,7 +377,6 @@ raise “con1”
require(path)
rescue RuntimeError
end
@@ -376,7 +384,7 @@ raise “con1”
end
t2 = Thread.new do
@@ -389,8 +397,8 @@ raise “con1”
assert_nothing_raised(ThreadError, bug5754) {t1.join}
assert_nothing_raised(ThreadError, bug5754) {t2.join}
- assert_equal(true, (t1_res ^ t2_res), bug5754)
- assert_equal([:pre, t2, :post, :t2, :t1], scratch, bug5754)
- assert_equal(true, (t1_res ^ t2_res), bug5754 + " t1:#{t1_res}
t2:#{t2_res}")
- assert_equal([:pre, t1, t2, :post, :t2, :t1], scratch, bug5754)
ensure
tmp.close(true) if tmp
end
diff --git a/thread.c b/thread.c
index ced10c2…c46653b 100644
— a/thread.c
+++ b/thread.c
@@ -3409,7 +3409,6 @@ lock_func(rb_thread_t *th, rb_mutex_t *mutex, int
timeout_ms)
int interrupted = 0;
int err = 0;
-
mutex->cond_waiting++;
for (;
{
if (!mutex->th) {
mutex->th = th;
@@ -3438,7 +3437,6 @@ lock_func(rb_thread_t *th, rb_mutex_t *mutex, int
timeout_ms)
err = 0;
}
}
-
mutex->cond_waiting–;
return interrupted;
}
@@ -3493,10 +3491,12 @@ rb_mutex_lock(VALUE self)
if (vm_living_thread_num(th->vm) == th->vm->sleeper) {
timeout_ms = 100;
}
-
mutex->cond_waiting++;
GVL_UNLOCK_BEGIN();
interrupted = lock_func(th, mutex, timeout_ms);
native_mutex_unlock(&mutex->lock);
GVL_UNLOCK_END();
-
mutex->cond_waiting--;
reset_unblock_function(th, &oldubf);
@@ -3727,9 +3727,11 @@ rb_barrier_release(VALUE self)
{
VALUE mutex = GetBarrierPtr(self);
rb_mutex_t *m;
- int waiting;
GetMutexPtr(mutex, m);
- return m->cond_waiting > 0 ? Qtrue : Qfalse;
- waiting = m->cond_waiting;
- rb_mutex_unlock(mutex);
- return waiting > 0 ? Qtrue : Qfalse;
}
/*
@@ -3740,10 +3742,12 @@ rb_barrier_destroy(VALUE self)
{
VALUE mutex = GetBarrierPtr(self);
rb_mutex_t *m;
- int waiting;
DATA_PTR(self) = 0;
- rb_mutex_unlock(mutex);
GetMutexPtr(mutex, m);
- return m->cond_waiting > 0 ? Qtrue : Qfalse;
- waiting = m->cond_waiting;
- rb_mutex_unlock(mutex);
- return waiting > 0 ? Qtrue : Qfalse;
}
int
Bug #5768: TestRequire#test_race_exceptionで競合するケースがまだある
Author: Yui NARUSE
Status: Assigned
Priority: Normal
Assignee: Nobuyoshi N.
Category:
Target version:
ruby -v: ruby 2.0.0dev (2011-12-16 trunk 34058) [x86_64-freebsd9.0]
まだrequireで競合するケースが残っています。
現在のテストだと確率的にしか起きませんが、以下の通り変更すると確実に起きるようになります。
diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb
index 9186a6f…262a5ef 100644
— a/test/ruby/test_require.rb
+++ b/test/ruby/test_require.rb
@@ -352,7 +352,7 @@ class TestRequire < Test::Unit::TestCase
TestRequire.scratch << :pre
Thread.pass until t2 = TestRequire.scratch[1]
Thread.pass until t2.stop?
-open(FILE, “w”) {|f| f.puts “TestRequire.scratch << :post”}
+open(FILE, “w”) {|f| f.puts “TestRequire.scratch << :post”; f.puts
“t1,t2=TestRequire.scratch[1, 2];if Thread.current == t2; Thread.pass
until t1.stopped?; end”}
raise “con1”
EOS
tmp.close
@@ -364,6 +364,7 @@ raise “con1”
t2_res = nil
t1 = Thread.new do
@@ -389,8 +390,8 @@ raise “con1”
assert_nothing_raised(ThreadError, bug5754) {t1.join}
assert_nothing_raised(ThreadError, bug5754) {t2.join}
- assert_equal(true, (t1_res ^ t2_res), bug5754)
- assert_equal([:pre, t2, :post, :t2, :t1], scratch, bug5754)
- assert_equal(true, (t1_res ^ t2_res), bug5754 + " t1:#{t1_res}
t2:#{t2_res}")
- assert_equal([:pre, t1, t2, :post, :t2, :t1], scratch, bug5754)
ensure
tmp.close(true) if tmp
end