Actions
Bug #20795
closedTimeout method doesn't check for negative time values
Description
require 'timeout'
# Case 1: Negative timeout with quick execution
Timeout.timeout(-5) do
puts "This should not execute"
end
# Case 2: Negative timeout with sleep
Timeout.timeout(-5) do
sleep(10)
end
Potential issues with current behaviour:
- Inconsistency: The behaviour differs based on the block's content, which may not be immediately apparent
- Silent failure: The negative timeout is silently ignored, potentially masking logical errors in the calling code.
- Unexpected source of error: one might expect the timeout method to validate its input, rather than relying on methods called within the block to catch invalid time values.
I suggest this change for the consistent behaviour regardless of code-block as well as the clear indication of the source of the error
def timeout(sec, klass = nil, message = nil, &block)
raise ArgumentError, "timeout must be positive" if sec.is_a?(Numeric) && sec < 0
# ...
end
Updated by Eregon (Benoit Daloze) 2 months ago
Could you make a PR to https://github.com/ruby/timeout ? (or if not open an issue there?)
Updated by jeremyevans0 (Jeremy Evans) about 1 month ago
PR was filed for this: https://github.com/ruby/timeout/pull/51
Updated by hsbt (Hiroshi SHIBATA) 19 days ago
We discussed this with Matz at last developer meeting.
He said "Let's raise an ArgumentError
against negative value".
Updated by hsbt (Hiroshi SHIBATA) 19 days ago
- Status changed from Open to Closed
- Backport changed from 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN to 3.1: DONTNEED, 3.2: DONTNEED, 3.3: DONTNEED
I merged https://github.com/ruby/timeout/pull/51 now. Thank you.
Actions
Like0
Like1Like0Like0Like0