Bug #16608: ConditionVariable#wait should return false when timeout exceeded

• Status: Closed
• Assignee: soutaro (Soutaro Matsumoto)
• Target version: -
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN

[shugo (Shugo Maeda)]

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.

[shugo (Shugo Maeda)]

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

[nobu (Nobuyoshi Nakada)]

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

[shugo (Shugo Maeda)]

• 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?

[Eregon (Benoit Daloze)]

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.

[jeremyevans0 (Jeremy Evans)]

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!.

[jeremyevans0 (Jeremy Evans)]

I've added a pull request that builds on @nobu'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

[jeremyevans0 (Jeremy Evans)]

@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.

[Eregon (Benoit Daloze)]

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

[ko1 (Koichi Sasada)]

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!

[jeremyevans0 (Jeremy Evans)]

• Assignee changed from nobu (Nobuyoshi Nakada) to soutaro (Soutaro Matsumoto)

I submitted a pull request to rbs to fix the failure (https://github.com/ruby/rbs/pull/683) I also updated the ruby pull request (https://github.com/ruby/ruby/pull/4256) to rebase it on the current master branch. Once the rbs pull request is merged and a release is made, we should be able to merge the ruby pull request.

[jeremyevans0 (Jeremy Evans)]

The rbs pull request has been merged. So we just need to wait for the next rbs gem release and for bundled gems to be updated, then we can merge the pull request.

[nobu (Nobuyoshi Nakada)]

• Status changed from Assigned to Closed

Applied in changeset git|070557afc4ca83876b951fe090806b59e3867ae5.

---

Distinguish signal and timeout [Bug #16608]