diff --git a/ChangeLog b/ChangeLog index 054befacba..13304e5ad8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +Sun Sep 9 20:20:31 2012 KOSAKI Motohiro + + * lib/sync.rb (Sync_m#sync_lock): Fixed wakeup/raise unsafe code. + Patched by Masaki Matsushita. [Bug #5355] [ruby-dev:44521] + + * test/thread/test_sync.rb (test_sync_lock_and_wakeup, + test_sync_upgrade_and_wakeup, test_sync_lock_and_raise): + new test. + Sun Sep 9 18:39:46 2012 KOSAKI Motohiro * include/ruby/intern.h (rb_thread_blocking_region): Added diff --git a/lib/sync.rb b/lib/sync.rb index a57b789d55..524f3804e9 100644 --- a/lib/sync.rb +++ b/lib/sync.rb @@ -138,16 +138,22 @@ module Sync_m while true @sync_mutex.synchronize do - if sync_try_lock_sub(m) - return self - else - if sync_sh_locker[Thread.current] - sync_upgrade_waiting.push [Thread.current, sync_sh_locker[Thread.current]] - sync_sh_locker.delete(Thread.current) + begin + if sync_try_lock_sub(m) + return self else - sync_waiting.push Thread.current + if sync_sh_locker[Thread.current] + sync_upgrade_waiting.push [Thread.current, sync_sh_locker[Thread.current]] + sync_sh_locker.delete(Thread.current) + else + unless sync_waiting.include?(Thread.current) || sync_upgrade_waiting.reverse_each.any?{|w| w.first == Thread.current } + sync_waiting.push Thread.current + end + end + @sync_mutex.sleep end - @sync_mutex.sleep + ensure + sync_waiting.delete(Thread.current) end end end diff --git a/test/thread/test_sync.rb b/test/thread/test_sync.rb new file mode 100644 index 0000000000..870a8e2d39 --- /dev/null +++ b/test/thread/test_sync.rb @@ -0,0 +1,57 @@ +require 'test/unit' +require 'sync' +require 'timeout' + +class SyncTest < Test::Unit::TestCase + class Tester + include Sync_m + end + + def test_sync_lock_and_wakeup + tester = Tester.new + + tester.sync_lock(:EX) + + t = Thread.new { tester.sync_lock(:EX) } + + sleep 0.1 until t.stop? + t.wakeup + sleep 0.1 until t.stop? + + assert_equal(tester.sync_waiting.uniq, tester.sync_waiting) + end + + def test_sync_upgrade_and_wakeup + tester = Tester.new + tester.sync_lock(:SH) + + t = Thread.new do + tester.sync_lock(:SH) + tester.sync_lock(:EX) + end + + sleep 0.1 until t.stop? + t.wakeup + sleep 0.1 until t.stop? + + tester.sync_upgrade_waiting.each { |ary| + assert(!tester.sync_waiting.include?(ary[0])) + } + assert_equal(tester.sync_waiting.uniq, tester.sync_waiting) + assert_equal(tester.sync_waiting, []) + end + + def test_sync_lock_and_raise + tester= Tester.new + tester.sync_lock(:EX) + + t = Thread.new { tester.sync_lock(:EX) } + + sleep 0.1 until t.stop? + t.raise + sleep 0.1 while t.alive? + + assert_equal(tester.sync_waiting.uniq, tester.sync_waiting) + assert_equal(tester.sync_waiting, []) + end +end