From f1ec8c869a38e4f6691c199ef266848dc9aef1a2 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 21 Dec 2022 16:02:24 +0000 Subject: [PATCH] DEV: Fix FinalDestination::Resolver race condition (#19558) We were adding to the resolver's work queue before setting up the `@lookup` and `@parent` information. That could lead to the lookup being performed on the wrong (or `nil`) hostname. This also lead to some flakiness in specs. --- lib/final_destination/resolver.rb | 4 +++- spec/lib/final_destination/resolver_spec.rb | 4 ---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/final_destination/resolver.rb b/lib/final_destination/resolver.rb index 551eb0270d4..f809099d4d2 100644 --- a/lib/final_destination/resolver.rb +++ b/lib/final_destination/resolver.rb @@ -8,12 +8,14 @@ class FinalDestination::Resolver @result = nil @queue ||= Queue.new - @queue << "" ensure_lookup_thread @lookup = addr @parent = Thread.current + # Wakeup the worker thread + @queue << "" + # This sleep will be interrupted by the lookup thread # if completed within timeout sleep timeout diff --git a/spec/lib/final_destination/resolver_spec.rb b/spec/lib/final_destination/resolver_spec.rb index bf519d843db..20ce673cd7f 100644 --- a/spec/lib/final_destination/resolver_spec.rb +++ b/spec/lib/final_destination/resolver_spec.rb @@ -18,16 +18,12 @@ describe FinalDestination::Resolver do expect { result = FinalDestination::Resolver.lookup("sleep.example.com", timeout: 0.001) - # If the test gets this far, it failed - puts "Flaky test debug: Result was #{result.inspect}" }.to raise_error(Timeout::Error) start_thread_count = alive_thread_count expect { result = FinalDestination::Resolver.lookup("sleep.example.com", timeout: 0.001) - # If the test gets this far, it failed - puts "Flaky test debug: Result was #{result.inspect}" }.to raise_error(Timeout::Error) expect(alive_thread_count).to eq(start_thread_count)