From mboxrd@z Thu Jan 1 00:00:00 1970 Delivered-To: chneukirchen@gmail.com Received: by 10.140.128.1 with SMTP id a1cs404599rvd; Thu, 18 Mar 2010 02:54:18 -0700 (PDT) Received-SPF: pass (google.com: domain of 3SPihSwwICi4XYbWKVZObcYXiRLd.XOdbKMU-NOfOVQYYQVOQbYeZc.MYW@groups.bounces.google.com designates 10.141.12.7 as permitted sender) client-ip=10.141.12.7; Authentication-Results: mr.google.com; spf=pass (google.com: domain of 3SPihSwwICi4XYbWKVZObcYXiRLd.XOdbKMU-NOfOVQYYQVOQbYeZc.MYW@groups.bounces.google.com designates 10.141.12.7 as permitted sender) smtp.mail=3SPihSwwICi4XYbWKVZObcYXiRLd.XOdbKMU-NOfOVQYYQVOQbYeZc.MYW@groups.bounces.google.com; dkim=pass header.i=3SPihSwwICi4XYbWKVZObcYXiRLd.XOdbKMU-NOfOVQYYQVOQbYeZc.MYW@groups.bounces.google.com Received: from mr.google.com ([10.141.12.7]) by 10.141.12.7 with SMTP id p7mr1521948rvi.24.1268906057653 (num_hops = 1); Thu, 18 Mar 2010 02:54:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=beta; h=domainkey-signature:received:x-beenthere:received:received:received :received:received-spf:received:date:from:to:subject:message-id :references:mime-version:in-reply-to:user-agent :x-original-authentication-results:x-original-sender:reply-to :precedence:mailing-list:list-id:list-post:list-help:list-archive :x-thread-url:x-message-url:sender:list-subscribe:list-unsubscribe :content-type:content-disposition:content-transfer-encoding; bh=cYDIMMeB+4C+pGFt1y37ZttNgn20uk7Ff6Hd29yZN2A=; b=YhhKLR34eOIEYGso0MsKF6HlBe323roG03Swn81NPSj6WGi8wGCp6A1QmYKOWH8joO o0JBsPEeuz4gY2Q+7InNrIX//HLA/v4lNqkNJWqN+w5KpwJr59giNWb24RiVYQAR6PYL iaZ8N9ly1uFb2KZnfX9tDysViKZiA4hhti1BA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlegroups.com; s=beta; h=x-beenthere:received-spf:date:from:to:subject:message-id:references :mime-version:in-reply-to:user-agent :x-original-authentication-results:x-original-sender:reply-to :precedence:mailing-list:list-id:list-post:list-help:list-archive :x-thread-url:x-message-url:sender:list-subscribe:list-unsubscribe :content-type:content-disposition:content-transfer-encoding; b=j7GqdgEjqlmNNo/W8kyoZR2IPurGDu9AtSWMQW5560OENjjD9deU5gPbhABQIpZG8o kEubz8dpCpkT2wWLdRcFsQNApC1DXFYjW8rZvFr3VXKqnpEPRqSPP5U/Uka/Vh4yu47N xofbEGcq21x7xnk7lpVXtVCFS9Qdt8e9qdvqk= Received: by 10.141.12.7 with SMTP id p7mr194606rvi.24.1268906056403; Thu, 18 Mar 2010 02:54:16 -0700 (PDT) X-BeenThere: rack-devel@googlegroups.com Received: by 10.141.124.10 with SMTP id b10ls740577rvn.3.p; Thu, 18 Mar 2010 02:54:13 -0700 (PDT) Received: by 10.141.124.15 with SMTP id b15mr2379339rvn.18.1268906053396; Thu, 18 Mar 2010 02:54:13 -0700 (PDT) Received: by 10.141.124.15 with SMTP id b15mr2379338rvn.18.1268906053371; Thu, 18 Mar 2010 02:54:13 -0700 (PDT) Return-Path: Received: from dcvr.yhbt.net (dcvr.yhbt.net [64.71.152.64]) by gmr-mx.google.com with ESMTP id 25si2233339pxi.4.2010.03.18.02.54.13; Thu, 18 Mar 2010 02:54:13 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of normalperson@yhbt.net designates 64.71.152.64 as permitted sender) client-ip=64.71.152.64; Received: from localhost (unknown [127.0.2.5]) by dcvr.yhbt.net (Postfix) with ESMTP id 63A8F1F4EF; Thu, 18 Mar 2010 09:54:09 +0000 (UTC) Date: Thu, 18 Mar 2010 02:54:09 -0700 From: Eric Wong To: rack-devel@googlegroups.com Subject: Re: Not cleaning up tempfiles for multipart? Message-ID: <20100318095409.GB15049@dcvr.yhbt.net> References: <20100306075548.GB6474@dcvr.yhbt.net> <44f3f951-889e-45ec-ae46-40a371329a9e@e1g2000yqh.googlegroups.com> <20100308002217.GB18365@dcvr.yhbt.net> <20100308011204.GA19119@dcvr.yhbt.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) X-Original-Authentication-Results: gmr-mx.google.com; spf=pass (google.com: best guess record for domain of normalperson@yhbt.net designates 64.71.152.64 as permitted sender) smtp.mail=normalperson@yhbt.net X-Original-Sender: normalperson@yhbt.net Reply-To: rack-devel@googlegroups.com Precedence: list Mailing-list: list rack-devel@googlegroups.com; contact rack-devel+owners@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: X-Thread-Url: http://groups.google.com/group/rack-devel/t/6eb2bc7a1f8c072c X-Message-Url: http://groups.google.com/group/rack-devel/msg/4b29da59532fccdd Sender: rack-devel@googlegroups.com List-Subscribe: , List-Unsubscribe: , Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit Charles Oliver Nutter wrote: > On Sun, Mar 7, 2010 at 7:12 PM, Eric Wong 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). I would expect major frameworks to stick this into the default middleware stack as a convenience, but some users want a more bare-bones Rack stack can still opt out. I see it as needless overhead as the vast majority of HTTP requests do not create tempfiles. I don't make decisions for Rack, though. > >      # 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? Yes, a Rack-compliant server should call body.close if it responds to body.close at the end of the response cycle for every individual request. I put this in the wrapped body because body.each {} could be relying on the tempfiles to generate the response. body.close is the absolute last action for any Rack application. > 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? Yes, it might be safer to do this before app.call above:   env["rack.tempfiles"] ||= [] That way if env gets replaced down the stack, e.g. via: app.call(env.merge("foo.hello" => "world")) # env.merge! would be correct above any use of env["rack.tempfiles"] will still point to the same array. Well, almost... Then again some bad middleware could do:   env["rack.tempfiles"] += [ tmp_a, tmp_b ] Instead of what they _should_ do:   env["rack.tempfiles"].concat([ tmp_a, tmp_b ]) So yes, discourage future middleware authors from replacing the rack.tempfiles array. -- Eric Wong