Regression in 4.3.3 with require json

[ylecuyer]

Describe the bug
Hello, #1801 introduced a regression in 4.3.3 when using puma with prune_bundler and control-app if you issue a phased restart with an app with a different json gem version you end up with the following error :

/home/ylecuyer/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/bundler-2.1.4/lib/bundler/runtime.rb:312:in `check_for_activated_spec!': You have already activated json 2.3.0, but your Gemfile requires json 2.1.0. Prepending `bundle exec` to your command may solve this. (Gem::LoadError)

Puma config:

# Puma can serve each request in a thread from an internal thread pool.
# The `threads` method setting takes two numbers: a minimum and maximum.
# Any libraries that use thread pools should be configured to match
# the maximum value specified for Puma. Default is set to 5 threads for minimum
# and maximum; this matches the default thread size of Active Record.
#
max_threads_count = ENV.fetch("RAILS_MAX_THREADS") { 5 }
min_threads_count = ENV.fetch("RAILS_MIN_THREADS") { max_threads_count }
threads min_threads_count, max_threads_count

# Specifies the `port` that Puma will listen on to receive requests; default is 3000.
#
port        ENV.fetch("PORT") { 3000 }

# Specifies the `environment` that Puma will run in.
#
environment ENV.fetch("RAILS_ENV") { "development" }

# Specifies the `pidfile` that Puma will use.
pidfile ENV.fetch("PIDFILE") { "tmp/pids/server.pid" }

# Specifies the number of `workers` to boot in clustered mode.
# Workers are forked web server processes. If using threads and workers together
# the concurrency of the application would be max `threads` * `workers`.
# Workers do not work on JRuby or Windows (both of which do not support
# processes).
#
# workers ENV.fetch("WEB_CONCURRENCY") { 2 }

# Use the `preload_app!` method when specifying a `workers` number.
# This directive tells Puma to first boot the application and load code
# before forking the application. This takes advantage of Copy On Write
# process behavior so workers use less memory.
#
# preload_app!

prune_bundler

# Allow puma to be restarted by `rails restart` command.
plugin :tmp_restart

This is the default rails new template for config/puma.rb I have only added prune_bundler

To Reproduce

https://youtu.be/6VYPTLrFKIg

rails new puma-json
cd puma-json
bundle exec puma -p 6000 -w 4 --control-url 'tcp://127.0.0.1:9293' --control-token foo

Edit Gemfile and add:

gem 'json', '2.1.0'

Send phased restart signal to puma

Expected behavior

Workers should restart successfully no matter the json version used in the app.

[dentarg]

#1801 introduced a regression in 4.3.3

Actually, 4.0.0 (but it is missing from History.md)

Have you tried reproducing this with the Puma master branch? I'm thinking perhaps this is related to issue #2133 and PR #2154

[ylecuyer]

I could reproduce with the master branch too (at master@0b737cc)

[nateberkopec]

Thanks for the detailed repro. This does sound similar to the issues we had with nio4r version changes and phased restarts, it's probably the same issue, but just surfaced differently.

[MSP-Greg]

There have also been issues with Bundler/RubyGems, especially with stdlib gems that they load themselves, which json is.

One solution may be to require 'json' when and if it's needed, inside the when /\/gc-stats$/ code?

[dentarg]

This does sound similar to the issues we had with nio4r version changes and phased restarts, it's probably the same issue, but just surfaced differently.

Ah yes, that's #2018 (and more in #1875)

[nateberkopec]

Let's move the require into the method, like @MSP-Greg suggested. It doesn't actually fix the issue (which is #2018) but it will reduce the impact.

[MSP-Greg]

It doesn't actually fix the issue

Sorry, I didn't make that clear.

but it will reduce the impact

I was hoping some of those experiencing the issue might try it. We would be arriving at the party much later...

Some updates require a 'hard reset'. We have to do our best to minimize that number.

[sur]

Actually this issue is there when we use prune_bundler
If we remove that from the puma.rb config file, it works just fine.

Which also means that we can no longer rely on the phased-restart reliably.

[MSP-Greg]

Can anyone try PR #2269 to see if it fixes the issue?