Feature #17363
Timeouts
Description
Builtin methods like Queue.pop
and Ractor.receive
have no timeout parameter.
We should either:
- provide such a parameter
- and/or provide a
Timeout::wake
that raises an timeout error only if the block is currently sleeping.
Details:
q = Queue.new
# ...
elem = Timeout::timeout(42) { q.pop } # => It is possible that an element is retreived from the queue but never stored in `elem`
elem = Timeout::wake(42) { q.pop } # => Guaranteed that either element is retrieved from the queue or an exception is raised, never both
Timeout::wake(42) { loop {} } # => infinite loop
# and/or
elem = q.pop(timeout: 42)
Currently, the only reliable way to have a Queue that accepts a timeout is to re-implement it from scratch. This post describe how involved that can be: https://spin.atomicobject.com/2017/06/28/queue-pop-with-timeout-fixed/
Related issues
Updated by jeremyevans0 (Jeremy Evans) about 2 months ago
I've wanted a timed version of Queue#pop
for a long time, to use as the backed for Sequel's connection pool. I was thinking of a separate method (Queue#timed_pop
), but a keyword argument works fine too. I think either is better than Timeout.wake
.
Updated by Eregon (Benoit Daloze) about 2 months ago
+1 for Queue#pop(timeout: 42)
.
FWIW TruffleRuby already has Queue#receive_timeout
as a private method,
and this is used to implement Timeout.timeout
without creating a new Thread every time.
It sounds like the proposed Timeout.wake{}
would be similar to Thread#wakeup
.
I'm not sure how it could work, because reading another thread state is always racy (without a GIL), and the thread checking timeouts must be a separate thread than the one doing the blocking call.
Also it could interrupt a blocking call in ensure
(e.g., cleaning up a connection), which would be unwanted.
Updated by nobu (Nobuyoshi Nakada) about 2 months ago
I'm positive about that option too.
But I wonder how Timeout.wake
works and if it is possible.
Updated by ko1 (Koichi Sasada) about 1 month ago
I also positive to introduce timeout
but not sure what happens on timeout.
- raise an exception -> which exception?
- return
nil
-> can't recognize returned value
I think timeout: nil
is same as no timeout:
given. Is it same as other methods?
Updated by ko1 (Koichi Sasada) about 1 month ago
Timeout::wake
you can implement it with Thread#handle_interrupt(RuntimeError => :never){ ... }
.
Updated by marcandre (Marc-Andre Lafortune) about 1 month ago
ko1 (Koichi Sasada) wrote in #note-4:
I also positive to introduce
timeout
but not sure what happens on timeout.
- raise an exception -> which exception?
How about subclassing Timeout::Error
to create Queue::Timeout
and Ractor::Timeout
?
- return
nil
-> can't recognize returned value
Agreed, it is not a good solution.
I think
timeout: nil
is same as notimeout:
given. Is it same as other methods?
Agree.
I think queue.pop(timeout: 0)
should be same as queue.pop(true)
but raise Queue:Timeout
.
Same idea with Ractor, timeout: 0
is non-blocking version of Ractor.receive/receive_if/select
.
Updated by Eregon (Benoit Daloze) about 1 month ago
marcandre (Marc-Andre Lafortune) wrote in #note-6:
How about subclassing
Timeout::Error
to createQueue::Timeout
andRactor::Timeout
?
Timeout
is stdlib, unlike the other 2 which are in core, so that's an issue.
Updated by marcandre (Marc-Andre Lafortune) about 1 month ago
Eregon (Benoit Daloze) wrote in #note-7:
marcandre (Marc-Andre Lafortune) wrote in #note-6:
How about subclassing
Timeout::Error
to createQueue::Timeout
andRactor::Timeout
?
Timeout
is stdlib, unlike the other 2 which are in core, so that's an issue.
Good point. We could create Thread::Timeout
as a common base class for all 3?
Updated by Eregon (Benoit Daloze) about 1 month ago
marcandre (Marc-Andre Lafortune) wrote in #note-8:
Good point. We could create
Thread::Timeout
as a common base class for all 3?
Thread::TimeoutError
then maybe?
Sounds OK, but not sure timeouts are always related to threads (e.g., an IO.select timeout).
Might not matter much, so Thread::TimeoutError
is fine for me.
Updated by marcandre (Marc-Andre Lafortune) about 1 month ago
We could also define ::TimeoutError
as base class, and modify timeout
lib so that Timeout::Error < ::TimeoutError
instead of ==
as it is currently.
Updated by nobu (Nobuyoshi Nakada) about 1 month ago
It is just one line to built-in Timeout::Error
.
rb_define_class_under(rb_define_module("Timeout"), "Error", rb_eRuntimeError);
Updated by Eregon (Benoit Daloze) about 1 month ago
nobu (Nobuyoshi Nakada) wrote in #note-11:
It is just one line to built-in
Timeout::Error
.rb_define_class_under(rb_define_module("Timeout"), "Error", rb_eRuntimeError);
I think that would be confusing, if Timeout::Error
is in core, and so a Timeout
module is always defined, and yet Timeout.timeout
is not defined.
Updated by Eregon (Benoit Daloze) about 1 month ago
So another option would be to move the timeout
stdlib to core, which could be interesting (can be better optimized, avoid an extra Ruby thread, etc).
Updated by Eregon (Benoit Daloze) 27 days ago
- Related to Bug #17470: Introduce non-blocking `Timeout.timeout` added
Updated by ko1 (Koichi Sasada) 12 days ago
- Assignee set to ko1 (Koichi Sasada)
- Status changed from Open to Assigned