From 7faa4569af125ab627e10478c82846f9969b479a Mon Sep 17 00:00:00 2001 From: Chris LaRose Date: Fri, 2 Oct 2020 14:53:19 -0700 Subject: [PATCH 1/5] Update extra_runtime_dependencies test to test master proccess's gems The `extra_runtime_dependencies` option allows one to activate gems in the puma master process after "pruning" the master process with `prune_bundler`. This is useful for activating gems that need to be loaded in the master process, such as `puma_worker_killer`. The integration test for `extra_runtime_dependencies` tested the `$LOAD_PATH` of the worker process instead. Since workers are forked from the master, it's normally fine to do this, but we might as well test the master process's `$LOAD_PATH` directly if we can. --- test/config/prune_bundler_with_deps.rb | 5 +++++ test/rackup/hello-last-load-path.ru | 3 --- test/test_integration_cluster.rb | 9 ++++++--- 3 files changed, 11 insertions(+), 6 deletions(-) delete mode 100644 test/rackup/hello-last-load-path.ru diff --git a/test/config/prune_bundler_with_deps.rb b/test/config/prune_bundler_with_deps.rb index 57fe7cba65..5c5675bd4d 100644 --- a/test/config/prune_bundler_with_deps.rb +++ b/test/config/prune_bundler_with_deps.rb @@ -1,2 +1,7 @@ prune_bundler true extra_runtime_dependencies ["rdoc"] +before_fork do + $LOAD_PATH.each do |path| + puts "LOAD_PATH: #{path}" + end +end diff --git a/test/rackup/hello-last-load-path.ru b/test/rackup/hello-last-load-path.ru deleted file mode 100644 index aea0c6b942..0000000000 --- a/test/rackup/hello-last-load-path.ru +++ /dev/null @@ -1,3 +0,0 @@ -run lambda { |env| - [200, {}, [$LOAD_PATH[-1]]] -} diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index cc5ea3a352..b3c23108f3 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -239,10 +239,13 @@ def test_prune_bundler_with_multiple_workers end def test_load_path_includes_extra_deps - cli_server "-w #{workers} -C test/config/prune_bundler_with_deps.rb test/rackup/hello-last-load-path.ru" - last_load_path = read_body(connect) + cli_server "-w #{workers} -C test/config/prune_bundler_with_deps.rb test/rackup/hello.ru" - assert_match(%r{gems/rdoc-[\d.]+/lib$}, last_load_path) + load_path = [] + while (line = @server.gets) =~ /^LOAD_PATH/ + load_path << line.gsub(/^LOAD_PATH: /, '') + end + assert_match(%r{gems/rdoc-[\d.]+/lib$}, load_path.last) end private From 915b9501dee408a7b6ed1a627ae2b7b4c6102b22 Mon Sep 17 00:00:00 2001 From: Chris LaRose Date: Tue, 13 Oct 2020 21:22:22 -0700 Subject: [PATCH 2/5] Add test to refute that nio4r is loaded into puma master process --- test/test_integration_cluster.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index b3c23108f3..ab4374461f 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -248,6 +248,19 @@ def test_load_path_includes_extra_deps assert_match(%r{gems/rdoc-[\d.]+/lib$}, load_path.last) end + def test_load_path_does_not_include_nio4r + cli_server "-w #{workers} -C test/config/prune_bundler_with_deps.rb test/rackup/hello.ru" + + load_path = [] + while (line = @server.gets) =~ /^LOAD_PATH/ + load_path << line.gsub(/^LOAD_PATH: /, '') + end + + load_path.each do |path| + refute_match(%r{gems/nio4r-[\d.]+/lib}, path) + end + end + private def worker_timeout(timeout, iterations, config) From 76e0f1194415384473cf0742b1336a6dbf3a1191 Mon Sep 17 00:00:00 2001 From: Chris LaRose Date: Tue, 13 Oct 2020 21:32:45 -0700 Subject: [PATCH 3/5] Remove nio4r from puma master $LOAD_PATH --- lib/puma/launcher.rb | 6 +----- lib/puma/reactor.rb | 2 +- test/test_launcher.rb | 6 ++---- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 06ae028412..c75f2d9f5d 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -268,11 +268,7 @@ def restart! def dependencies_and_files_to_require_after_prune puma = spec_for_gem("puma") - deps = puma.runtime_dependencies.map do |d| - "#{d.name}:#{spec_for_gem(d.name).version}" - end - - [deps, require_paths_for_gem(puma) + extra_runtime_deps_directories] + [[], require_paths_for_gem(puma) + extra_runtime_deps_directories] end # @!attribute [r] extra_runtime_deps_directories diff --git a/lib/puma/reactor.rb b/lib/puma/reactor.rb index 520cfc0e53..89517270e5 100644 --- a/lib/puma/reactor.rb +++ b/lib/puma/reactor.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require 'nio' require 'puma/queue_close' if RUBY_VERSION < '2.3' module Puma @@ -20,6 +19,7 @@ class Reactor # The provided block will be invoked when an IO has data available to read, # its timeout elapses, or when the Reactor shuts down. def initialize(&block) + require 'nio' @selector = NIO::Selector.new @input = Queue.new @timeouts = [] diff --git a/test/test_launcher.rb b/test/test_launcher.rb index 2a69ac25a3..25c6da3a0e 100644 --- a/test/test_launcher.rb +++ b/test/test_launcher.rb @@ -12,8 +12,7 @@ def test_dependencies_and_files_to_require_after_prune_is_correctly_built_for_no deps, dirs = launcher.send(:dependencies_and_files_to_require_after_prune) - assert_equal(1, deps.length) - assert_match(%r{^nio4r:[\d.]+$}, deps.first) + assert_empty deps assert_equal(2, dirs.length) assert_match(%r{puma/lib$}, dirs[0]) # lib dir assert_match(%r{puma-#{Puma::Const::PUMA_VERSION}$}, dirs[1]) # native extension dir @@ -28,8 +27,7 @@ def test_dependencies_and_files_to_require_after_prune_is_correctly_built_with_e deps, dirs = launcher(conf).send(:dependencies_and_files_to_require_after_prune) - assert_equal(1, deps.length) - assert_match(%r{^nio4r:[\d.]+$}, deps.first) + assert_empty deps assert_equal(3, dirs.length) assert_match(%r{puma/lib$}, dirs[0]) # lib dir assert_match(%r{puma-#{Puma::Const::PUMA_VERSION}$}, dirs[1]) # native extension dir From a3b5613afadddcb8783200d4b33241d860c18ee4 Mon Sep 17 00:00:00 2001 From: Chris LaRose Date: Tue, 13 Oct 2020 23:01:20 -0700 Subject: [PATCH 4/5] Remove list of gems to activate from puma-wild --- bin/puma-wild | 12 +++--------- lib/puma/launcher.rb | 10 +++++----- test/test_launcher.rb | 10 ++++------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/bin/puma-wild b/bin/puma-wild index b3ba0152e3..5db409f795 100644 --- a/bin/puma-wild +++ b/bin/puma-wild @@ -5,24 +5,18 @@ require 'rubygems' -gems = ARGV.shift +cli_arg = ARGV.shift inc = "" -if gems == "-I" +if cli_arg == "-I" inc = ARGV.shift $LOAD_PATH.concat inc.split(":") - gems = ARGV.shift -end - -gems.split(",").each do |s| - name, ver = s.split(":",2) - gem name, ver end module Puma; end -Puma.const_set("WILD_ARGS", ["-I", inc, gems]) +Puma.const_set("WILD_ARGS", ["-I", inc]) require 'puma/cli' diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index c75f2d9f5d..c8811b975a 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -264,11 +264,11 @@ def restart! end end - # @!attribute [r] dependencies_and_files_to_require_after_prune - def dependencies_and_files_to_require_after_prune + # @!attribute [r] files_to_require_after_prune + def files_to_require_after_prune puma = spec_for_gem("puma") - [[], require_paths_for_gem(puma) + extra_runtime_deps_directories] + require_paths_for_gem(puma) + extra_runtime_deps_directories end # @!attribute [r] extra_runtime_deps_directories @@ -300,7 +300,7 @@ def prune_bundler return end - deps, dirs = dependencies_and_files_to_require_after_prune + dirs = files_to_require_after_prune log '* Pruning Bundler environment' home = ENV['GEM_HOME'] @@ -309,7 +309,7 @@ def prune_bundler ENV['GEM_HOME'] = home ENV['BUNDLE_GEMFILE'] = bundle_gemfile ENV['PUMA_BUNDLER_PRUNED'] = '1' - args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':'), deps.join(',')] + @original_argv + args = [Gem.ruby, puma_wild_location, '-I', dirs.join(':')] + @original_argv # Ruby 2.0+ defaults to true which breaks socket activation args += [{:close_others => false}] Kernel.exec(*args) diff --git a/test/test_launcher.rb b/test/test_launcher.rb index 25c6da3a0e..7ac368ab15 100644 --- a/test/test_launcher.rb +++ b/test/test_launcher.rb @@ -7,27 +7,25 @@ class TestLauncher < Minitest::Test include TmpPath - def test_dependencies_and_files_to_require_after_prune_is_correctly_built_for_no_extra_deps + def test_files_to_require_after_prune_is_correctly_built_for_no_extra_deps skip_on :no_bundler - deps, dirs = launcher.send(:dependencies_and_files_to_require_after_prune) + dirs = launcher.send(:files_to_require_after_prune) - assert_empty deps assert_equal(2, dirs.length) assert_match(%r{puma/lib$}, dirs[0]) # lib dir assert_match(%r{puma-#{Puma::Const::PUMA_VERSION}$}, dirs[1]) # native extension dir refute_match(%r{gems/rdoc-[\d.]+/lib$}, dirs[2]) end - def test_dependencies_and_files_to_require_after_prune_is_correctly_built_with_extra_deps + def test_files_to_require_after_prune_is_correctly_built_with_extra_deps skip_on :no_bundler conf = Puma::Configuration.new do |c| c.extra_runtime_dependencies ['rdoc'] end - deps, dirs = launcher(conf).send(:dependencies_and_files_to_require_after_prune) + dirs = launcher(conf).send(:files_to_require_after_prune) - assert_empty deps assert_equal(3, dirs.length) assert_match(%r{puma/lib$}, dirs[0]) # lib dir assert_match(%r{puma-#{Puma::Const::PUMA_VERSION}$}, dirs[1]) # native extension dir From fa5161ed6c18964d625cbfaa7f4cc95b244a4d46 Mon Sep 17 00:00:00 2001 From: Chris LaRose Date: Tue, 13 Oct 2020 23:21:06 -0700 Subject: [PATCH 5/5] Update History.md --- History.md | 1 + 1 file changed, 1 insertion(+) diff --git a/History.md b/History.md index c4449a81a8..3510d07bf2 100644 --- a/History.md +++ b/History.md @@ -5,6 +5,7 @@ * Bugfixes * Cleanup daemonization in rc.d script (#2409) + * Fix `Bundler::GemNotFound` errors for `nio4r` gem during phased restarts (#2427) * Refactor * Extract req/resp methods to new request.rb from server.rb (#2419)