* [PATCH 1/2] test_server: avoid multiple workers for dead parent check
2014-07-16 19:57 [PATCH 0/2] FreeBSD-related fixes (affects Linux users, too) Eric Wong
@ 2014-07-16 19:57 ` Eric Wong
2014-07-16 19:57 ` [PATCH 2/2] wbuf: avoid corrupted large responses with zero-copy sendfile Eric Wong
1 sibling, 0 replies; 4+ messages in thread
From: Eric Wong @ 2014-07-16 19:57 UTC (permalink / raw)
To: yahns-public; +Cc: e, EW
From: EW <e+fbsd@80x24.org>
This test is less reliable when there are multiple workers as the
second worker may not be ready to detect a dead parent.
This is still a possible race if the master dies very quicklly
before a worker is fully setup.
---
test/test_server.rb | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/test/test_server.rb b/test/test_server.rb
index bd38ce3..69babb3 100644
--- a/test/test_server.rb
+++ b/test/test_server.rb
@@ -302,7 +302,7 @@ class TestServer < Testcase
end
def test_mp_dead_parent
- pid, host, port = new_mp_server
+ pid, host, port = new_mp_server(1)
wpid = nil
run_client(host, port) do |res|
wpid ||= res.body.to_i
@@ -334,7 +334,7 @@ class TestServer < Testcase
c.close
end
- def new_mp_server(nr = 1)
+ def new_mp_server(nr = 2)
ru = @ru = tmpfile(%w(config .ru))
@ru.puts('a = $$.to_s')
@ru.puts('run lambda { |_| [ 200, {"Content-Length"=>a.size.to_s},[a]]}')
@@ -342,7 +342,7 @@ class TestServer < Testcase
cfg = Yahns::Config.new
host, port = @srv.addr[3], @srv.addr[1]
cfg.instance_eval do
- worker_processes 2
+ worker_processes nr
GTL.synchronize { app(:rack, ru.path) { listen "#{host}:#{port}" } }
logger(Logger.new(File.open(err.path, "a")))
end
--
EW
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] wbuf: avoid corrupted large responses with zero-copy sendfile
2014-07-16 19:57 [PATCH 0/2] FreeBSD-related fixes (affects Linux users, too) Eric Wong
2014-07-16 19:57 ` [PATCH 1/2] test_server: avoid multiple workers for dead parent check Eric Wong
@ 2014-07-16 19:57 ` Eric Wong
2015-03-07 21:36 ` Eric Wong
1 sibling, 1 reply; 4+ messages in thread
From: Eric Wong @ 2014-07-16 19:57 UTC (permalink / raw)
To: yahns-public; +Cc: e, Eric Wong
From: Eric Wong <e+fbsd@80x24.org>
This bug is noticeable on a amd64 FreeBSD 9.2 VM, and possible under
Linux, too. This happens as a zero-copy sendfile implementation means
pages queued for transmission by the sendfile system call should not be
modified at any point after the sendfile syscall is made.
To prevent modification, we replace the temporary file with a new one.
This has a similar effect as truncate and can still prevent a dirty
flush in cases when a client consumes the response fast enough.
This reverts the misguided ade89b5142bedbcf07f38aa062bfdbfcb8bc48d3
commit ("wbuf: hack to avoid response corruption on FreeBSD")
Note: this bug was finally fixed because I finally noticed this flaw
in a different (non-Ruby, non-HTTP) server of mine.
---
lib/yahns/wbuf.rb | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/lib/yahns/wbuf.rb b/lib/yahns/wbuf.rb
index 21bccce..991557e 100644
--- a/lib/yahns/wbuf.rb
+++ b/lib/yahns/wbuf.rb
@@ -30,16 +30,9 @@ require_relative 'wbuf_common'
class Yahns::Wbuf # :nodoc:
include Yahns::WbufCommon
- # TODO: Figure out why this hack is needed to pass output buffering tests.
- # It could be a bug in our code, Ruby, the sendfile gem, or FreeBSD itself.
- # Tested on FreeBSD fbsd 9.2-RELEASE FreeBSD 9.2-RELEASE #0 r255898
- # We are able to use bypass mode on Linux to reduce buffering in some
- # cases. Without bypass mode, we must always finish writing the entire
- # response completely before sending more data to the client.
- bypass_ok = RUBY_PLATFORM =~ /linux/
-
def initialize(body, persist, tmpdir)
@tmpio = Yahns::TmpIO.new(tmpdir)
+ @tmpdir = tmpdir
@sf_offset = @sf_count = 0
@wbuf_persist = persist # whether or not we keep the connection alive
@body = body
@@ -71,16 +64,12 @@ class Yahns::Wbuf # :nodoc:
# we're all caught up, try to prevent dirty data from getting flushed
# to disk if we can help it.
- @tmpio.truncate(@sf_offset = 0)
- @tmpio.rewind
+ @tmpio.close
+ @sf_offset = 0
+ @tmpio = Yahns::TmpIO.new(@tmpdir)
@bypass = true
nil
- end if bypass_ok
-
- def wbuf_write(client, buf)
- @sf_count += @tmpio.write(buf)
- :wait_writable
- end unless bypass_ok
+ end
# called by last wbuf_flush
def wbuf_close(client)
--
EW
^ permalink raw reply related [flat|nested] 4+ messages in thread