Project

General

Profile

Actions

Bug #15735

closed

Thread#handle_interrupt does not prevent Thread#kill from interrupting

Added by inoueyu (Yuki INOUE) over 2 years ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
2.6.2p47 (2019-03-13 revision 67232) [x86_64-darwin18]
[ruby-core:92038]
Tags:

Description

https://docs.ruby-lang.org/en/trunk/Thread.html#method-c-handle_interrupt

According to the documentation above, Thread#handle_interrupt(Exception => :never) would prevent the interrupts from Thread#kill.

In the following code, however, the Thread#kill seems to just kill the thread immediately even if the thread is instructed to never handle interrupt.

t = Thread.new do
  Thread.handle_interrupt(Exception => :never) do
    puts "thread started"
    sleep 2
    puts "thread end"
  end
end
sleep 1
t.kill
puts "main end"
sleep 2

Outputs

$ ruby test.rb
thread started
main end
$ 

Is the document wrong? Or, implementation not working as expected?

Updated by Eregon (Benoit Daloze) over 2 years ago

If that is the case, that might also mean the fix in https://bugs.ruby-lang.org/issues/14998 might not be sufficient.

Actions #2

Updated by jeremyevans (Jeremy Evans) 4 months ago

  • Status changed from Open to Closed

Applied in changeset git|5c4ff3f00c6bd84ef0721c1077ee9c525daa68f8.


Document how to handle kill/terminate interrupts in Thread.handle_interrupt

The kill/terminate interrupts are internally handled not as Exception
instances, but as integers. So using Exception doesn't handle these
interrupts, but Object does. You can use Integer if you only want to
handle kill/terminate interrupts, but that's probably more of an
implementation detail, while handling Object should work regardless
of the implementation.

Fixes [Bug #15735]

Updated by jeremyevans0 (Jeremy Evans) 4 months ago

I've determined this is just a documentation issue, and there is a way for Thread.handle_interrupt to handle kill/terminate interrupts, which I've committed.

If we handle to allow Exception to include these interrupts, we could do something like this:

--- a/thread.c
+++ b/thread.c
@@ -1992,6 +1992,7 @@ rb_threadptr_pending_interrupt_check_mask(rb_thread_t *th, VALUE err)
     VALUE mod;
     long i;

+  retry:
     for (i=0; i<mask_stack_len; i++) {
        mask = mask_stack[mask_stack_len-(i+1)];

@@ -2023,6 +2024,15 @@ rb_threadptr_pending_interrupt_check_mask(rb_thread_t *th, VALUE err)
        }
        /* try to next mask */
     }
+
+    /* eKillSignal and eTerminateSignal are Integers, but should be
+     * rescued if using Exception in Thread.handle_interrupt.
+     */
+    if (err == rb_cInteger) {
+        err = rb_eException;
+        goto retry;
+    }
+
     return INTERRUPT_NONE;
 }


Actions

Also available in: Atom PDF