Issue #17795 has been updated by Dan0042 (Daniel DeLorme).


akr (Akira Tanaka) wrote in #note-17:
> Another idea is introducing Process.fork_level which can be used to detec=
t fork instead of getpid.

If that's possible, then it should be equally possible to just [pre-]cache =
`Process.pid` and it would be just as performant, right? Whatever were the =
reasons for pid cache removal in glibc, they would also apply to `fork_leve=
l`. From what I understand above it's not relevant here because calling `fo=
rk(2)` would leave the VM in an unusable state. Since checking the pid is a=
 pattern that already exists, imho it's better to make that pattern perform=
ant than to introduce a new pattern with almost no advantage. I don't think=
 pid reuse/collision is a realistic concern either.

But checking either `Process.pid` or `fork_level` doesn't fix one of the bi=
ggest issues I've had with forking: _finalizers_. If the database object ha=
s a finalizer that closes the connection, you need to prevent that finalize=
r from ever executing in the child process. You can use `fork{ work(); exit=
! }` to prevent finalizers from running on exit, but that also prevents any=
 finalizers that were defined in the child process. So you actually need to=
 use `fork{ at_exit{exit!}; work() }`, but even then it doesn't cover the c=
ase where a finalizer is run when the object is garbage-collected. This is =
a real thorn, and to me it's the main reason why the socket must be closed =
in the child process right after fork.

byroot (Jean Boussier) wrote in #note-23:
> But I (and apparently a bunch of other library writers, since clearly a b=
unch of libraries already do it, and other languages offer that capability)=
 disagree with this stance, and don't understand how Ruby design should be =
influenced by this types of "proper way to do it" arguments.

+1; for me I believe fork safety should be handled by libraries because of =
encapsulation. If you have independant libs, libA that needs to close socke=
ts on fork, and libB that uses fork, these two should remain independant. Y=
ou should not need the app to insert glue code in order for the two libs to=
 play nice together. That really breaks encapsulation and separation of con=
cerns. 2=A2

> Make Kernel.fork a delegator

I have two concerns with this:
1. Some existing gems already override both `Kernel#fork` and `Process.fork=
` in order to setup callbacks; this change would cause the callbacks to be =
triggered twice when using `Kernel#fork`. So it's somewhat backward-incompa=
tible.
2. You need to handle the with-block and without-block forms differently.

So I'd like to make a counter-proposal: what about introducing a separate m=
ethod (let's say `Process._fork_`) which _doesn't accept a block_ and is ca=
lled by the various forking methods, including `IO.popen("-")`. That would =
solve the two concerns above, and it might even be simple enough to be back=
ported.


----------------------------------------
Feature #17795: Around `Process.fork` callbacks API
https://bugs.ruby-lang.org/issues/17795#change-91653

* 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, uni=
corn, etc), and it is common some types of libraries to need to know when t=
he Ruby process was forked. For instance:

  - Most database clients, ORMs or other libraries keeping a connection poo=
l might need to close connections before the fork happens.
  - Libraries relying on some kind of dispatcher thread might need to resta=
rt the thread in the forked children, and clear any internal buffer (e.g. s=
tatsd 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` c=
allback.

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.h=
tml
  - Rails's Active Record: https://devcenter.heroku.com/articles/concurrenc=
y-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/fa83793b9e8d2f9a32c920f59=
b57d7f198f466b8/lib/scout_apm/environment.rb#L142-L146
  - NewRelic RPM (similarly tries to register to popular forking setups): h=
ttps://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 t=
end 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/Sh=
opify/rails/blob/411ccbdab2608c62aabdb320d52cb02d446bb39c/activerecord/lib/=
active_record/connection_adapters/abstract/connection_pool.rb#L946, it stil=
l does but a bit less: https://github.com/rails/rails/pull/41850
  - the `Typhoeus` HTTP client: https://github.com/typhoeus/typhoeus/blob/a=
345545e5e4ac0522b883fe0cf19e5e2e807b4b0/lib/typhoeus/pool.rb#L34-L42
  - Redis client: https://github.com/redis/redis-rb/blob/6542934f01b9c390ee=
450bd372209a04bc3a239b/lib/redis/client.rb#L384
  - Some older versions of NewRelic RPM: https://github.com/opendna/scorera=
nking-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 assu=
med 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 t=
he 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 decor=
ate 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/9aed3dcdfea6b64c180=
35f8e2622c474ba499846/activesupport/lib/active_support/fork_tracker.rb
  - `dd-trace-rb`: https://github.com/DataDog/dd-trace-rb/blob/793946146b47=
09289cfd459f3b68e8227a9f5fa7/lib/ddtrace/profiling/ext/forking.rb
  - To some extent, `nakayoshi_fork` decorates the `fork` method: https://g=
ithub.com/ko1/nakayoshi_fork/blob/19ef5efc51e0ae51d7f5f37a0b785309bf16e97f/=
lib/nakayoshi_fork.rb

### Proposals

I see two possible features to improve this situation:

####  Fork callbacks

One solution would be for Ruby to expose a callback API for these two event=
s, similar to `Kernel.at_exit`.

Most implementations of this functionnality in other languages ([C's `pthre=
ad_atfork`](https://man7.org/linux/man-pages/man3/pthread_atfork.3.html), [=
Python's `os.register_at_fork`](https://docs.python.org/3/library/os.html#o=
s.register_at_fork)) expose 3 callbacks:

  - `prepare` or `before` executed in the parent process before the `fork(2=
)`
  - `parent` or `after_in_parent` executed in the parent process after the =
`fork(2)`
  - `child` or `after_in_child` executed in the child process after the `fo=
rk(2)`

A direct translation of such API in Ruby could look like `Process.at_fork(p=
repare: Proc, parent: Proc, child: Proc)` if inspired by `pthread_atfork`.

Or alternatively each callback could be exposed idependently: `Process.befo=
re_fork {}`, `Process.after_fork_parent {}`, `Process.after_fork_child {}`.

Also note that similar APIs don't expose any way to unregister callbacks, a=
nd expect users to use weak references or to not hold onto objects that sho=
uld be garbage collected.

Pseudo code:

```ruby
module Process
  @prepare =3D []
  @parent =3D []
  @child =3D []

  def self.at_fork(prepare: nil, parent: nil, child: nil)
    @prepare.unshift(prepare) if prepare # prepare callbacks are executed i=
n reverse registration order
    @parent << parent if parent
    @child << child if child
  end

  def self.fork
    @prepare.each(&:call)
    if pid =3D Primitive.fork
      @parent.each(&:call) # We could consider passing the pid here.
    else
      @child.each(&:call)
    end
  end
end

```

#### Make `Kernel.fork` a delegator

A simpler change would be to just make `Kernel.fork` a delegator to `Proces=
s.fork`. This would make it much easier to prepend a module on `Process` fo=
r each library to implement its own callback.

Proposed patch: https://github.com/ruby/ruby/pull/4361



-- =

https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=3Dunsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>