From d6dd46a1d9558c32ed17f1e9ce1726b272b06404 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Mon, 10 Apr 2017 16:45:02 -0700 Subject: [PATCH 1/2] fix flakey race in ruby tests --- src/ruby/spec/generic/rpc_server_pool_spec.rb | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/ruby/spec/generic/rpc_server_pool_spec.rb b/src/ruby/spec/generic/rpc_server_pool_spec.rb index 69e8222cb97..75b40225680 100644 --- a/src/ruby/spec/generic/rpc_server_pool_spec.rb +++ b/src/ruby/spec/generic/rpc_server_pool_spec.rb @@ -52,27 +52,36 @@ describe GRPC::Pool do expect(p.ready_for_work?).to be(false) end - it 'it stops being ready after all workers jobs waiting or running' do + it 'it stops being ready after all workers are busy, ' \ + 'and it becomes ready again after jobs complete' do p = Pool.new(5) p.start - job = proc { sleep(3) } # sleep so workers busy when done scheduling - 5.times do - expect(p.ready_for_work?).to be(true) - p.schedule(&job) + + wait_mu = Mutex.new + wait_cv = ConditionVariable.new + wait = true + + job = proc do + wait_mu.synchronize do + wait_cv.wait(wait_mu) while wait + end end - expect(p.ready_for_work?).to be(false) - end - it 'it becomes ready again after jobs complete' do - p = Pool.new(5) - p.start - job = proc {} 5.times do expect(p.ready_for_work?).to be(true) p.schedule(&job) end + expect(p.ready_for_work?).to be(false) - sleep 5 # give the pool time do get at least one task done + + wait_mu.synchronize do + wait = false + wait_cv.broadcast + end + + # There's a potential race here but it shouldn't ever be + # reached with a 5 second sleep. + sleep 5 expect(p.ready_for_work?).to be(true) end end @@ -105,13 +114,13 @@ describe GRPC::Pool do it 'stops jobs when there are long running jobs' do p = Pool.new(1) p.start - o, q = Object.new, Queue.new + job_running = Queue.new job = proc do - sleep(5) # long running - q.push(o) + job_running.push(Object.new) + sleep(5000) end p.schedule(&job) - sleep(1) # should ensure the long job gets scheduled + job_running.pop expect { p.stop }.not_to raise_error end end From 9b020019497f5fb9fb035027d55d350450fb4ed0 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Tue, 11 Apr 2017 10:14:13 -0700 Subject: [PATCH 2/2] get rid of racey sleep 5 and use a cv to wait forever --- src/ruby/spec/generic/rpc_server_pool_spec.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ruby/spec/generic/rpc_server_pool_spec.rb b/src/ruby/spec/generic/rpc_server_pool_spec.rb index 75b40225680..0803ca74ed6 100644 --- a/src/ruby/spec/generic/rpc_server_pool_spec.rb +++ b/src/ruby/spec/generic/rpc_server_pool_spec.rb @@ -52,8 +52,7 @@ describe GRPC::Pool do expect(p.ready_for_work?).to be(false) end - it 'it stops being ready after all workers are busy, ' \ - 'and it becomes ready again after jobs complete' do + it 'it stops being ready after all workers are busy' do p = Pool.new(5) p.start @@ -78,11 +77,6 @@ describe GRPC::Pool do wait = false wait_cv.broadcast end - - # There's a potential race here but it shouldn't ever be - # reached with a 5 second sleep. - sleep 5 - expect(p.ready_for_work?).to be(true) end end @@ -114,10 +108,17 @@ describe GRPC::Pool do it 'stops jobs when there are long running jobs' do p = Pool.new(1) p.start + + wait_forever_mu = Mutex.new + wait_forever_cv = ConditionVariable.new + wait_forever = true + job_running = Queue.new job = proc do job_running.push(Object.new) - sleep(5000) + wait_forever_mu.synchronize do + wait_forever_cv.wait while wait_forever + end end p.schedule(&job) job_running.pop