http_response: reorder wbuf_maybe on successful early flush

We can use the wbuf_close return value instead to ensure we close
tmpio properly and follow the same code path as a normal
(:wait_writable-triggering) buffered response would.

Add a few tests to ensure we properly close the response body
for exec_cgi, where I noticed zombies and started me down this
rabbit hole looking for places where the response body was not
closed properly.

1 files changed, 99 insertions, 5 deletions

diff --git a/test/test_extras_exec_cgi.rb b/test/test_extras_exec_cgi.rb
index 403925b..4e1f5b3 100644
--- a/test/test_extras_exec_cgi.rb
+++ b/test/test_extras_exec_cgi.rb
@@ -7,17 +7,15 @@ class TestExtrasExecCGI < Testcase
   include ServerHelper
   alias setup server_helper_setup
   alias teardown server_helper_teardown
+  RUNME = "#{Dir.pwd}/test/test_extras_exec_cgi.sh"
 
   def test_exec_cgi
     err, cfg, host, port = @err, Yahns::Config.new, @srv.addr[3], @srv.addr[1]
-    runme = "#{Dir.pwd}/test/test_extras_exec_cgi.sh"
-    assert File.executable?(runme), "run test in project root"
+    assert File.executable?(RUNME), "run test in project root"
     pid = mkserver(cfg) do
       require './extras/exec_cgi'
       cfg.instance_eval do
-        app(:rack, ExecCgi.new(runme)) do
-          listen "#{host}:#{port}"
-        end
+        app(:rack, ExecCgi.new(RUNME)) { listen "#{host}:#{port}" }
         stderr_path err.path
       end
     end
@@ -75,7 +73,103 @@ class TestExtrasExecCGI < Testcase
       assert_nil body
       c.close
     end
+
+    Timeout.timeout(30) do # pid of executable
+      c = get_tcp_client(host, port)
+      c.write "GET /pid HTTP/1.0\r\n\r\n"
+      head, body = c.read.split(/\r\n\r\n/, 2)
+      assert_match %r{200 OK}, head
+      assert_match %r{\A\d+\n\z}, body
+      exec_pid = body.to_i
+      c.close
+      poke_until_dead exec_pid
+    end
+  ensure
+    quit_wait(pid)
+  end
+
+  def test_cgi_died
+    err, cfg, host, port = @err, Yahns::Config.new, @srv.addr[3], @srv.addr[1]
+    pid = mkserver(cfg) do
+      require './extras/exec_cgi'
+      cfg.instance_eval do
+        app(:rack, ExecCgi.new(RUNME)) { listen "#{host}:#{port}" }
+        stderr_path err.path
+      end
+    end
+    exec_pid_tmp = tmpfile(%w(exec_cgi .pid))
+    c = get_tcp_client(host, port)
+    Timeout.timeout(20) do
+      c.write "GET /die HTTP/1.0\r\nX-PID-DEST: #{exec_pid_tmp.path}\r\n\r\n"
+      head, body = c.read.split(/\r\n\r\n/, 2)
+      assert_match %r{500 Internal Server Error}, head
+      assert_match "", body
+      exec_pid = exec_pid_tmp.read
+      assert_match %r{\A(\d+)\n\z}, exec_pid
+      poke_until_dead exec_pid.to_i
+    end
+  ensure
+    exec_pid_tmp.close! if exec_pid_tmp
+    quit_wait(pid)
+  end
+
+  [ 9, 10, 11 ].each do |rtype|
+    [ 1, 2, 3 ].each do |block_on|
+      define_method("test_block_on_block_on_#{block_on}_rtype_#{rtype}") do
+        _blocked_zombie([block_on], rtype)
+      end
+    end
+  end
+
+  def _blocked_zombie(block_on, rtype)
+    err, cfg, host, port = @err, Yahns::Config.new, @srv.addr[3], @srv.addr[1]
+    pid = mkserver(cfg) do
+      $_tw_blocked = 0
+      $_tw_block_on = block_on
+      Yahns::HttpClient.__send__(:include, TrywriteBlocked)
+      require './extras/exec_cgi'
+      cfg.instance_eval do
+        app(:rack, ExecCgi.new(RUNME)) { listen "#{host}:#{port}" }
+        stderr_path err.path
+      end
+    end
+
+    c = get_tcp_client(host, port)
+    Timeout.timeout(20) do
+      case rtype
+      when 9 # non-persistent (HTTP/0.9)
+        c.write "GET /pid\r\n\r\n"
+        body = c.read
+        assert_match %r{\A\d+\n\z}, body
+        exec_pid = body.to_i
+        poke_until_dead exec_pid
+      when 10 # non-persistent (HTTP/1.0)
+        c.write "GET /pid HTTP/1.0\r\n\r\n"
+        head, body = c.read.split(/\r\n\r\n/, 2)
+        assert_match %r{200 OK}, head
+        assert_match %r{\A\d+\n\z}, body
+        exec_pid = body.to_i
+        poke_until_dead exec_pid
+      when 11 # pid of executable, persistent
+        c.write "GET /pid HTTP/1.0\r\nConnection: keep-alive\r\n\r\n"
+        buf = ""
+        begin
+          buf << c.readpartial(666)
+        end until buf =~ /\r\n\r\n\d+\n/
+        head, body = buf.split(/\r\n\r\n/, 2)
+        assert_match %r{200 OK}, head
+        assert_match %r{\A\d+\n\z}, body
+        exec_pid = body.to_i
+        assert_raises(Errno::EAGAIN, IO::WaitReadable) { c.read_nonblock(666) }
+        poke_until_dead exec_pid
+        # still alive?
+        assert_raises(Errno::EAGAIN, IO::WaitReadable) { c.read_nonblock(666) }
+      else
+        raise "BUG in test, bad rtype"
+      end
+    end
   ensure
+    c.close if c
     quit_wait(pid)
   end
 end