From 63d9ab33fe419167382fc03be2c00413a78c05cb Mon Sep 17 00:00:00 2001 From: nagachika Date: Wed, 23 Jan 2019 14:14:25 +0000 Subject: merge revision(s) 62934,63210,63215,63309: [Backport #14634] thread_sync.c: avoid reaching across stacks of dead threads rb_ensure is insufficient cleanup for fork and we must reinitialize all waitqueues in the child process. Unfortunately this increases the footprint of ConditionVariable, Queue and SizedQueue by 8 bytes on 32-bit (16 bytes on 64-bit). [ruby-core:86316] [Bug #14634] variable.c: fix thread + fork errors in autoload This is fairly non-intrusive bugfix to prevent children from trying to reach into thread stacks of the parent. I will probably reuse this idea and redo r62934, too (same bug). * vm_core.h (typedef struct rb_vm_struct): add fork_gen counter * thread.c (rb_thread_atfork_internal): increment fork_gen * variable.c (struct autoload_data_i): store fork_gen * variable.c (check_autoload_data): remove (replaced with get_...) * variable.c (get_autoload_data): check fork_gen when retrieving * variable.c (check_autoload_required): use get_autoload_data * variable.c (rb_autoloading_value): ditto * variable.c (rb_autoload_p): ditto * variable.c (current_autoload_data): ditto * variable.c (autoload_reset): reset fork_gen, adjust indent * variable.c (rb_autoload_load): set fork_gen when setting state * test/ruby/test_autoload.rb (test_autoload_fork): new test [ruby-core:86410] [Bug #14634] thread_sync: redo r62934 to use fork_gen Instead of maintaining linked-lists to store all rb_queue/rb_szqueue/rb_condvar structs; store only a fork_gen serial number to simplify management of these items. This reduces initialization costs and avoids the up-front cost of resetting all Queue/SizedQueue/ConditionVariable objects at fork while saving 8 bytes per-structure on 64-bit. There are no savings on 32-bit. * thread.c (rb_thread_atfork_internal): remove rb_thread_sync_reset_all call * thread_sync.c (rb_thread_sync_reset_all): remove * thread_sync.c (queue_live): remove * thread_sync.c (queue_free): remove * thread_sync.c (struct rb_queue): s/live/fork_gen/ * thread_sync.c (queue_data_type): use default free * thread_sync.c (queue_alloc): remove list_add * thread_sync.c (queue_fork_check): new function * thread_sync.c (queue_ptr): call queue_fork_check * thread_sync.c (szqueue_free): remove * thread_sync.c (szqueue_data_type): use default free * thread_sync.c (szqueue_alloc): remove list_add * thread_sync.c (szqueue_ptr): check fork_gen via queue_fork_check * thread_sync.c (struct rb_condvar): s/live/fork_gen/ * thread_sync.c (condvar_free): remove * thread_sync.c (cv_data_type): use default free * thread_sync.c (condvar_ptr): check fork_gen * thread_sync.c (condvar_alloc): remove list_add [ruby-core:86316] [Bug #14634] thread_sync.c (condvar_ptr): reset fork_gen after forking Otherwise the condition variable waiter list will always be empty, which is wrong :x [Bug #14725] [Bug #14634] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@66912 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- test/ruby/test_autoload.rb | 26 +++++++++++++++++++++++++ test/thread/test_cv.rb | 19 ++++++++++++++++++ test/thread/test_queue.rb | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) (limited to 'test') diff --git a/test/ruby/test_autoload.rb b/test/ruby/test_autoload.rb index 0220f3e27d..3095052a81 100644 --- a/test/ruby/test_autoload.rb +++ b/test/ruby/test_autoload.rb @@ -285,6 +285,32 @@ p Foo::Bar end end + def test_autoload_fork + EnvUtil.default_warning do + Tempfile.create(['autoload', '.rb']) {|file| + file.puts 'sleep 0.3; class AutoloadTest; end' + file.close + add_autoload(file.path) + begin + thrs = [] + 3.times do + thrs << Thread.new { AutoloadTest; nil } + thrs << Thread.new { fork { AutoloadTest } } + end + thrs.each(&:join) + thrs.each do |th| + pid = th.value or next + _, status = Process.waitpid2(pid) + assert_predicate status, :success? + end + ensure + remove_autoload_constant + assert_nil $!, '[ruby-core:86410] [Bug #14634]' + end + } + end + end if Process.respond_to?(:fork) + def add_autoload(path) (@autoload_paths ||= []) << path ::Object.class_eval {autoload(:AutoloadTest, path)} diff --git a/test/thread/test_cv.rb b/test/thread/test_cv.rb index 1e15d2e9ec..306cadd63b 100644 --- a/test/thread/test_cv.rb +++ b/test/thread/test_cv.rb @@ -217,4 +217,23 @@ INPUT Marshal.dump(condvar) end end + + def test_condvar_fork + mutex = Mutex.new + condvar = ConditionVariable.new + thrs = (1..10).map do + Thread.new { mutex.synchronize { condvar.wait(mutex) } } + end + thrs.each { 3.times { Thread.pass } } + pid = fork do + mutex.synchronize { condvar.broadcast } + exit!(0) + end + _, s = Process.waitpid2(pid) + assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]' + until thrs.empty? + mutex.synchronize { condvar.broadcast } + thrs.delete_if { |t| t.join(0.01) } + end + end if Process.respond_to?(:fork) end diff --git a/test/thread/test_queue.rb b/test/thread/test_queue.rb index 4e6c9fa4c9..d69ecf92b2 100644 --- a/test/thread/test_queue.rb +++ b/test/thread/test_queue.rb @@ -565,4 +565,52 @@ class TestQueue < Test::Unit::TestCase puts 'exit' INPUT end + + def test_fork_while_queue_waiting + q = Queue.new + sq = SizedQueue.new(1) + thq = Thread.new { q.pop } + thsq = Thread.new { sq.pop } + Thread.pass until thq.stop? && thsq.stop? + + pid = fork do + exit!(1) if q.num_waiting != 0 + exit!(2) if sq.num_waiting != 0 + exit!(6) unless q.empty? + exit!(7) unless sq.empty? + q.push :child_q + sq.push :child_sq + exit!(3) if q.pop != :child_q + exit!(4) if sq.pop != :child_sq + exit!(0) + end + _, s = Process.waitpid2(pid) + assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]' + + q.push :thq + sq.push :thsq + assert_equal :thq, thq.value + assert_equal :thsq, thsq.value + + sq.push(1) + th = Thread.new { q.pop; sq.pop } + thsq = Thread.new { sq.push(2) } + Thread.pass until th.stop? && thsq.stop? + pid = fork do + exit!(1) if q.num_waiting != 0 + exit!(2) if sq.num_waiting != 0 + exit!(3) unless q.empty? + exit!(4) if sq.empty? + exit!(5) if sq.pop != 1 + exit!(0) + end + _, s = Process.waitpid2(pid) + assert_predicate s, :success?, 'no segfault [ruby-core:86316] [Bug #14634]' + + assert_predicate thsq, :stop? + assert_equal 1, sq.pop + assert_same sq, thsq.value + q.push('restart th') + assert_equal 2, th.value + end if Process.respond_to?(:fork) end -- cgit v1.2.3