tl; dr
Thread.handle_interrupt won't work well when the timeout thread is reused. It is because blocked interrupts are inherited when the thread is created. In my opinion such an inheritance should not happen, but removing it will be a breaking change and requires core Ruby change.
Long story
#15 changed Timeout to reuse a thread. On the other hand, a thread inherits the blocked interrupts of the creating thread. This means the list of blocked interrupts will persist once after a thread is created first time.
This breaks the code something like follows:
# Part X
timeout(10) {}
# Part Y
Thread.handle_interrupt(Timeout::Error => :never) do
timeout(10) do
Thread.handle_interrupt(Timeout::Error => :on_blocking) do
do_something
end
ensure
clean # Timeout::Error shouldn't occur here (but it can since #15)
end
endCode Y follows the example shown at: https://docs.ruby-lang.org/en/3.2/Thread.html#method-c-handle_interrupt-label-Guarding+from+Timeout-3A-3AError
In this case, the later Thread.handle_interrupt(Timeout::Error => :never) will have no effect.
The opposite scenario can also happen. Consider the following code:
# Part Y
Thread.handle_interrupt(Timeout::Error => :never) do
timeout(10) do
Thread.handle_interrupt(Timeout::Error => :on_blocking) do
do_something
end
ensure
clean
end
end
# Part X
timeout(10) do
# Timeout::Error should happen here but it won't in reality.
endIn my opinion, this kind of problem occurs due to a bad design of Ruby threads; a new thread should not inherit the blocked interrupts. The inheritance of blocked interrupts can break multithreading code so easily. Thread reuse as seen in Timeout is one of patterns affected by this design. Any kind of thread pools will also be affected.
The inheritance is also problematic when a method called from Thread.handle_interrupt block uses Thread#raise. For example, think of the following code:
module SomeExternalLibrary
def gracefully_close_connection
begin
timeout(10) { tell_the_remote_machine_that_i_am_leaving }
rescue Timeout::Error
# I guess the remote machine is dead.
end
@@socket.close
end
end
# I want to make sure graceful shutdown happens even when SIGINT happens.
Thread.handle_interrupt(Object => :never) do
SomeExternalLibrary.gracefully_close_connection
endSomeExternalLibrary.gracefully_close_connection may not work because timeout is blocked with Thread.handle_interrupt(Object => :never).
This kind of problems will not happen if the inheritance of blocked interrupts does not happen. Removing the inheritance of blocked interrupts would break code part Y, but I don't think that matters much. In fact, if you should be able to rewrite the code as follows:
begin
timeout(10) do
do_something
end
ensure
clean # Here Timeout::Error won't happen because it's out of the timeout block.
endIf you still need to block Timeout::Error at the beginning of the timeout block, timeout method can have an extra parameter to block interrupts before the timer gets armed though I don't see a use case for that.
That said, removing the inheritance of blocked interrupts will be a breaking change so it's not really practical. Also it will be a core Ruby change so timeout gem may need its own solution, perhaps just reverting #15, adding an option, or just edit the documentation to say "do not use timeout with Thread.handle_interrupt ever".
![dependabot[bot] avatar](https://avatars.githubusercontent.com/in/29110?v=4 "dependabot[bot]")
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent