Project

General

Profile

Actions

Bug #20393

closed

`after_fork_ruby` clears all pending interrupts for both parent and child process.

Bug #20393: `after_fork_ruby` clears all pending interrupts for both parent and child process.

Added by ioquatix (Samuel Williams) over 1 year ago. Updated over 1 year ago.


Description

In the following program, the behaviour of the parent process is affected by whether Process.fork is invoked or not.

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:

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:

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 (Koichi Sasada)


Related issues 1 (0 open1 closed)

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #1 [ruby-core:117323]

@nobu (Nobuyoshi Nakada) can you please review https://github.com/ruby/ruby/pull/10365 thanks!

Some more background found by @mame (Yusuke Endoh):

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #2

  • Related to Bug #885: Thread.new{fork{}} added

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #3 [ruby-core:117333]

  • 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.

Updated by k0kubun (Takashi Kokubun) over 1 year ago Actions #4 [ruby-core:118071]

  • 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

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago Actions #5 [ruby-core:118459]

  • 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
Actions

Also available in: PDF Atom