[PATCH] tests: thread-safety fixes

[PATCH] tests: thread-safety fixes

[Eric Wong]

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(-)

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 @@ def server_helper_setup
   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 @@ def test_listen_fd3
     [ %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 @@ def bin_daemon(worker, inherit)
     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 @@ def usr2_dir(tmpdir, preload, worker)
     # 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 @@ def usr2_dir(tmpdir, preload, worker)
     }
     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 @@ def test_initialize
   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 @@ def test_multi_conf_example
   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 @@ def test_gzip_static
       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 'digest'
 begin
   require 'kcar'
+  require 'yahns/proxy_pass'
 rescue LoadError
 end
 
@@ -188,7 +189,6 @@ def test_unix_socket_no_path
     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 @@ def test_proxy_pass
     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 @@ def setup
     @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 @@ def a.each
     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
-- 
EW