unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: "Mulvaney, Mike" <MMulvaney@bna.com>
Cc: "unicorn-public@bogomips.org" <unicorn-public@bogomips.org>
Subject: Re: TeeInput leaks file handles/space
Date: Wed, 22 Apr 2015 18:38:08 +0000	[thread overview]
Message-ID: <20150422183808.GA5277@dcvr.yhbt.net> (raw)
In-Reply-To: <CY1PR0301MB078011EB5A22B733EB222A45A4EE0@CY1PR0301MB0780.namprd03.prod.outlook.com>

How about this patch to tee_input?  It won't pollute the common code
path, at least, and unicorn won't support multiple clients/tee_inputs

I might release 4.8.4 with this, but it does break behavior for
hijackers, but 5.0 will have lots of tiny internal incompatibilities
for corner-cases anyways.
-----------------------------8<---------------------------
Subject: [PATCH] tee_input: close temporary file upon allocating the next

This prevents unicorn from using up too much space due to large
uploads while not polluting the common code path of most clients.

This is only fine for unicorn since unicorn itself will never
support multiple clients in the same process.

This may break users of rack.hijack (are there any on unicorn?), but
they can call IO#dup on the file if they're relying on hijack and
server internals like that.

In a perfect world, we could even truncate and rewind to avoid the
FD/inode deallocation/allocations in the kernel; but that would
break any IO#dup workarounds used by hijackers.

Notes: StringIO#close does not release memory immediately, so
there's no point in calling it here

Ref: <CY1PR0301MB078011EB5A22B733EB222A45A4EE0@CY1PR0301MB0780.namprd03.prod.outlook.com>
Reported-by: Mike Mulvaney <MMulvaney@bna.com>
---
 lib/unicorn/tee_input.rb | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb
index 637c583..6ff8210 100644
--- a/lib/unicorn/tee_input.rb
+++ b/lib/unicorn/tee_input.rb
@@ -9,12 +9,17 @@
 # strict interpretation of Rack::Lint::InputWrapper functionality and
 # will not support any deviations from it.
 #
+# This is an internal class meant for Unicorn only, any use beyond
+# what appears in the Rack specificiation is unsupported and subject
+# to change.
+#
 # When processing uploads, Unicorn exposes a TeeInput object under
 # "rack.input" of the Rack environment.
 class Unicorn::TeeInput < Unicorn::StreamInput
   # The maximum size (in +bytes+) to buffer in memory before
   # resorting to a temporary file.  Default is 112 kilobytes.
   @@client_body_buffer_size = Unicorn::Const::MAX_BODY
+  @@cur_tmpio = nil
 
   # sets the maximum size of request bodies to buffer in memory,
   # amounts larger than this are buffered to the filesystem
@@ -28,13 +33,21 @@ class Unicorn::TeeInput < Unicorn::StreamInput
     @@client_body_buffer_size
   end
 
+  # This class is essentially a singleton since unicorn will never support
+  # multiple clients; and we don't want to pollute the common code path
+  # with extra ivars/state
+  def new_tmpio
+    @@cur_tmpio.close if @@cur_tmpio
+    @@cur_tmpio = Unicorn::TmpIO.new
+  end
+
   # Initializes a new TeeInput object.  You normally do not have to call
   # this unless you are writing an HTTP server.
   def initialize(socket, request)
     @len = request.content_length
     super
     @tmp = @len && @len <= @@client_body_buffer_size ?
-           StringIO.new("") : Unicorn::TmpIO.new
+           StringIO.new("") : new_tmpio
   end
 
   # :call-seq:
-- 
EW

  reply	other threads:[~2015-04-22 18:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-22 16:42 TeeInput leaks file handles/space Mulvaney, Mike
2015-04-22 18:38 ` Eric Wong [this message]
2015-04-22 19:10   ` Mulvaney, Mike
2015-04-22 19:16     ` Eric Wong
2015-04-22 19:24       ` Mulvaney, Mike
2015-04-24  3:08         ` Eric Wong
2015-04-24 11:56           ` Mulvaney, Mike

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/unicorn/

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

  git send-email \
    --in-reply-to=20150422183808.GA5277@dcvr.yhbt.net \
    --to=e@80x24.org \
    --cc=MMulvaney@bna.com \
    --cc=unicorn-public@bogomips.org \
    /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/unicorn.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).