Remove use of json gem from WorkerHandle#ping! (#2473)

* Add test to change the version of nio4r gem in phased restart

* Add test to change the version of json gem in phased restart

* Remove use of json gem when handling worker stats messages

* DRY up gem change phased restart tests

* Add History entry

History.md

@@ -10,6 +10,7 @@
   * Your bugfix goes here <Most recent on the top, like GitHub> (#Github Number)
   * Ignore illegal (by Rack spec) response header ([#2439])
   * Close idle connections immediately on shutdown ([#2460])
+  * Fix some instances of phased restart errors related to the `json` gem (#2473)
 
 ## 5.0.4 / 2020-10-27
 

lib/puma/cluster/worker.rb

@@ -108,11 +108,17 @@ module Puma
           server_thread = server.run
           stat_thread ||= Thread.new(@worker_write) do |io|
             Puma.set_thread_name "stat payload"
+            base_payload = "p#{Process.pid}"
 
             while true
               begin
-                require 'json'
-                io << "p#{Process.pid}#{server.stats.to_json}\n"
+                b = server.backlog || 0
+                r = server.running || 0
+                t = server.pool_capacity || 0
+                m = server.max_threads || 0
+                rc = server.requests_count || 0
+                payload = %Q!#{base_payload}{ "backlog":#{b}, "running":#{r}, "pool_capacity":#{t}, "max_threads": #{m}, "requests_count": #{rc} }\n!
+                io << payload
               rescue IOError
                 Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
                 break

lib/puma/cluster/worker_handle.rb

@@ -42,8 +42,11 @@ module Puma
 
       def ping!(status)
         @last_checkin = Time.now
-        require 'json'
-        @last_status = JSON.parse(status, symbolize_names: true)
+        captures = status.match(/{ "backlog":(?<backlog>\d*), "running":(?<running>\d*), "pool_capacity":(?<pool_capacity>\d*), "max_threads": (?<max_threads>\d*), "requests_count": (?<requests_count>\d*) }/)
+        @last_status = captures.names.inject({}) do |hash, key|
+          hash[key.to_sym] = captures[key].to_i
+          hash
+        end
       end
 
       # @see Puma::Cluster#check_workers

test/helpers/integration.rb

@@ -53,11 +53,12 @@ class TestIntegration < Minitest::Test
       config_file.close
       config = "-C #{config_file.path}"
     end
+    puma_path = File.expand_path '../../../bin/puma', __FILE__
     if unix
-      cmd = "#{BASE} bin/puma #{config} -b unix://#{@bind_path} #{argv}"
+      cmd = "#{BASE} #{puma_path} #{config} -b unix://#{@bind_path} #{argv}"
     else
       @tcp_port = UniquePort.call
-      cmd = "#{BASE} bin/puma #{config} -b tcp://#{HOST}:#{@tcp_port} #{argv}"
+      cmd = "#{BASE} #{puma_path} #{config} -b tcp://#{HOST}:#{@tcp_port} #{argv}"
     end
     @server = IO.popen(cmd, "r")
     wait_for_server_to_boot

test/test_worker_gem_independence.rb

@@ -0,0 +1,86 @@
+require_relative "helper"
+require_relative "helpers/integration"
+
+class TestWorkerGemIndependence < TestIntegration
+  def setup
+    skip NO_FORK_MSG unless HAS_FORK
+    super
+  end
+
+  def teardown
+    return if skipped?
+    FileUtils.rm current_release_symlink, force: true
+    super
+  end
+
+  def test_changing_nio4r_version_during_phased_restart
+    change_gem_version_during_phased_restart old_app_dir: 'worker_gem_independence_test/old_nio4r',
+                                             old_version: '2.3.0',
+                                             new_app_dir: 'worker_gem_independence_test/new_nio4r',
+                                             new_version: '2.3.1'
+  end
+
+  def test_changing_json_version_during_phased_restart
+    change_gem_version_during_phased_restart old_app_dir: 'worker_gem_independence_test/old_json',
+                                             old_version: '2.3.1',
+                                             new_app_dir: 'worker_gem_independence_test/new_json',
+                                             new_version: '2.3.0'
+  end
+
+  private
+
+  def change_gem_version_during_phased_restart(old_app_dir:, new_app_dir:, old_version:, new_version:)
+    skip_unless_signal_exist? :USR1
+
+    set_release_symlink File.expand_path(old_app_dir, __dir__)
+
+    Dir.chdir(current_release_symlink) do
+      bundle_install
+      cli_server '--prune-bundler -w 1'
+    end
+
+    connection = connect
+    initial_reply = read_body(connection)
+    assert_equal old_version, initial_reply
+
+    set_release_symlink File.expand_path(new_app_dir, __dir__)
+    Dir.chdir(current_release_symlink) do
+      bundle_install
+    end
+    start_phased_restart
+
+    connection = connect
+    new_reply = read_body(connection)
+    assert_equal new_version, new_reply
+  end
+
+  def current_release_symlink
+    File.expand_path "worker_gem_independence_test/current", __dir__
+  end
+
+  def set_release_symlink(target_dir)
+    FileUtils.rm current_release_symlink, force: true
+    FileUtils.symlink target_dir, current_release_symlink, force: true
+  end
+
+  def start_phased_restart
+    Process.kill :USR1, @pid
+
+    true while @server.gets !~ /booted, phase: 1/
+  end
+
+  def with_unbundled_env
+    bundler_ver = Gem::Version.new(Bundler::VERSION)
+    if bundler_ver < Gem::Version.new('2.1.0')
+      Bundler.with_clean_env { yield }
+    else
+      Bundler.with_unbundled_env { yield }
+    end
+  end
+
+  def bundle_install
+    with_unbundled_env do
+      system("bundle install", out: File::NULL)
+    end
+  end
+end

test/worker_gem_independence_test/new_json/Gemfile

@@ -0,0 +1,4 @@
+source "https://rubygems.org"
+
+gem 'puma', path: '../../..'
+gem 'json', '= 2.3.0'

test/worker_gem_independence_test/new_json/config.ru

@@ -0,0 +1,2 @@
+require 'json'
+run lambda { |env| [200, {'Content-Type'=>'text/plain'}, [JSON::VERSION]] }

test/worker_gem_independence_test/new_json/config/puma.rb

@@ -0,0 +1 @@
+directory File.expand_path("../../current", __dir__)

test/worker_gem_independence_test/new_nio4r/Gemfile

@@ -0,0 +1,4 @@
+source "https://rubygems.org"
+
+gem 'puma', path: '../../..'
+gem 'nio4r', '= 2.3.1'

test/worker_gem_independence_test/new_nio4r/config.ru

@@ -0,0 +1 @@
+run lambda { |env| [200, {'Content-Type'=>'text/plain'}, [NIO::VERSION]] }

test/worker_gem_independence_test/new_nio4r/config/puma.rb

@@ -0,0 +1 @@
+directory File.expand_path("../../current", __dir__)

test/worker_gem_independence_test/old_json/Gemfile

@@ -0,0 +1,4 @@
+source "https://rubygems.org"
+
+gem 'puma', path: '../../..'
+gem 'json', '= 2.3.1'

test/worker_gem_independence_test/old_json/config.ru

@@ -0,0 +1,2 @@
+require 'json'
+run lambda { |env| [200, {'Content-Type'=>'text/plain'}, [JSON::VERSION]] }

test/worker_gem_independence_test/old_json/config/puma.rb

@@ -0,0 +1 @@
+directory File.expand_path("../../current", __dir__)

test/worker_gem_independence_test/old_nio4r/Gemfile

@@ -0,0 +1,4 @@
+source "https://rubygems.org"
+
+gem 'puma', path: '../../..'
+gem 'nio4r', '= 2.3.0'

test/worker_gem_independence_test/old_nio4r/config.ru

@@ -0,0 +1 @@
+run lambda { |env| [200, {'Content-Type'=>'text/plain'}, [NIO::VERSION]] }

test/worker_gem_independence_test/old_nio4r/config/puma.rb

@@ -0,0 +1 @@
+directory File.expand_path("../../current", __dir__)