[ruby-core:117321] [Ruby master Bug#20393] `after_fork_ruby` clears all pending interrupts for both parent and child process.

Issue #20393 has been reported by ioquatix (Samuel Williams). ---------------------------------------- Bug #20393: `after_fork_ruby` clears all pending interrupts for both parent and child process. https://bugs.ruby-lang.org/issues/20393 * Author: ioquatix (Samuel Williams) * Status: Open * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED ---------------------------------------- In the following program, the behaviour of the parent process is affected by whether `Process.fork` is invoked or not. ```ruby Thread.handle_interrupt(RuntimeError => :never) do Thread.current.raise(RuntimeError, "Queued error") puts "Pending interrupt: #{Thread.pending_interrupt?}" # true pid = Process.fork do puts "Pending interrupt (child process): #{Thread.pending_interrupt?}" Thread.handle_interrupt(RuntimeError => :immediate){} end _, status = Process.waitpid2(pid) puts "Child process status: #{status.inspect}" puts "Pending interrupt: #{Thread.pending_interrupt?}" # false end puts "Exiting..." ``` I don't think the parent process pending interrupts should be cleared by `after_fork_ruby`: ```c static void after_fork_ruby(rb_pid_t pid) { rb_threadptr_pending_interrupt_clear(GET_THREAD()); if (pid == 0) { // child clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` How about this implementation: ```c static void after_fork_ruby(rb_pid_t pid) { if (pid == 0) { // child rb_threadptr_pending_interrupt_clear(GET_THREAD()); clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` cc @ko1 -- https://bugs.ruby-lang.org/

Issue #20393 has been updated by ioquatix (Samuel Williams). @nobu can you please review <https://github.com/ruby/ruby/pull/10365> thanks! Some more background found by @mame: - Originally introduced in <https://github.com/ruby/ruby/commit/2d5061bd98a59fc1b9a477074f8f7a3500db8342>. It looks like the bug exists here too, as `GET_THREAD()->thrown_errinfo = 0` would be cleared both in the child and parent process. - Modified here to use a list of pending interrupts: <https://github.com/ruby/ruby/commit/28144433b2f951279552b39bc358769a5267e26a> - Renamed from `async_errinfo` to `pending_interrupts`: <https://github.com/ruby/ruby/commit/0f9b33c793f225c1b817d73e5c915050c429edc4> ---------------------------------------- Bug #20393: `after_fork_ruby` clears all pending interrupts for both parent and child process. https://bugs.ruby-lang.org/issues/20393#change-107466 * Author: ioquatix (Samuel Williams) * Status: Open * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED ---------------------------------------- In the following program, the behaviour of the parent process is affected by whether `Process.fork` is invoked or not. ```ruby Thread.handle_interrupt(RuntimeError => :never) do Thread.current.raise(RuntimeError, "Queued error") puts "Pending interrupt: #{Thread.pending_interrupt?}" # true pid = Process.fork do puts "Pending interrupt (child process): #{Thread.pending_interrupt?}" Thread.handle_interrupt(RuntimeError => :immediate){} end _, status = Process.waitpid2(pid) puts "Child process status: #{status.inspect}" puts "Pending interrupt: #{Thread.pending_interrupt?}" # false end puts "Exiting..." ``` I don't think the parent process pending interrupts should be cleared by `after_fork_ruby`: ```c static void after_fork_ruby(rb_pid_t pid) { rb_threadptr_pending_interrupt_clear(GET_THREAD()); if (pid == 0) { // child clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` How about this implementation: ```c static void after_fork_ruby(rb_pid_t pid) { if (pid == 0) { // child rb_threadptr_pending_interrupt_clear(GET_THREAD()); clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` cc @ko1 -- https://bugs.ruby-lang.org/

Issue #20393 has been updated by ioquatix (Samuel Williams). Status changed from Open to Closed Nobu approved this change on the PR, so I've merged it: https://github.com/ruby/ruby/commit/a7ff264477105b5dc0ade6facad4176a1b73df0b I'll introduce a separate PR to add the test to ruby-spec. ---------------------------------------- Bug #20393: `after_fork_ruby` clears all pending interrupts for both parent and child process. https://bugs.ruby-lang.org/issues/20393#change-107477 * Author: ioquatix (Samuel Williams) * Status: Closed * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED ---------------------------------------- In the following program, the behaviour of the parent process is affected by whether `Process.fork` is invoked or not. ```ruby Thread.handle_interrupt(RuntimeError => :never) do Thread.current.raise(RuntimeError, "Queued error") puts "Pending interrupt: #{Thread.pending_interrupt?}" # true pid = Process.fork do puts "Pending interrupt (child process): #{Thread.pending_interrupt?}" Thread.handle_interrupt(RuntimeError => :immediate){} end _, status = Process.waitpid2(pid) puts "Child process status: #{status.inspect}" puts "Pending interrupt: #{Thread.pending_interrupt?}" # false end puts "Exiting..." ``` I don't think the parent process pending interrupts should be cleared by `after_fork_ruby`: ```c static void after_fork_ruby(rb_pid_t pid) { rb_threadptr_pending_interrupt_clear(GET_THREAD()); if (pid == 0) { // child clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` How about this implementation: ```c static void after_fork_ruby(rb_pid_t pid) { if (pid == 0) { // child rb_threadptr_pending_interrupt_clear(GET_THREAD()); clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` cc @ko1 -- https://bugs.ruby-lang.org/

Issue #20393 has been updated by k0kubun (Takashi Kokubun). Backport changed from 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED to 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE ruby_3_3 commit:6e46a363a8f29d93cf6992805ee67d029cea030f merged revision(s) commit:a7ff264477105b5dc0ade6facad4176a1b73df0b. ---------------------------------------- Bug #20393: `after_fork_ruby` clears all pending interrupts for both parent and child process. https://bugs.ruby-lang.org/issues/20393#change-108498 * Author: ioquatix (Samuel Williams) * Status: Closed * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE ---------------------------------------- In the following program, the behaviour of the parent process is affected by whether `Process.fork` is invoked or not. ```ruby Thread.handle_interrupt(RuntimeError => :never) do Thread.current.raise(RuntimeError, "Queued error") puts "Pending interrupt: #{Thread.pending_interrupt?}" # true pid = Process.fork do puts "Pending interrupt (child process): #{Thread.pending_interrupt?}" Thread.handle_interrupt(RuntimeError => :immediate){} end _, status = Process.waitpid2(pid) puts "Child process status: #{status.inspect}" puts "Pending interrupt: #{Thread.pending_interrupt?}" # false end puts "Exiting..." ``` I don't think the parent process pending interrupts should be cleared by `after_fork_ruby`: ```c static void after_fork_ruby(rb_pid_t pid) { rb_threadptr_pending_interrupt_clear(GET_THREAD()); if (pid == 0) { // child clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` How about this implementation: ```c static void after_fork_ruby(rb_pid_t pid) { if (pid == 0) { // child rb_threadptr_pending_interrupt_clear(GET_THREAD()); clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` cc @ko1 -- https://bugs.ruby-lang.org/

Issue #20393 has been updated by nagachika (Tomoyuki Chikanaga). Backport changed from 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE to 3.0: REQUIRED, 3.1: REQUIRED, 3.2: DONE, 3.3: DONE ruby_3_2 commit:5577e5d396cc8f062833b67d6280db6cc8501e7a merged revision(s) commit:a7ff264477105b5dc0ade6facad4176a1b73df0b. ---------------------------------------- Bug #20393: `after_fork_ruby` clears all pending interrupts for both parent and child process. https://bugs.ruby-lang.org/issues/20393#change-108973 * Author: ioquatix (Samuel Williams) * Status: Closed * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: REQUIRED, 3.1: REQUIRED, 3.2: DONE, 3.3: DONE ---------------------------------------- In the following program, the behaviour of the parent process is affected by whether `Process.fork` is invoked or not. ```ruby Thread.handle_interrupt(RuntimeError => :never) do Thread.current.raise(RuntimeError, "Queued error") puts "Pending interrupt: #{Thread.pending_interrupt?}" # true pid = Process.fork do puts "Pending interrupt (child process): #{Thread.pending_interrupt?}" Thread.handle_interrupt(RuntimeError => :immediate){} end _, status = Process.waitpid2(pid) puts "Child process status: #{status.inspect}" puts "Pending interrupt: #{Thread.pending_interrupt?}" # false end puts "Exiting..." ``` I don't think the parent process pending interrupts should be cleared by `after_fork_ruby`: ```c static void after_fork_ruby(rb_pid_t pid) { rb_threadptr_pending_interrupt_clear(GET_THREAD()); if (pid == 0) { // child clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` How about this implementation: ```c static void after_fork_ruby(rb_pid_t pid) { if (pid == 0) { // child rb_threadptr_pending_interrupt_clear(GET_THREAD()); clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` cc @ko1 -- https://bugs.ruby-lang.org/

Issue #20393 has been updated by ioquatix (Samuel Williams). Thanks! ---------------------------------------- Bug #20393: `after_fork_ruby` clears all pending interrupts for both parent and child process. https://bugs.ruby-lang.org/issues/20393#change-108975 * Author: ioquatix (Samuel Williams) * Status: Closed * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: REQUIRED, 3.1: REQUIRED, 3.2: DONE, 3.3: DONE ---------------------------------------- In the following program, the behaviour of the parent process is affected by whether `Process.fork` is invoked or not. ```ruby Thread.handle_interrupt(RuntimeError => :never) do Thread.current.raise(RuntimeError, "Queued error") puts "Pending interrupt: #{Thread.pending_interrupt?}" # true pid = Process.fork do puts "Pending interrupt (child process): #{Thread.pending_interrupt?}" Thread.handle_interrupt(RuntimeError => :immediate){} end _, status = Process.waitpid2(pid) puts "Child process status: #{status.inspect}" puts "Pending interrupt: #{Thread.pending_interrupt?}" # false end puts "Exiting..." ``` I don't think the parent process pending interrupts should be cleared by `after_fork_ruby`: ```c static void after_fork_ruby(rb_pid_t pid) { rb_threadptr_pending_interrupt_clear(GET_THREAD()); if (pid == 0) { // child clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` How about this implementation: ```c static void after_fork_ruby(rb_pid_t pid) { if (pid == 0) { // child rb_threadptr_pending_interrupt_clear(GET_THREAD()); clear_pid_cache(); rb_thread_atfork(); } else { // parent after_exec(); } } ``` cc @ko1 -- https://bugs.ruby-lang.org/
participants (3)
-
ioquatix (Samuel Williams)
-
k0kubun (Takashi Kokubun)
-
nagachika (Tomoyuki Chikanaga)