[Catalyst-dev] Leaking files / file descriptors in HTTP::Body..

[Andy Grundman]

On Oct 6, 2008, at 5:49 PM, Tomas Doran wrote:

> Switching this to the dev list as it is a bit more involved.
>
> On 6 Oct 2008, at 15:30, James R. Leu wrote:
>>
> <snip>
>> ... or running out of file descriptors.  Do a 'lsof -p <pid>' where  
>> PID
>> is that of the httpd children.  I see a file descriptor leak in
>> Catalyst::Engine::finalize_body in my cat app.
>>
>> See this post for more info:
>>
>>    http://lists.scsys.co.uk/pipermail/catalyst/2007-November/015817.html
>
> and to quote that:
>
> > The contents of the
> > files is the JSON data for the API calls I'm making from the  
> client back
> > to the server.  I've traced the origins of the files back to  
> Catalyst::Engine
> > (more specifically HTTP::Body's use of File::Temp).  If I add the  
> following
> > 'hack' to the end of Catalyst::Engine::finalize_body the files in / 
> tmp
> > get closed and I no longer experience the file handle exhaustion:
> >
> >     if (defined($c->request->{_body})) {
> >         delete $c->request->{_body};
> >    }
> >
> >
> > So I have a couple of questions:
> > - is there a better way to 'fix' this?
> > - has anyone else seen this?
>
> I'm guessing from the above that you're also getting a  
> HTTP::Body::OctetStream object?
>
> I've seen something similar to this. My processes don't appear to  
> leak file descriptors, but I _do_ leak temporary files (i.e. they  
> don't get cleaned up).
>
> As this is happening for me, I thought I'd knock up some tests of  
> the unlink behavior (attached). They're not quite perfect yet, but a  
> good start at testing this specific case.
>
> This test however steadfastly refuses to fail (on my laptop, haven't  
> tested at work yet), however if I alter the File::Temp->new call in  
> HTTP::Body::OctetStream to add UNLINK => 0, it fails as I'd expect  
> (so proving at least that it's testing what I think it is testing).
>
> The only things I can think of which could be causing this are:
> * Really old File::Temp versions (is UNLINK new behavior) - I'll  
> check this out when I get into work tomorrow.
> * $File::Temp::KEEP_ALL = 1 - would it be sane to localise this  
> variable to 0 in HTTP::Body to stop users shooting themselves in the  
> foot?
>
> Anyone else got any suggestions or ideas about what may be causing  
> this?

Currently you need to manually clean up any temp files HTTP::Body  
creates that are not the result of a form upload.  I think we'll  
probably change this at some point, since it really should not be your  
problem to clean up the files, but Catalyst's.