Feature #7503: make timeout.rb async-interrupt safe by default

• Status: Assigned
• Assignee: matz (Yukihiro Matsumoto)
• Target version: -

[kosaki (Motohiro KOSAKI)]

Hi

Again and again we discussed, current timeout.rb is very dangerous because ExitException interrupt argument code and unwind call stack immediately.
It may prevent to run ensure block and makes resource leak.

I proposed change default to interrupted only on blocking point.

patch is here.
https://gist.github.com/4195015

I also propse to add 'immediate' optional argument because it may help to make a workaround timeout.rb + zero blocking point corner case.

What do you think?

[ko1 (Koichi Sasada)]

Actually, I like your proposal.
But I make weak objection.

The following code doesn't stop forever.

timeout(3){
# long calculation / infinite loop w/o IO operation
}

I guess most of case, the block includes IO operation and no problem.
However, there are several case for it.

This is Dec and Preview 2 was released.
I think it is too slow to introduce it.

Again, I like this proposal.
If no people have object, I can agree with this proposal.

Alternative proposal is making such a safe timeout method with different
name such as `safe_timeout'.

(2012/12/03 22:33), kosaki (Motohiro KOSAKI) wrote:

Issue #7503 has been reported by kosaki (Motohiro KOSAKI).

---

Bug #7503: make timeout.rb async-interrupt safe by default
https://bugs.ruby-lang.org/issues/7503

Author: kosaki (Motohiro KOSAKI)
Status: Open
Priority: Normal
Assignee:
Category: core
Target version: 2.0.0
ruby -v: trunk

Hi

Again and again we discussed, current timeout.rb is very dangerous because ExitException interrupt argument code and unwind call stack immediately.
It may prevent to run ensure block and makes resource leak.

I proposed change default to interrupted only on blocking point.

patch is here.
https://gist.github.com/4195015

I also propse to add 'immediate' optional argument because it may help to make a workaround timeout.rb + zero blocking point corner case.

What do you think?

--
// SASADA Koichi at atdot dot net

[kosaki (Motohiro KOSAKI)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r38216.
Motohiro, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• lib/timeout.rb (Timeout#timeout): set
  async_interrupt_timeing(:on_blocking) by default.
  [Bug #7503] [ruby-core:50524]

• test/test_timeout.rb (#test_timeout_blocking): test for the above.

• test/test_timeout.rb (test_timeout_immediate): ditto

• test/test_timeout.rb (test_timeout_immediate2): ditto.

• NEWS: news for the above.

[ko1 (Koichi Sasada)]

• Status changed from Closed to Open

I need to make strongly objection about this commit (r38216) because this discussion is not concluded.

I believe at least mame-san's permission is needed to introduce this change.
This is big behavior change.

[ko1 (Koichi Sasada)]

• Assignee set to mame (Yusuke Endoh)
• Priority changed from Normal to 5

[mame (Yusuke Endoh)]

• Tracker changed from Bug to Feature
• Status changed from Open to Assigned
• Assignee changed from mame (Yusuke Endoh) to matz (Yukihiro Matsumoto)

I do NOT accept r38216 for 2.0.0. Please revert it.

Not only it is a new feature missing the deadline, but also it will lead
to actual compatibility issues. I often use timeout to bound not only IO
operation but also pure calculation, such as game tree search and brute-
force search.

(Thanks ko1 for letting me know!)

--
Yusuke Endoh [e-mail]

[mame (Yusuke Endoh)]

• Priority changed from 5 to Normal
• Target version changed from 2.0.0 to 2.6

[nobu (Nobuyoshi Nakada)]

• % Done changed from 100 to 50

[nobu (Nobuyoshi Nakada)]

• Status changed from Assigned to Closed
• % Done changed from 50 to 100

This issue was solved with changeset r38255.
Motohiro, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

timeout.rb: replace deferred exception after async_interrupt_timing

• lib/timeout.rb (Timeout#timeout): since async_interrupt_timing
  re-raises a deferred exception, replace the timeout exception with
  Timeout::Error after it. [Bug #7503]

[kosaki (Motohiro KOSAKI)]

• Status changed from Closed to Assigned

Target version changed from 2.0.0 to next minor

r38216 was reverted then.

[naruse (Yui NARUSE)]

• Target version deleted (2.6)