Oh yeah, I like that even better. The app I'm currently working on is using Rails 4.1 and Rack 1.5.x. I don't have any problem with upgrading rack, I just haven't done it yet.
I think it would be reasonable to fix this for Rack 1.6+. It won't cause any problems for Rack 1.5 users, right? The environment variable gets set and then ignored, so the app would behave exactly the same way. If they want to use the new cleanup code, they can upgrade rack. -Mike -----Original Message----- From: Eric Wong [mailto:[email protected]] Sent: Wednesday, April 22, 2015 3:16 PM To: Mulvaney, Mike Cc: [email protected] Subject: Re: TeeInput leaks file handles/space "Mulvaney, Mike" <[email protected]> wrote: > That looks reasonable to me -- this way you would only have one file > still open per process at a maximum, right? I think that's a good > solution. Right. Below is a barely-tested alternative patch for Rack::TempfileReaper, for Rack 1.6+ users only. I'm not sure how prevalent 1.6+ was only released in December 2014... It's more standardized, but maybe Rack 1.6 isn't prevalent enough, yet. What do you think? (Sorry, in a rush, no commit message, yet) diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb index 637c583..7f6baa2 100644 --- a/lib/unicorn/tee_input.rb +++ b/lib/unicorn/tee_input.rb @@ -28,13 +28,20 @@ class Unicorn::TeeInput < Unicorn::StreamInput @@client_body_buffer_size end + # for Rack::TempfileReaper in rack 1.6+ def new_tmpio + tmpio = Unicorn::TmpIO.new + (@parser.env['rack.tempfiles'] ||= []) << tmpio + tmpio + 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: diff --git a/lib/unicorn/tmpio.rb b/lib/unicorn/tmpio.rb index c97979a..db88ed3 100644 --- a/lib/unicorn/tmpio.rb +++ b/lib/unicorn/tmpio.rb @@ -21,4 +21,7 @@ class Unicorn::TmpIO < File fp.sync = true fp end + + # pretend we're Tempfile for Rack::TempfileReaper alias close! close end
