Project

General

Profile

Actions

Feature #7503

open

make timeout.rb async-interrupt safe by default

Added by kosaki (Motohiro KOSAKI) over 11 years ago. Updated over 6 years ago.

Status:
Assigned
Target version:
-
[ruby-core:50524]

Description

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?

Updated by ko1 (Koichi Sasada) over 11 years ago

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

Actions #2

Updated by kosaki (Motohiro KOSAKI) over 11 years ago

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

Updated by ko1 (Koichi Sasada) over 11 years ago

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

Updated by ko1 (Koichi Sasada) over 11 years ago

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

Updated by mame (Yusuke Endoh) over 11 years ago

  • 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

Updated by mame (Yusuke Endoh) over 11 years ago

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

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

  • % Done changed from 100 to 50
Actions #8

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

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

Updated by kosaki (Motohiro KOSAKI) over 11 years ago

  • Status changed from Closed to Assigned

Target version changed from 2.0.0 to next minor

r38216 was reverted then.

Actions #10

Updated by naruse (Yui NARUSE) over 6 years ago

  • Target version deleted (2.6)
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0