cleanup error handling for aborted downloads

We shouldn't ever spew errors to the stderr/logger
on client disconnects (ECONNRESET/EPIPE/etc...).

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
 }