[ruby-core:125864] [Ruby Bug#22133] Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking.
Issue #22133 has been reported by ioquatix (Samuel Williams). ---------------------------------------- Bug #22133: Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking. https://bugs.ruby-lang.org/issues/22133 * Author: ioquatix (Samuel Williams) * Status: Open * Assignee: ioquatix (Samuel Williams) * Backport: 3.3: REQUIRED, 3.4: REQUIRED, 4.0: REQUIRED ---------------------------------------- Ruby's default SIGINT handling currently bypasses `Thread.handle_interrupt` masking. When a process receives SIGINT with the default Ruby handler installed, CRuby calls `rb_interrupt()` directly. That can raise `Interrupt` immediately inside code wrapped by `Thread.handle_interrupt(SignalException => :never)`, including while blocked in operations like `Thread::Queue#pop`. This is inconsistent with other asynchronous exception delivery paths, such as `Thread#raise`, and with other default signal exceptions such as SIGTERM. Those paths enqueue a pending interrupt on the target thread, allowing `Thread.handle_interrupt` to defer delivery until the configured point. ## Background This was discovered while working on `Async`/`Async::Container` signal handling. Async relies on `Thread.handle_interrupt` and scheduler-level interruption boundaries to keep signal delivery deterministic. The current default SIGINT path means a Ctrl-C can escape a masked section and interrupt internal synchronization code, which can break graceful shutdown behavior. A minimal reproduction is: ```ruby waiting = Thread::Queue.new release = Thread::Queue.new inner = false Thread.new do waiting.pop Process.kill(:INT, Process.pid) release.push(true) end begin Thread.handle_interrupt(SignalException => :never) do begin waiting.push(true) release.pop rescue Interrupt inner = true raise end end rescue Interrupt puts "outer" end puts inner ``` Expected output: ```text outer false ``` Current affected behavior can produce `inner == true`, meaning the `Interrupt` was delivered inside the masked region. ## Proposed fix Route default SIGINT through the same pending-interrupt mechanism as other default signal exceptions, while preserving the traditional exception class and no-message `Interrupt` object. Concretely, the PR replaces the direct `rb_interrupt()` default SIGINT path with a helper that enqueues an `Interrupt` on the main thread and wakes that thread. This makes default SIGINT respect `Thread.handle_interrupt(SignalException => :never)` in the same way as default SIGTERM/SignalException and `Thread#raise`. ## Pull request PR: https://github.com/ruby/ruby/pull/17533 The PR includes: 1. A failing regression test for default SIGINT with `Thread.handle_interrupt`. 2. The implementation change routing default SIGINT through the pending interrupt queue. 3. Ruby/spec coverage for default SIGINT -> `Interrupt` and default SIGTERM -> `SignalException` being maskable by `Thread.handle_interrupt`. Once accepted, I would like to request backports for supported Ruby branches where this default SIGINT behavior is present. -- https://bugs.ruby-lang.org/
Issue #22133 has been updated by kosaki (Motohiro KOSAKI). I haven't read the code yet, but what you're saying seems reasonable. ---------------------------------------- Bug #22133: Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking. https://bugs.ruby-lang.org/issues/22133#change-117799 * Author: ioquatix (Samuel Williams) * Status: Open * Assignee: ioquatix (Samuel Williams) * Backport: 3.3: REQUIRED, 3.4: REQUIRED, 4.0: REQUIRED ---------------------------------------- Ruby's default SIGINT handling currently bypasses `Thread.handle_interrupt` masking. When a process receives SIGINT with the default Ruby handler installed, CRuby calls `rb_interrupt()` directly. That can raise `Interrupt` immediately inside code wrapped by `Thread.handle_interrupt(SignalException => :never)`, including while blocked in operations like `Thread::Queue#pop`. This is inconsistent with other asynchronous exception delivery paths, such as `Thread#raise`, and with other default signal exceptions such as SIGTERM. Those paths enqueue a pending interrupt on the target thread, allowing `Thread.handle_interrupt` to defer delivery until the configured point. ## Background This was discovered while working on `Async`/`Async::Container` signal handling. Async relies on `Thread.handle_interrupt` and scheduler-level interruption boundaries to keep signal delivery deterministic. The current default SIGINT path means a Ctrl-C can escape a masked section and interrupt internal synchronization code, which can break graceful shutdown behavior. A minimal reproduction is: ```ruby waiting = Thread::Queue.new release = Thread::Queue.new inner = false Thread.new do waiting.pop Process.kill(:INT, Process.pid) release.push(true) end begin Thread.handle_interrupt(SignalException => :never) do begin waiting.push(true) release.pop rescue Interrupt inner = true raise end end rescue Interrupt puts "outer" end puts inner ``` Expected output: ```text outer false ``` Current affected behavior can produce `inner == true`, meaning the `Interrupt` was delivered inside the masked region. ## Proposed fix Route default SIGINT through the same pending-interrupt mechanism as other default signal exceptions, while preserving the traditional exception class and no-message `Interrupt` object. Concretely, the PR replaces the direct `rb_interrupt()` default SIGINT path with a helper that enqueues an `Interrupt` on the main thread and wakes that thread. This makes default SIGINT respect `Thread.handle_interrupt(SignalException => :never)` in the same way as default SIGTERM/SignalException and `Thread#raise`. ## Pull request PR: https://github.com/ruby/ruby/pull/17533 The PR includes: 1. A failing regression test for default SIGINT with `Thread.handle_interrupt`. 2. The implementation change routing default SIGINT through the pending interrupt queue. 3. Ruby/spec coverage for default SIGINT -> `Interrupt` and default SIGTERM -> `SignalException` being maskable by `Thread.handle_interrupt`. Once accepted, I would like to request backports for supported Ruby branches where this default SIGINT behavior is present. -- https://bugs.ruby-lang.org/
Issue #22133 has been updated by Eregon (Benoit Daloze). Seems like a clear bugfix. TruffleRuby already behaves like this. ---------------------------------------- Bug #22133: Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking. https://bugs.ruby-lang.org/issues/22133#change-117824 * Author: ioquatix (Samuel Williams) * Status: Open * Assignee: ioquatix (Samuel Williams) * Backport: 3.3: REQUIRED, 3.4: REQUIRED, 4.0: REQUIRED ---------------------------------------- Ruby's default SIGINT handling currently bypasses `Thread.handle_interrupt` masking. When a process receives SIGINT with the default Ruby handler installed, CRuby calls `rb_interrupt()` directly. That can raise `Interrupt` immediately inside code wrapped by `Thread.handle_interrupt(SignalException => :never)`, including while blocked in operations like `Thread::Queue#pop`. This is inconsistent with other asynchronous exception delivery paths, such as `Thread#raise`, and with other default signal exceptions such as SIGTERM. Those paths enqueue a pending interrupt on the target thread, allowing `Thread.handle_interrupt` to defer delivery until the configured point. ## Background This was discovered while working on `Async`/`Async::Container` signal handling. Async relies on `Thread.handle_interrupt` and scheduler-level interruption boundaries to keep signal delivery deterministic. The current default SIGINT path means a Ctrl-C can escape a masked section and interrupt internal synchronization code, which can break graceful shutdown behavior. A minimal reproduction is: ```ruby waiting = Thread::Queue.new release = Thread::Queue.new inner = false Thread.new do waiting.pop Process.kill(:INT, Process.pid) release.push(true) end begin Thread.handle_interrupt(SignalException => :never) do begin waiting.push(true) release.pop rescue Interrupt inner = true raise end end rescue Interrupt puts "outer" end puts inner ``` Expected output: ```text outer false ``` Current affected behavior can produce `inner == true`, meaning the `Interrupt` was delivered inside the masked region. ## Proposed fix Route default SIGINT through the same pending-interrupt mechanism as other default signal exceptions, while preserving the traditional exception class and no-message `Interrupt` object. Concretely, the PR replaces the direct `rb_interrupt()` default SIGINT path with a helper that enqueues an `Interrupt` on the main thread and wakes that thread. This makes default SIGINT respect `Thread.handle_interrupt(SignalException => :never)` in the same way as default SIGTERM/SignalException and `Thread#raise`. ## Pull request PR: https://github.com/ruby/ruby/pull/17533 The PR includes: 1. A failing regression test for default SIGINT with `Thread.handle_interrupt`. 2. The implementation change routing default SIGINT through the pending interrupt queue. 3. Ruby/spec coverage for default SIGINT -> `Interrupt` and default SIGTERM -> `SignalException` being maskable by `Thread.handle_interrupt`. Once accepted, I would like to request backports for supported Ruby branches where this default SIGINT behavior is present. -- https://bugs.ruby-lang.org/
Issue #22133 has been updated by ko1 (Koichi Sasada). There are no objection, so let's fix it. ---------------------------------------- Bug #22133: Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking. https://bugs.ruby-lang.org/issues/22133#change-117966 * Author: ioquatix (Samuel Williams) * Status: Open * Assignee: ioquatix (Samuel Williams) * Backport: 3.3: REQUIRED, 3.4: REQUIRED, 4.0: REQUIRED ---------------------------------------- Ruby's default SIGINT handling currently bypasses `Thread.handle_interrupt` masking. When a process receives SIGINT with the default Ruby handler installed, CRuby calls `rb_interrupt()` directly. That can raise `Interrupt` immediately inside code wrapped by `Thread.handle_interrupt(SignalException => :never)`, including while blocked in operations like `Thread::Queue#pop`. This is inconsistent with other asynchronous exception delivery paths, such as `Thread#raise`, and with other default signal exceptions such as SIGTERM. Those paths enqueue a pending interrupt on the target thread, allowing `Thread.handle_interrupt` to defer delivery until the configured point. ## Background This was discovered while working on `Async`/`Async::Container` signal handling. Async relies on `Thread.handle_interrupt` and scheduler-level interruption boundaries to keep signal delivery deterministic. The current default SIGINT path means a Ctrl-C can escape a masked section and interrupt internal synchronization code, which can break graceful shutdown behavior. A minimal reproduction is: ```ruby waiting = Thread::Queue.new release = Thread::Queue.new inner = false Thread.new do waiting.pop Process.kill(:INT, Process.pid) release.push(true) end begin Thread.handle_interrupt(SignalException => :never) do begin waiting.push(true) release.pop rescue Interrupt inner = true raise end end rescue Interrupt puts "outer" end puts inner ``` Expected output: ```text outer false ``` Current affected behavior can produce `inner == true`, meaning the `Interrupt` was delivered inside the masked region. ## Proposed fix Route default SIGINT through the same pending-interrupt mechanism as other default signal exceptions, while preserving the traditional exception class and no-message `Interrupt` object. Concretely, the PR replaces the direct `rb_interrupt()` default SIGINT path with a helper that enqueues an `Interrupt` on the main thread and wakes that thread. This makes default SIGINT respect `Thread.handle_interrupt(SignalException => :never)` in the same way as default SIGTERM/SignalException and `Thread#raise`. ## Pull request PR: https://github.com/ruby/ruby/pull/17533 The PR includes: 1. A failing regression test for default SIGINT with `Thread.handle_interrupt`. 2. The implementation change routing default SIGINT through the pending interrupt queue. 3. Ruby/spec coverage for default SIGINT -> `Interrupt` and default SIGTERM -> `SignalException` being maskable by `Thread.handle_interrupt`. Once accepted, I would like to request backports for supported Ruby branches where this default SIGINT behavior is present. -- https://bugs.ruby-lang.org/
Issue #22133 has been updated by headius (Charles Nutter). JRuby does not by default trap `SIGINT` to raise `Interrupt`, since that would interfere with the JVM's own clean shutdown connected to `SIGINT` by default. Users can opt into that behavior if they want. If a JRuby user traps `SIGINT` and chooses to raise `Interrupt`, they would be subject to `Thread.handle_interrupt` controls like any other Ruby exception. ---------------------------------------- Bug #22133: Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking. https://bugs.ruby-lang.org/issues/22133#change-117984 * Author: ioquatix (Samuel Williams) * Status: Open * Assignee: ioquatix (Samuel Williams) * Backport: 3.3: REQUIRED, 3.4: REQUIRED, 4.0: REQUIRED ---------------------------------------- Ruby's default SIGINT handling currently bypasses `Thread.handle_interrupt` masking. When a process receives SIGINT with the default Ruby handler installed, CRuby calls `rb_interrupt()` directly. That can raise `Interrupt` immediately inside code wrapped by `Thread.handle_interrupt(SignalException => :never)`, including while blocked in operations like `Thread::Queue#pop`. This is inconsistent with other asynchronous exception delivery paths, such as `Thread#raise`, and with other default signal exceptions such as SIGTERM. Those paths enqueue a pending interrupt on the target thread, allowing `Thread.handle_interrupt` to defer delivery until the configured point. ## Background This was discovered while working on `Async`/`Async::Container` signal handling. Async relies on `Thread.handle_interrupt` and scheduler-level interruption boundaries to keep signal delivery deterministic. The current default SIGINT path means a Ctrl-C can escape a masked section and interrupt internal synchronization code, which can break graceful shutdown behavior. A minimal reproduction is: ```ruby waiting = Thread::Queue.new release = Thread::Queue.new inner = false Thread.new do waiting.pop Process.kill(:INT, Process.pid) release.push(true) end begin Thread.handle_interrupt(SignalException => :never) do begin waiting.push(true) release.pop rescue Interrupt inner = true raise end end rescue Interrupt puts "outer" end puts inner ``` Expected output: ```text outer false ``` Current affected behavior can produce `inner == true`, meaning the `Interrupt` was delivered inside the masked region. ## Proposed fix Route default SIGINT through the same pending-interrupt mechanism as other default signal exceptions, while preserving the traditional exception class and no-message `Interrupt` object. Concretely, the PR replaces the direct `rb_interrupt()` default SIGINT path with a helper that enqueues an `Interrupt` on the main thread and wakes that thread. This makes default SIGINT respect `Thread.handle_interrupt(SignalException => :never)` in the same way as default SIGTERM/SignalException and `Thread#raise`. ## Pull request PR: https://github.com/ruby/ruby/pull/17533 The PR includes: 1. A failing regression test for default SIGINT with `Thread.handle_interrupt`. 2. The implementation change routing default SIGINT through the pending interrupt queue. 3. Ruby/spec coverage for default SIGINT -> `Interrupt` and default SIGTERM -> `SignalException` being maskable by `Thread.handle_interrupt`. Once accepted, I would like to request backports for supported Ruby branches where this default SIGINT behavior is present. -- https://bugs.ruby-lang.org/
Issue #22133 has been updated by k0kubun (Takashi Kokubun). Backport changed from 3.3: REQUIRED, 3.4: REQUIRED, 4.0: REQUIRED to 3.3: REQUIRED, 3.4: REQUIRED, 4.0: DONE ruby_4_0 commit:10b2ee7f1c0b860ad0e9b55619c52743a5818f0e merged revision(s) commit:ef67c377508ea3a07516c1fa413a038b5268fc1c. ---------------------------------------- Bug #22133: Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking. https://bugs.ruby-lang.org/issues/22133#change-118081 * Author: ioquatix (Samuel Williams) * Status: Closed * Assignee: ioquatix (Samuel Williams) * Backport: 3.3: REQUIRED, 3.4: REQUIRED, 4.0: DONE ---------------------------------------- Ruby's default SIGINT handling currently bypasses `Thread.handle_interrupt` masking. When a process receives SIGINT with the default Ruby handler installed, CRuby calls `rb_interrupt()` directly. That can raise `Interrupt` immediately inside code wrapped by `Thread.handle_interrupt(SignalException => :never)`, including while blocked in operations like `Thread::Queue#pop`. This is inconsistent with other asynchronous exception delivery paths, such as `Thread#raise`, and with other default signal exceptions such as SIGTERM. Those paths enqueue a pending interrupt on the target thread, allowing `Thread.handle_interrupt` to defer delivery until the configured point. ## Background This was discovered while working on `Async`/`Async::Container` signal handling. Async relies on `Thread.handle_interrupt` and scheduler-level interruption boundaries to keep signal delivery deterministic. The current default SIGINT path means a Ctrl-C can escape a masked section and interrupt internal synchronization code, which can break graceful shutdown behavior. A minimal reproduction is: ```ruby waiting = Thread::Queue.new release = Thread::Queue.new inner = false Thread.new do waiting.pop Process.kill(:INT, Process.pid) release.push(true) end begin Thread.handle_interrupt(SignalException => :never) do begin waiting.push(true) release.pop rescue Interrupt inner = true raise end end rescue Interrupt puts "outer" end puts inner ``` Expected output: ```text outer false ``` Current affected behavior can produce `inner == true`, meaning the `Interrupt` was delivered inside the masked region. ## Proposed fix Route default SIGINT through the same pending-interrupt mechanism as other default signal exceptions, while preserving the traditional exception class and no-message `Interrupt` object. Concretely, the PR replaces the direct `rb_interrupt()` default SIGINT path with a helper that enqueues an `Interrupt` on the main thread and wakes that thread. This makes default SIGINT respect `Thread.handle_interrupt(SignalException => :never)` in the same way as default SIGTERM/SignalException and `Thread#raise`. ## Pull request PR: https://github.com/ruby/ruby/pull/17533 The PR includes: 1. A failing regression test for default SIGINT with `Thread.handle_interrupt`. 2. The implementation change routing default SIGINT through the pending interrupt queue. 3. Ruby/spec coverage for default SIGINT -> `Interrupt` and default SIGTERM -> `SignalException` being maskable by `Thread.handle_interrupt`. Once accepted, I would like to request backports for supported Ruby branches where this default SIGINT behavior is present. -- https://bugs.ruby-lang.org/
Issue #22133 has been updated by nagachika (Tomoyuki Chikanaga). Backport changed from 3.3: REQUIRED, 3.4: REQUIRED, 4.0: DONE to 3.3: REQUIRED, 3.4: DONE, 4.0: DONE ruby_3_4 commit:d6dbd1dc4c3c8a82fedb20b59c5750b8a5816091 merged revision(s) commit:ef67c377508ea3a07516c1fa413a038b5268fc1c. ---------------------------------------- Bug #22133: Ruby's default SIGINT handling ignores `Thread.handle_interrupt` masking. https://bugs.ruby-lang.org/issues/22133#change-118456 * Author: ioquatix (Samuel Williams) * Status: Closed * Assignee: ioquatix (Samuel Williams) * Backport: 3.3: REQUIRED, 3.4: DONE, 4.0: DONE ---------------------------------------- Ruby's default SIGINT handling currently bypasses `Thread.handle_interrupt` masking. When a process receives SIGINT with the default Ruby handler installed, CRuby calls `rb_interrupt()` directly. That can raise `Interrupt` immediately inside code wrapped by `Thread.handle_interrupt(SignalException => :never)`, including while blocked in operations like `Thread::Queue#pop`. This is inconsistent with other asynchronous exception delivery paths, such as `Thread#raise`, and with other default signal exceptions such as SIGTERM. Those paths enqueue a pending interrupt on the target thread, allowing `Thread.handle_interrupt` to defer delivery until the configured point. ## Background This was discovered while working on `Async`/`Async::Container` signal handling. Async relies on `Thread.handle_interrupt` and scheduler-level interruption boundaries to keep signal delivery deterministic. The current default SIGINT path means a Ctrl-C can escape a masked section and interrupt internal synchronization code, which can break graceful shutdown behavior. A minimal reproduction is: ```ruby waiting = Thread::Queue.new release = Thread::Queue.new inner = false Thread.new do waiting.pop Process.kill(:INT, Process.pid) release.push(true) end begin Thread.handle_interrupt(SignalException => :never) do begin waiting.push(true) release.pop rescue Interrupt inner = true raise end end rescue Interrupt puts "outer" end puts inner ``` Expected output: ```text outer false ``` Current affected behavior can produce `inner == true`, meaning the `Interrupt` was delivered inside the masked region. ## Proposed fix Route default SIGINT through the same pending-interrupt mechanism as other default signal exceptions, while preserving the traditional exception class and no-message `Interrupt` object. Concretely, the PR replaces the direct `rb_interrupt()` default SIGINT path with a helper that enqueues an `Interrupt` on the main thread and wakes that thread. This makes default SIGINT respect `Thread.handle_interrupt(SignalException => :never)` in the same way as default SIGTERM/SignalException and `Thread#raise`. ## Pull request PR: https://github.com/ruby/ruby/pull/17533 The PR includes: 1. A failing regression test for default SIGINT with `Thread.handle_interrupt`. 2. The implementation change routing default SIGINT through the pending interrupt queue. 3. Ruby/spec coverage for default SIGINT -> `Interrupt` and default SIGTERM -> `SignalException` being maskable by `Thread.handle_interrupt`. Once accepted, I would like to request backports for supported Ruby branches where this default SIGINT behavior is present. -- https://bugs.ruby-lang.org/
participants (7)
-
Eregon (Benoit Daloze) -
headius (Charles Nutter) -
ioquatix (Samuel Williams) -
k0kubun (Takashi Kokubun) -
ko1 (Koichi Sasada) -
kosaki (Motohiro KOSAKI) -
nagachika (Tomoyuki Chikanaga)