From c2a0658fc26467a3950bb2848948932ae4f33f61 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 1 May 2018 19:42:22 +0000 Subject: tests: thread-safety fixes We can't require 'proxy_pass' in both a parent and forked child, so require it up front (as kcar will become a hard dependency in place of unicorn). Then, rely on GTL (global test lock) to synchronize around fork since the VM may not always be able to protect that. However, there's no need to synchronize around spawn/system/`backtick`, as the VM should always be using those in a thread-safe way (via vfork). --- test/helper.rb | 4 ++++ test/server_helper.rb | 2 +- test/test_bin.rb | 8 ++++---- test/test_config.rb | 4 ++-- test/test_extras_try_gzip_static.rb | 2 +- test/test_proxy_pass.rb | 3 +-- test/test_proxy_pass_no_buffering.rb | 2 +- test/test_server.rb | 2 +- 8 files changed, 15 insertions(+), 12 deletions(-) (limited to 'test') diff --git a/test/helper.rb b/test/helper.rb index 634f63e..3023dee 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -136,6 +136,10 @@ def require_exec(cmd) false end +def xfork + GTL.synchronize { fork { yield } } +end + class DieIfUsed @@n = 0 def each diff --git a/test/server_helper.rb b/test/server_helper.rb index 9ef15c6..91d7208 100644 --- a/test/server_helper.rb +++ b/test/server_helper.rb @@ -91,7 +91,7 @@ module ServerHelper end def mkserver(cfg, srv = @srv) - fork do + xfork do ENV["YAHNS_FD"] = srv.fileno.to_s srv.autoclose = false yield if block_given? diff --git a/test/test_bin.rb b/test/test_bin.rb index e59c9b4..6d4faf6 100644 --- a/test/test_bin.rb +++ b/test/test_bin.rb @@ -22,7 +22,7 @@ class TestBin < Testcase [ %w(-O listen=inherit), %W(-p #{port} -o #{host}) ].each do |opt| @srv.setsockopt(:SOL_SOCKET, :SO_KEEPALIVE, 0) begin - pid = fork do # emulate a systemd environment + pid = xfork do # emulate a systemd environment env = { 'LISTEN_PID' => $$.to_s, 'LISTEN_FDS' => '1' } cmd.concat(opt) exec env, *cmd, 3 => @srv, err: @err.path @@ -77,7 +77,7 @@ class TestBin < Testcase cfg.puts "end" @cmd.concat(%W(-D -c #{cfg.path})) addr = cloexec_pipe - pid = fork do + pid = xfork do opts = { close_others: true } addr[0].close if inherit @@ -159,7 +159,7 @@ class TestBin < Testcase # need to fork here since tests are MT and the FD can leak out and go to # other processes which fork (but do not exec), causing ETXTBUSY on # Process.spawn - pid = fork do + pid = xfork do File.open(exe, "w") { |y| lines = File.readlines("bin/yahns") lines[0] = "#!#{RbConfig.ruby}\n" @@ -190,7 +190,7 @@ class TestBin < Testcase } cmd = %W(#{exe} -D -c #{cfg.path}) cmd << { @srv => @srv, close_others: true } - pid = GTL.synchronize { Process.spawn(env, *cmd) } + pid = Process.spawn(env, *cmd) res = Net::HTTP.start(host, port) { |h| h.get("/") } assert_equal 200, res.code.to_i orig = res.body diff --git a/test/test_config.rb b/test/test_config.rb index 73d9456..c609d5a 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -13,7 +13,7 @@ class TestConfig < Testcase end def test_multi_conf_example - pid = fork do + pid = xfork do tmpdir = yahns_mktmpdir # modify the example config file for testing @@ -37,7 +37,7 @@ class TestConfig < Testcase end def test_rack_basic_conf_example - pid = fork do + pid = xfork do tmpdir = yahns_mktmpdir # modify the example config file for testing diff --git a/test/test_extras_try_gzip_static.rb b/test/test_extras_try_gzip_static.rb index b6d5943..5901c9d 100644 --- a/test/test_extras_try_gzip_static.rb +++ b/test/test_extras_try_gzip_static.rb @@ -52,7 +52,7 @@ class TestExtrasTryGzipStatic < Testcase File.symlink "COPYING", "#{tmpdir}/COPYING.relsymlink" gplgz = "#{tmpdir}/COPYING.gz" FileUtils.cp("COPYING", gpl) - _, status = Process.waitpid2(fork do + _, status = Process.waitpid2(xfork do File.open(gplgz, "w") do |fp| Zlib::GzipWriter.wrap(fp.dup) { |io| io.write(GPL_TEXT) } end diff --git a/test/test_proxy_pass.rb b/test/test_proxy_pass.rb index 22f1802..51d0d3f 100644 --- a/test/test_proxy_pass.rb +++ b/test/test_proxy_pass.rb @@ -6,6 +6,7 @@ require 'json' require 'digest' begin require 'kcar' + require 'yahns/proxy_pass' rescue LoadError end @@ -188,7 +189,6 @@ class TestProxyPass < Testcase pid = mkserver(cfg) do @srv.autoclose = @srv2.autoclose = false ENV["YAHNS_FD"] = "#{@srv.fileno},#{@srv2.fileno}" - require 'yahns/proxy_pass' cfg.instance_eval do app(:rack, Yahns::ProxyPass.new("unix:#{unix_path}:/$fullpath")) do listen "#{host}:#{port}" @@ -251,7 +251,6 @@ class TestProxyPass < Testcase err, cfg, host, port = @err, Yahns::Config.new, @srv.addr[3], @srv.addr[1] host2, port2 = @srv2.addr[3], @srv2.addr[1] pid = mkserver(cfg) do - require 'yahns/proxy_pass' @srv2.close cfg.instance_eval do app(:rack, Yahns::ProxyPass.new("http://#{host2}:#{port2}")) do diff --git a/test/test_proxy_pass_no_buffering.rb b/test/test_proxy_pass_no_buffering.rb index 356623f..61f2959 100644 --- a/test/test_proxy_pass_no_buffering.rb +++ b/test/test_proxy_pass_no_buffering.rb @@ -4,6 +4,7 @@ require_relative 'server_helper' begin require 'kcar' + require 'yahns/proxy_pass' rescue LoadError end require 'digest/md5' @@ -39,7 +40,6 @@ class TestProxyPassNoBuffering < Testcase @srv2 = TCPServer.new(ENV["TEST_HOST"] || "127.0.0.1", 0) server_helper_setup skip "kcar missing yahns/proxy_pass" unless defined?(Kcar) - require 'yahns/proxy_pass' @tmpdir = yahns_mktmpdir end diff --git a/test/test_server.rb b/test/test_server.rb index c6d70cb..a113b43 100644 --- a/test/test_server.rb +++ b/test/test_server.rb @@ -225,7 +225,7 @@ class TestServer < Testcase assert_equal 1, eggs.size assert_equal 1, eggs.first[1].instance_variable_get(:@worker_threads) - pid = fork do + pid = xfork do bpipe[1].close ENV["YAHNS_FD"] = unix_srv.fileno.to_s unix_srv.autoclose = false -- cgit v1.2.3-24-ge0c7