From 8b65858a864aa0ecfa24ccf8f910c36af0ec1ad6 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 6 Jul 2010 04:01:30 +0000 Subject: cleanup error handling for aborted downloads We shouldn't ever spew errors to the stderr/logger on client disconnects (ECONNRESET/EPIPE/etc...). --- lib/rainbows/fiber/body.rb | 3 -- lib/rainbows/rev/client.rb | 15 +++---- lib/rainbows/rev/sendfile.rb | 2 +- lib/rainbows/writer_thread_pool.rb | 2 +- lib/rainbows/writer_thread_spawn.rb | 2 +- t/t0020-large-sendfile-response.sh | 83 ++++++++++++++++++++++++++++++++++--- 6 files changed, 88 insertions(+), 19 deletions(-) diff --git a/lib/rainbows/fiber/body.rb b/lib/rainbows/fiber/body.rb index cd6c55c..b77e310 100644 --- a/lib/rainbows/fiber/body.rb +++ b/lib/rainbows/fiber/body.rb @@ -19,9 +19,6 @@ module Rainbows::Fiber::Body # :nodoc: client.wait_writable rescue EOFError break - rescue => e - Rainbows::Error.app(e) - break end while true end else diff --git a/lib/rainbows/rev/client.rb b/lib/rainbows/rev/client.rb index e0572bb..f9284e8 100644 --- a/lib/rainbows/rev/client.rb +++ b/lib/rainbows/rev/client.rb @@ -124,13 +124,11 @@ module Rainbows return if DeferredResponse === body begin - begin - rev_sendfile(body) - rescue EOFError # expected at file EOF - @deferred_bodies.shift - body.close - close if :close == @state && @deferred_bodies.empty? - end + rev_sendfile(body) + rescue EOFError # expected at file EOF + @deferred_bodies.shift + body.close + close if :close == @state && @deferred_bodies.empty? rescue => e handle_error(e) end @@ -140,6 +138,9 @@ module Rainbows end def on_close + while f = @deferred_bodies.shift + DeferredResponse === f or f.close + end CONN.delete(self) end diff --git a/lib/rainbows/rev/sendfile.rb b/lib/rainbows/rev/sendfile.rb index 03ce41c..11cd114 100644 --- a/lib/rainbows/rev/sendfile.rb +++ b/lib/rainbows/rev/sendfile.rb @@ -14,8 +14,8 @@ module Rainbows::Rev::Sendfile def rev_sendfile(body) body.offset += @_io.sendfile_nonblock(body, body.offset, 0x10000) + enable_write_watcher rescue Errno::EAGAIN - ensure enable_write_watcher end else diff --git a/lib/rainbows/writer_thread_pool.rb b/lib/rainbows/writer_thread_pool.rb index b6c53e8..a2ef1ba 100644 --- a/lib/rainbows/writer_thread_pool.rb +++ b/lib/rainbows/writer_thread_pool.rb @@ -78,7 +78,7 @@ module Rainbows io.write(arg1) end rescue => err - Error.app(err) + Error.write(io, err) end end end diff --git a/lib/rainbows/writer_thread_spawn.rb b/lib/rainbows/writer_thread_spawn.rb index e1f9e53..9e793fc 100644 --- a/lib/rainbows/writer_thread_spawn.rb +++ b/lib/rainbows/writer_thread_spawn.rb @@ -61,7 +61,7 @@ module Rainbows io.write(arg1) end rescue => e - Error.app(e) + Error.write(io, e) end end CUR.delete(Thread.current) diff --git a/t/t0020-large-sendfile-response.sh b/t/t0020-large-sendfile-response.sh index 8e71db1..df9ab23 100755 --- a/t/t0020-large-sendfile-response.sh +++ b/t/t0020-large-sendfile-response.sh @@ -9,11 +9,11 @@ ruby) ;; ;; esac -t_plan 7 "large sendfile response for $model" +t_plan 12 "large sendfile response for $model" t_begin "setup and startup" && { - rtmpfiles curl_out a b c - rainbows_setup $model 2 + rtmpfiles curl_out a b c slow_a slow_b + rainbows_setup $model echo 'require "sendfile"' >> $unicorn_config echo 'def (::IO).copy_stream(*x); abort "NO"; end' >> $unicorn_config @@ -24,9 +24,66 @@ t_begin "setup and startup" && { t_begin "read random blob sha1" && { random_blob_sha1=$(rsha1 < random_blob) + three_sha1=$(cat random_blob random_blob random_blob | rsha1) } -t_begin "send a series of HTTP/1.1 requests in parallel" && { +t_begin "send keepalive HTTP/1.1 requests in parallel" && { + for i in $a $b $c $slow_a $slow_b + do + curl -sSf http://$listen/random_blob \ + http://$listen/random_blob \ + http://$listen/random_blob | rsha1 > $i & + done + wait + for i in $a $b $c $slow_a $slow_b + do + test x$(cat $i) = x$three_sha1 + done +} + +t_begin "send a batch of abortive HTTP/1.1 requests in parallel" && { + for i in $a $b $c $slow_a $slow_b + do + rm -f $i + ( + curl -sSf --max-time 5 --limit-rate 1K \ + http://$listen/random_blob >/dev/null || echo ok > $i + ) & + done + wait +} + +t_begin "all requests timed out" && { + for i in $a $b $c $slow_a $slow_b + do + test x$(cat $i) = xok + done +} + +s='$NF ~ /worker_connections=[0-9]+/{gsub(/[^0-9]/,"",$3); print $3; exit}' +t_begin "check proc to ensure file is closed properly (Linux only)" && { + worker_pid=$(awk "$s" < $r_err) + test -n "$worker_pid" + if test -d /proc/$worker_pid/fd + then + if ls -l /proc/$worker_pid/fd | grep random_blob + then + t_info "random_blob file is open ($model)" + fi + else + t_info "/proc/$worker_pid/fd not found" + fi +} + +t_begin "send a bunch of HTTP/1.1 requests in parallel" && { + ( + curl -sSf --limit-rate 1M http://$listen/random_blob | \ + rsha1 > $slow_a + ) & + ( + curl -sSf --limit-rate 750K http://$listen/random_blob | \ + rsha1 > $slow_b + ) & for i in $a $b $c do ( @@ -34,7 +91,7 @@ t_begin "send a series of HTTP/1.1 requests in parallel" && { ) & done wait - for i in $a $b $c + for i in $a $b $c $slow_a $slow_b do test x$(cat $i) = x$random_blob_sha1 done @@ -42,7 +99,7 @@ t_begin "send a series of HTTP/1.1 requests in parallel" && { # this was a problem during development t_begin "HTTP/1.0 test" && { - sha1=$( (curl -0 -sSfv http://$listen/random_blob && + sha1=$( (curl -0 -sSf http://$listen/random_blob && echo ok >$ok) | rsha1) test $sha1 = $random_blob_sha1 test xok = x$(cat $ok) @@ -59,6 +116,20 @@ t_begin "HTTP/0.9 test" && { test xok = x$(cat $ok) } +t_begin "check proc to ensure file is closed properly (Linux only)" && { + worker_pid=$(awk "$s" < $r_err) + test -n "$worker_pid" + if test -d /proc/$worker_pid/fd + then + if ls -l /proc/$worker_pid/fd | grep random_blob + then + t_info "random_blob file is open ($model)" + fi + else + t_info "/proc/$worker_pid/fd not found" + fi +} + t_begin "shutdown server" && { kill -QUIT $rainbows_pid } -- cgit v1.2.3-24-ge0c7