yahns Ruby server user/dev discussion
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: yahns-public@yhbt.net
Cc: e@80x24.org, Eric Wong <e+fbsd@80x24.org>
Subject: [PATCH 2/2] wbuf: avoid corrupted large responses with zero-copy sendfile
Date: Wed, 16 Jul 2014 19:57:20 +0000	[thread overview]
Message-ID: <1405540640-10669-3-git-send-email-e@80x24.org> (raw)
In-Reply-To: <1405540640-10669-1-git-send-email-e@80x24.org>

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


  parent reply	other threads:[~2014-07-16 19:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2015-03-07 21:36   ` [PATCH 2/2] wbuf: avoid corrupted large responses with zero-copy sendfile Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://yhbt.net/yahns/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405540640-10669-3-git-send-email-e@80x24.org \
    --to=e@80x24.org \
    --cc=e+fbsd@80x24.org \
    --cc=yahns-public@yhbt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhbt.net/yahns.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).