Project

General

Profile

Actions

Bug #16608

open

ConditionVariable#wait should return false when timeout exceeded

Added by shugo (Shugo Maeda) over 1 year ago. Updated 21 days ago.

Status:
Assigned
Priority:
Normal
Target version:
-
[ruby-core:97063]

Description

The following program prints false on Ruby 1.8, but true on Ruby 1.9 or later.

require "monitor"
m = Monitor.new
c = m.new_cond
m.synchronize { p c.wait(0.1) }

However, it's not critical because most programs check the condition after wait.

Updated by shugo (Shugo Maeda) about 1 year ago

  • Assignee set to shugo (Shugo Maeda)
  • Status changed from Open to Assigned

Updated by shugo (Shugo Maeda) about 1 year ago

  • Assignee changed from shugo (Shugo Maeda) to nobu (Nobuyoshi Nakada)

nobu (Nobuyoshi Nakada) wrote in #note-2:

How about https://github.com/ruby/ruby/pull/2884

ko1 suggested Mutex#release (new version of Mutex#sleep) for backward compatibility.
What do you think of it?

Updated by Eregon (Benoit Daloze) about 1 year ago

shugo (Shugo Maeda) wrote in #note-3:

ko1 suggested Mutex#release (new version of Mutex#sleep) for backward compatibility.

release sounds like unlock as in acquire/release is similar to lock/unlock.
So I think we need a different name there.

I'm not sure about backward compatibility, but I would think it's fine to change the return value here as probably almost nothing uses it.

Updated by jeremyevans0 (Jeremy Evans) 11 months ago

I agree with Eregon (Benoit Daloze) that we can probably change the return value of ConditionVariable#wait in Ruby 3. As it always returns true currently, it's worthless and nothing should be relying on it.

I can see that changing Mutex#sleep to start returning nil could break things, though. I think we do need a new method. Maybe Mutex#sleep_for or Mutex#sleep!.

Updated by jeremyevans0 (Jeremy Evans) about 2 months ago

I've added a pull request that builds on nobu (Nobuyoshi Nakada)'s pull request, but uses a new method (Mutex#sleep_for) instead of making backwards-incompatible changes to Mutex#sleep: https://github.com/ruby/ruby/pull/4256

Updated by jeremyevans0 (Jeremy Evans) about 2 months ago

matz (Yukihiro Matsumoto) agreed to change the return value of Mutex#sleep at the last developer meeting. However, that breaks a Mutex_m test: https://github.com/ruby/ruby/pull/4256/checks?check_run_id=2135513377#step:15:1295

So the Mutex_m test needs to be fixed first, a new release needs to be made, and the bundled gems need to be updated, before the Mutex#sleep change can be merged.

Updated by Eregon (Benoit Daloze) about 2 months ago

That looks like an error of rbs actually, cc soutaro (Soutaro Matsumoto)

Updated by ko1 (Koichi Sasada) 21 days ago

Previous dev-meeting (March), there is no objection to change the return value of Mutex#sleep.

matz: agreed to change the return value of Mutex#sleep.
naruse: agreed to change in Ruby 3.1 because this is so detail

Congrats!

Actions

Also available in: Atom PDF