Skip to content

Commit

Permalink
Fix spurious return in Promises#wait_until_resolved
Browse files Browse the repository at this point in the history
* Fixes #1015
  • Loading branch information
eregon committed Dec 7, 2023
1 parent 068212f commit 36ae16c
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
14 changes: 12 additions & 2 deletions lib/concurrent-ruby/concurrent/promises.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'concurrent/configuration'
require 'concurrent/errors'
require 'concurrent/re_include'
require 'concurrent/utility/monotonic_time'

module Concurrent

Expand Down Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions spec/concurrent/promises_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 36ae16c

Please sign in to comment.