rack-devel archive mirror (unofficial) https://groups.google.com/group/rack-devel
 help / color / mirror / Atom feed
From: Charles Oliver Nutter <headius@headius.com>
To: rack-devel@googlegroups.com
Subject: Re: Not cleaning up tempfiles for multipart?
Date: Wed, 17 Mar 2010 09:41:11 -0600	[thread overview]
Message-ID: <f04d2211003170841o5f97169cy165e0fedb3ebe552@mail.gmail.com> (raw)
In-Reply-To: <20100308011204.GA19119@dcvr.yhbt.net>

On Sun, Mar 7, 2010 at 7:12 PM, Eric Wong <normalperson@yhbt.net> wrote:
> I guess this deals with wonky multipart uploads that browsers still
> generate these days[1].  Ugh, yeah, it's nasty...
> The comment below in _call is very important.
>
>
> 1.  Ensure all tempfiles created by Rack go into an array in env,
>    probably env["rack.tempfiles"]:
>
>    tempfile = Tempfile.new("foo")
>    (env["rack.tempfiles"] ||= []) << tempfile

I'm with you so far...

> 2.  Have a middleware wrap everything, including the response body:

I'm a bit new to the middleware thing. Does this mean everyone would
have to configure middleware on their setups to get this
tempfile-closing behavior? And your comment below...does that mean
there are situations where this wouldn't work?

Closing and removing tempfiles when they're no longer needed should be
the default behavior, not something you have to configure. It's a bug
to not close and remove them (or at least, a bug to keep creating new
ones and expecting GC to clean everything up eventually).

>    class TempfileReaper < Struct.new(:app, :env, :body)
>
>      # --------------- in config.ru -----------
>      # use TempfileReaper
>      # run MyApp.new
>      def initialize(app)
>        super(app)
>      end
>
>      # we wrap the entire response body later on, so dup it for
>      # reentrancy.  You can actually avoid the enforced dup as
>      # an optimization, probably...
>      def call(env)
>        dup._call(env)
>      end
>
>      # used when wrapping the response body
>      def each(&block)
>        body.each { |chunk| yield chunk }
>      end
>
>      # the Rack server should call this (when we're the body)
>      def close
>        tempfiles = env["rack.tempfiles"]
>        if tempfiles
>          tempfiles.each { |tmp| tmp.close! rescue nil }
>        end
>      end

By "the Rack server should call this" do you mean that if the server
doesn't call this, tempfiles Rack creates will not be cleaned up until
GC runs?

Shouldn't Rack itself be guaranteeing it doesn't leave garbage files around?

>      # wrap the normal application call, saving env
>      def _call(env)
>        self.env = env
>
>        # XXX VERY IMPORTANT:
>        # you need to ensure env stays the same throughout the request,
>        # some middlewares overwrite/replace it instead of merge!-ing into it
>        status, headers, body = app.call(env)
>        self.body = body
>        [ status, headers, self ]
>      end
>    end

Same question as above...can badly-written middleware now cause
tempfiles to linger?

- Charlie

  reply	other threads:[~2010-03-17 15:41 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-05 14:40 Not cleaning up tempfiles for multipart? Charles Oliver Nutter
2010-03-05 14:47 ` Charles Oliver Nutter
2010-03-05 14:48   ` Charles Oliver Nutter
2010-03-06 10:20   ` Hongli Lai
2010-03-07 14:25     ` Charles Oliver Nutter
2010-03-06  7:55 ` Eric Wong
2010-03-06 10:25   ` Hongli Lai
2010-03-07 14:34     ` Charles Oliver Nutter
2010-03-08  0:22       ` Eric Wong
2010-03-08  1:12         ` Eric Wong
2010-03-17 15:41           ` Charles Oliver Nutter [this message]
2010-03-18  9:54             ` Eric Wong
2014-01-06 22:45               ` Wojtek Kruszewski
2014-02-11 21:05                 ` Eric Wong
2014-03-27 21:40                   ` Lenny Marks
2010-03-07 23:53     ` Eric Wong
2010-03-08 11:26       ` Hongli Lai
2010-03-08 11:30         ` Hongli Lai
2010-03-08 14:33           ` Randy Fischer
2010-03-08 14:43           ` Charles Oliver Nutter
2010-03-08 14:49             ` James Tucker
2010-03-17  2:37           ` Eric Wong
2010-03-08 13:22         ` Charles Oliver Nutter
2010-03-08 14:42         ` James Tucker
2010-03-08 17:24           ` Hongli Lai
2010-03-09  7:43             ` Charles Oliver Nutter
2010-03-08 10:05     ` James Tucker
2010-03-07 14:27   ` Charles Oliver Nutter
2010-03-08  0:18     ` Eric Wong
2010-03-08 10:07       ` James Tucker

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-list from there: mbox

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

  List information: https://groups.google.com/group/rack-devel

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

  git send-email \
    --in-reply-to=f04d2211003170841o5f97169cy165e0fedb3ebe552@mail.gmail.com \
    --to=rack-devel@googlegroups.com \
    /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.
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).