From 36ae16c44a3de2728ea6d45aaff6b0a13118c8ec Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 7 Dec 2023 14:36:50 +0100 Subject: [PATCH] Fix spurious return in Promises#wait_until_resolved * Fixes https://github.com/ruby-concurrency/concurrent-ruby/issues/1015 --- lib/concurrent-ruby/concurrent/promises.rb | 14 +++++++++-- spec/concurrent/promises_spec.rb | 28 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/lib/concurrent-ruby/concurrent/promises.rb b/lib/concurrent-ruby/concurrent/promises.rb index 1a3bcc9dc..54a998307 100644 --- a/lib/concurrent-ruby/concurrent/promises.rb +++ b/lib/concurrent-ruby/concurrent/promises.rb @@ -5,6 +5,7 @@ require 'concurrent/configuration' require 'concurrent/errors' require 'concurrent/re_include' +require 'concurrent/utility/monotonic_time' module Concurrent @@ -772,8 +773,17 @@ def wait_until_resolved(timeout) @Lock.synchronize do @Waiters.increment begin - unless resolved? - @Condition.wait @Lock, timeout + if timeout + start = Concurrent.monotonic_time + until resolved? + break if @Condition.wait(@Lock, timeout) == nil # nil means timeout + timeout -= (Concurrent.monotonic_time - start) + break if timeout <= 0 + end + else + until resolved? + @Condition.wait(@Lock, timeout) + end end ensure # JRuby may raise ConcurrencyError diff --git a/spec/concurrent/promises_spec.rb b/spec/concurrent/promises_spec.rb index d5a58bfa1..d89ce309f 100644 --- a/spec/concurrent/promises_spec.rb +++ b/spec/concurrent/promises_spec.rb @@ -754,4 +754,32 @@ def behaves_as_delay(delay, value) specify 'zip_futures_over' do expect(zip_futures_over([1, 2]) { |v| v.succ }.value!).to eq [2, 3] end + + describe 'value!' do + %w[with without].each do |timeout| + it "does not return spuriously #{timeout} timeout" do + # https://github.com/ruby-concurrency/concurrent-ruby/issues/1015 + trapped = false + original_handler = Signal.trap(:SIGHUP) { trapped = true } + begin + task = Concurrent::Promises.future { sleep } + main = Thread.current + t = Thread.new do + Thread.pass until main.stop? + Process.kill(:SIGHUP, Process.pid) + sleep 0.1 + main.raise "Done" + end + expect { + timeout == 'with' ? task.value!(3600) : task.value! + expect(task).to be_resolved # fail if #value! returned + }.to raise_error(RuntimeError, "Done") + expect(trapped).to be true + t.join + ensure + Signal.trap(:SIGHUP, original_handler) + end + end + end + end end