[ruby-core:126379] [Ruby Feature#22243] Change fork behavior when Process._fork raises an exception during fork with a block argument
Issue #22243 has been reported by rpeng (Richard Peng). ---------------------------------------- Feature #22243: Change fork behavior when Process._fork raises an exception during fork with a block argument https://bugs.ruby-lang.org/issues/22243 * Author: rpeng (Richard Peng) * Status: Open ---------------------------------------- # Problem Ruby exposes Process.fork in two formats - one with block arguments and one without. When a block is passed in, the implicit contract is that a child process will terminate once the block ends. However, it is possible for the child to "escape" the enclosed block in certain circumstances when Process._fork raises an exception. Here's a demonstration of what may occur: ```ruby module BadForkTracker def _fork pid = super if pid == 0 raise "exception during fork in child" # end pid end end Process.singleton_class.prepend(BadForkTracker) loop do sleep(1) puts "I'm parent #{Process.pid}" fork do puts "I'm child #{Process.pid} and I'm exiting" end rescue StandardError => e puts "#{Process.pid} Received Error: #{e}" # <- child ends up in this block, then resumes the loop. most users will not be expecting this. end ``` In practice, there are many libraries that implement "fork tracking" (active_support, datadog, connection_pool, to name a few) by prepending to the `Process` module. Many of which are designed to run user specified callbacks which increases the surface area of errors occurring. This means that on _fork, there's a good chance that something will throw and cause a child to resume execution in the "parent" context. In addition, many workloads (sidekiq, puma) have global rescues that are similar to the example case I provided above, which would unintentionally cause the child to continue execution. # Proposal I am proposing a change in behavior: when Process.fork is called with a block, if Process._fork raises an exception in the child, immediately terminate the child with an error exit code (instead of relying on an enclosing rescue to handle it). This guarantees that a child cannot escape out of the block it was meant to execute. In my opinion, this is more in line with how libraries generally expect this to work (e.g. there are very few libraries that are being defensive and attempting not to throw in `def _fork` overrides). -- https://bugs.ruby-lang.org/
Issue #22243 has been updated by luke-gru (Luke Gruber). Tracker changed from Feature to Bug Backport set to 4.0: REQUIRED Thanks for the report. I'm changing this from feature request to bug because this behavior is very surprising and shouldn't happen. ---------------------------------------- Bug #22243: Change fork behavior when Process._fork raises an exception during fork with a block argument https://bugs.ruby-lang.org/issues/22243#change-118517 * Author: rpeng (Richard Peng) * Status: Open * Backport: 4.0: REQUIRED ---------------------------------------- # Problem Ruby exposes Process.fork in two formats - one with block arguments and one without. When a block is passed in, the implicit contract is that a child process will terminate once the block ends. However, it is possible for the child to "escape" the enclosed block in certain circumstances when Process._fork raises an exception. Here's a demonstration of what may occur: ```ruby module BadForkTracker def _fork pid = super if pid == 0 raise "exception during fork in child" # end pid end end Process.singleton_class.prepend(BadForkTracker) loop do sleep(1) puts "I'm parent #{Process.pid}" fork do puts "I'm child #{Process.pid} and I'm exiting" end rescue StandardError => e puts "#{Process.pid} Received Error: #{e}" # <- child ends up in this block, then resumes the loop. most users will not be expecting this. end ``` In practice, there are many libraries that implement "fork tracking" (active_support, datadog, connection_pool, to name a few) by prepending to the `Process` module. Many of which are designed to run user specified callbacks which increases the surface area of errors occurring. This means that on _fork, there's a good chance that something will throw and cause a child to resume execution in the "parent" context. In addition, many workloads (sidekiq, puma) have global rescues that are similar to the example case I provided above, which would unintentionally cause the child to continue execution. # Proposal I am proposing a change in behavior: when Process.fork is called with a block, if Process._fork raises an exception in the child, immediately terminate the child with an error exit code (instead of relying on an enclosing rescue to handle it). This guarantees that a child cannot escape out of the block it was meant to execute. In my opinion, this is more in line with how libraries generally expect this to work (e.g. there are very few libraries that are being defensive and attempting not to throw in `def _fork` overrides). -- https://bugs.ruby-lang.org/
Issue #22243 has been updated by nobu (Nobuyoshi Nakada). [GH-18316](https://github.com/ruby/ruby/pull/18316) ---------------------------------------- Bug #22243: Change fork behavior when Process._fork raises an exception during fork with a block argument https://bugs.ruby-lang.org/issues/22243#change-118518 * Author: rpeng (Richard Peng) * Status: Open * Backport: 4.0: REQUIRED ---------------------------------------- # Problem Ruby exposes Process.fork in two formats - one with block arguments and one without. When a block is passed in, the implicit contract is that a child process will terminate once the block ends. However, it is possible for the child to "escape" the enclosed block in certain circumstances when Process._fork raises an exception. Here's a demonstration of what may occur: ```ruby module BadForkTracker def _fork pid = super if pid == 0 raise "exception during fork in child" # end pid end end Process.singleton_class.prepend(BadForkTracker) loop do sleep(1) puts "I'm parent #{Process.pid}" fork do puts "I'm child #{Process.pid} and I'm exiting" end rescue StandardError => e puts "#{Process.pid} Received Error: #{e}" # <- child ends up in this block, then resumes the loop. most users will not be expecting this. end ``` In practice, there are many libraries that implement "fork tracking" (active_support, datadog, connection_pool, to name a few) by prepending to the `Process` module. Many of which are designed to run user specified callbacks which increases the surface area of errors occurring. This means that on _fork, there's a good chance that something will throw and cause a child to resume execution in the "parent" context. In addition, many workloads (sidekiq, puma) have global rescues that are similar to the example case I provided above, which would unintentionally cause the child to continue execution. # Proposal I am proposing a change in behavior: when Process.fork is called with a block, if Process._fork raises an exception in the child, immediately terminate the child with an error exit code (instead of relying on an enclosing rescue to handle it). This guarantees that a child cannot escape out of the block it was meant to execute. In my opinion, this is more in line with how libraries generally expect this to work (e.g. there are very few libraries that are being defensive and attempting not to throw in `def _fork` overrides). -- https://bugs.ruby-lang.org/
Issue #22243 has been updated by rpeng (Richard Peng). nobu (Nobuyoshi Nakada) wrote in #note-3:
Applied in changeset commit:git|fff4e38e4a44a1e9748e9f47b87cbe393b4495a1.
---------- [Bug #22243] Ensure forked process to terminate
The forked process should terminate reliably even if an exception occurs.
Thanks everyone for the quick triage and fix!! ---------------------------------------- Bug #22243: Change fork behavior when Process._fork raises an exception during fork with a block argument https://bugs.ruby-lang.org/issues/22243#change-118521 * Author: rpeng (Richard Peng) * Status: Closed * Backport: 4.0: REQUIRED ---------------------------------------- # Problem Ruby exposes Process.fork in two formats - one with block arguments and one without. When a block is passed in, the implicit contract is that a child process will terminate once the block ends. However, it is possible for the child to "escape" the enclosed block in certain circumstances when Process._fork raises an exception. Here's a demonstration of what may occur: ```ruby module BadForkTracker def _fork pid = super if pid == 0 raise "exception during fork in child" # end pid end end Process.singleton_class.prepend(BadForkTracker) loop do sleep(1) puts "I'm parent #{Process.pid}" fork do puts "I'm child #{Process.pid} and I'm exiting" end rescue StandardError => e puts "#{Process.pid} Received Error: #{e}" # <- child ends up in this block, then resumes the loop. most users will not be expecting this. end ``` In practice, there are many libraries that implement "fork tracking" (active_support, datadog, connection_pool, to name a few) by prepending to the `Process` module. Many of which are designed to run user specified callbacks which increases the surface area of errors occurring. This means that on _fork, there's a good chance that something will throw and cause a child to resume execution in the "parent" context. In addition, many workloads (sidekiq, puma) have global rescues that are similar to the example case I provided above, which would unintentionally cause the child to continue execution. # Proposal I am proposing a change in behavior: when Process.fork is called with a block, if Process._fork raises an exception in the child, immediately terminate the child with an error exit code (instead of relying on an enclosing rescue to handle it). This guarantees that a child cannot escape out of the block it was meant to execute. In my opinion, this is more in line with how libraries generally expect this to work (e.g. there are very few libraries that are being defensive and attempting not to throw in `def _fork` overrides). -- https://bugs.ruby-lang.org/
Issue #22243 has been updated by k0kubun (Takashi Kokubun). Backport changed from 4.0: REQUIRED to 4.0: DONE ruby_4_0 commit:e06fe010c81c67a048a4bbd4d3b9e5891b4a5cff. ---------------------------------------- Bug #22243: Change fork behavior when Process._fork raises an exception during fork with a block argument https://bugs.ruby-lang.org/issues/22243#change-119011 * Author: rpeng (Richard Peng) * Status: Closed * Backport: 4.0: DONE ---------------------------------------- # Problem Ruby exposes Process.fork in two formats - one with block arguments and one without. When a block is passed in, the implicit contract is that a child process will terminate once the block ends. However, it is possible for the child to "escape" the enclosed block in certain circumstances when Process._fork raises an exception. Here's a demonstration of what may occur: ```ruby module BadForkTracker def _fork pid = super if pid == 0 raise "exception during fork in child" # end pid end end Process.singleton_class.prepend(BadForkTracker) loop do sleep(1) puts "I'm parent #{Process.pid}" fork do puts "I'm child #{Process.pid} and I'm exiting" end rescue StandardError => e puts "#{Process.pid} Received Error: #{e}" # <- child ends up in this block, then resumes the loop. most users will not be expecting this. end ``` In practice, there are many libraries that implement "fork tracking" (active_support, datadog, connection_pool, to name a few) by prepending to the `Process` module. Many of which are designed to run user specified callbacks which increases the surface area of errors occurring. This means that on _fork, there's a good chance that something will throw and cause a child to resume execution in the "parent" context. In addition, many workloads (sidekiq, puma) have global rescues that are similar to the example case I provided above, which would unintentionally cause the child to continue execution. # Proposal I am proposing a change in behavior: when Process.fork is called with a block, if Process._fork raises an exception in the child, immediately terminate the child with an error exit code (instead of relying on an enclosing rescue to handle it). This guarantees that a child cannot escape out of the block it was meant to execute. In my opinion, this is more in line with how libraries generally expect this to work (e.g. there are very few libraries that are being defensive and attempting not to throw in `def _fork` overrides). -- https://bugs.ruby-lang.org/
participants (4)
-
k0kubun (Takashi Kokubun) -
luke-gru (Luke Gruber)
-
nobu (Nobuyoshi Nakada)
-
rpeng (Richard Peng)