From: jean.boussier@... Date: 2021-04-13T10:52:10+00:00 Subject: [ruby-core:103421] [Ruby master Feature#17795] `before_fork` and `after_fork` callback API Issue #17795 has been updated by byroot (Jean Boussier). @jeremyevans0 I understand your point, and I agree that in an ideal world this wouldn't be necessary. However pragmatically speaking, `getpid(2)` checks are about as old as `fork(2)`. Even the [`glibc` changelog](https://sourceware.org/glibc/wiki/Release/2.25#pid_cache_removal) acknowledge it: > Applications performing getpid() calls in a loop will see the worst case performance degradation as the library call will perform a system call at each invocation. And suggest to use `pthread_atfork()` instead: > Applications performing getpid() in a loop that need to do some level of fork()-based invalidation can instead use pthread_atfork() to register handlers to handle the invalidation. I think it's a clear indication that this need is nothing new. > The main issue I see with this is the potential for misuse. I agree, but this argument could apply to a large part of Ruby, I'm not sure it's relevant. ---------------------------------------- Feature #17795: `before_fork` and `after_fork` callback API https://bugs.ruby-lang.org/issues/17795#change-91512 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- Replaces: https://bugs.ruby-lang.org/issues/5446 ### Context Ruby code in production is very often running in a forking setup (puma, unicorn, etc), and it is common some types of libraries to need to know when the Ruby process was forked. For instance: - Most database clients, ORMs or other libraries keeping a connection pool might need to close connections before the fork happens. - Libraries relying on some kind of dispatcher thread might need to restart the thread in the forked children, and clear any internal buffer (e.g. statsd clients, newrelic_rpm). **This need is only for forking the whole ruby process, extensions doing a `fork(2) + exec(2)` combo etc are not a concern, this aim at only catching `kernel.fork`, `Process.fork` and maybe `Process.daemon`.**. The use case is for forks that end up executing Ruby code. ### Current solutions Right now this use case is handled in several ways. #### Rely on the integrating code to call a `before_fork` or `after_fork` callback. Some libraries simply rely on documentation and require the user to use the hooks provided by their forking server. Examples: - Sequel: http://sequel.jeremyevans.net/rdoc/files/doc/fork_safety_rdoc.html - Rails's Active Record: https://devcenter.heroku.com/articles/concurrency-and-database-connections#multi-process-servers - ScoutAPM (it tries to detect popular forking setup and register itself): https://github.com/scoutapp/scout_apm_ruby/blob/fa83793b9e8d2f9a32c920f59b57d7f198f466b8/lib/scout_apm/environment.rb#L142-L146 - NewRelic RPM (similarly tries to register to popular forking setups): https://www.rubydoc.info/github/newrelic/rpm/NewRelic%2FAgent:after_fork #### Continuously check `Process.pid` Some libraries chose to instead keep the process PID in a variable, and to regularly compare it to `Process.pid` to detect forked children. Unfortunately `Process.pid` is relatively slow on Linux, and these checks tend to be in tight loops, so it's not uncommon when using these libraries to spend `1` or `2%` of runtime in `Process.pid`. Examples: - Rails's Active Record used to check `Process.pid` https://github.com/Shopify/rails/blob/411ccbdab2608c62aabdb320d52cb02d446bb39c/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb#L946, it still does but a bit less: https://github.com/rails/rails/pull/41850 - the `Typhoeus` HTTP client: https://github.com/typhoeus/typhoeus/blob/a345545e5e4ac0522b883fe0cf19e5e2e807b4b0/lib/typhoeus/pool.rb#L34-L42 - Redis client: https://github.com/redis/redis-rb/blob/6542934f01b9c390ee450bd372209a04bc3a239b/lib/redis/client.rb#L384 - Some older versions of NewRelic RPM: https://github.com/opendna/scoreranking-api/blob/8fba96d23b4d3e6b64f625079c184f3a292bbc12/vendor/gems/ruby/1.9.1/gems/newrelic_rpm-3.7.3.204/lib/new_relic/agent/harvester.rb#L39-L41 #### Continuously check `Thread#alive?` Similar to checking `Process.pid`, but for the background thread use case. `Thread#alive?` is regularly checked, and if the thread is dead, it is assumed that the process was forked. It's much less costly than a `Process.pid`, but also a bit less reliable as the thread could have died for other reasons. It also delays re-creating the thread to the next check rather than immediately upon forking. Examples: - `statsd-instrument`: https://github.com/Shopify/statsd-instrument/blob/0445cca46e29aa48e9f1efec7c72352aff7ec931/lib/statsd/instrument/batched_udp_sink.rb#L63 #### Decorate `Kernel.fork` and `Process.fork` Another solution is to prepend a module in `Process` and `Kernel`, to decorate the fork method and implement your own callback. It works well, but is made difficult by `Kernel.fork`. Examples: - Active Support: https://github.com/rails/rails/blob/9aed3dcdfea6b64c18035f8e2622c474ba499846/activesupport/lib/active_support/fork_tracker.rb - `dd-trace-rb`: https://github.com/DataDog/dd-trace-rb/blob/793946146b4709289cfd459f3b68e8227a9f5fa7/lib/ddtrace/profiling/ext/forking.rb - To some extent, `nakayoshi_fork` decorates the `fork` method: https://github.com/ko1/nakayoshi_fork/blob/19ef5efc51e0ae51d7f5f37a0b785309bf16e97f/lib/nakayoshi_fork.rb ### Proposals I see two possible features to improve this situation: #### `Process.before_fork` and `Process.after_fork` callbacks One solution would be for Ruby to expose a callback API for these two events, similar to `Kernel.at_exit`. #### Make `Kernel.fork` a delegator A simpler change would be to just make `Kernel.fork` a delegator to `Process.fork`. This would make it much easier to prepend a module on `Process` for each library to implement its own callback. Proposed patch: https://github.com/ruby/ruby/pull/4361 -- https://bugs.ruby-lang.org/ Unsubscribe: