Skip Menu |

This queue is for tickets about the Catalyst-Runtime CPAN distribution.

Report information
The Basics
Id: 41442
Status: resolved
Priority: 0/
Queue: Catalyst-Runtime

People
Owner: bobtfish [...] bobtfish.net
Requestors: ANDK [...] cpan.org
Cc: bobtfish [...] bobtfish.net
AdminCc:

Bug Information
Severity: Normal
Broken in: 5.8000_03
Fixed in: (no value)



Subject: Cleaning up temporary directory
While cleaning up my /tmp directory I found one file left by Catalyst-Runtime. I tracked it down to t/aggregate/live_engine_request_uploads.t: -rw------- 1 sand sand 5701 2008-12-05 06:14 2lYaH0dA7n Regards,
On Fri Dec 05 00:17:41 2008, ANDK wrote: Show quoted text
> While cleaning up my /tmp directory I found one file left by > Catalyst-Runtime. I tracked it down to > t/aggregate/live_engine_request_uploads.t: > > -rw------- 1 sand sand 5701 2008-12-05 06:14 2lYaH0dA7n > > Regards,
Can you try the latest 5.8 svn (>=r8734)? I added a few more tests to make sure all temporary files are cleaned up. If you still see the problem, can you add this patch and run just t/aggregate/live_engine_request_uploads.t? It will show all the files the test creates and you can figure out which test the left-over file is coming from. --- lib/Catalyst/Engine.pm (revision 47053) +++ lib/Catalyst/Engine.pm (local) @@ -294,6 +294,7 @@ my $request = $c->request; foreach my $key (keys %{ $request->uploads }) { my $upload = $request->uploads->{$key}; + warn "unlinking " . Data::Dump::dump($upload) . "\n"; unlink grep { -e $_ } map { $_->tempname } (ref $upload eq 'ARRAY' ? @{$upload} : ($upload)); }
CC: ANDK [...] cpan.org
Subject: Re: [rt.cpan.org #41442] Cleaning up temporary directory
Date: Fri, 05 Dec 2008 22:52:33 +0100
To: bug-Catalyst-Runtime [...] rt.cpan.org
From: andreas.koenig.7os6VVqR [...] franz.ak.mind.de (Andreas J. Koenig)
Show quoted text
>>>>> On Fri, 05 Dec 2008 02:30:44 -0500, "AGRUNDMA via RT" <bug-Catalyst-Runtime@rt.cpan.org> said:
Show quoted text
> Can you try the latest 5.8 svn (>=r8734)? I added a few more tests to > make sure all temporary files are cleaned up.
I have checked out 5758 and reproduced the problem. Show quoted text
> If you still see the problem, can you add this patch and run just > t/aggregate/live_engine_request_uploads.t?
Yes, I've added the patch. The remaining file has 5701 bytes, so it i clear which one it is. Your patch seems to indicate that it gets unlinked although then there are "numeric" warnings that may or may not have to do with the failure to remove this beast. I paste the last part of the diagnostice: unlinking bless({ filename => "catalyst_130pix.gif", headers => bless({ "content-disposition" => "form-data; name=\"file1\"; filename=\"catalyst_130pix.gif\"", "content-type" => "image/gif", }, "HTTP::Headers"), size => 5701, tempname => "/tmp/5D3yMZc0EG", type => "image/gif", }, "Catalyst::Request::Upload") unlinking bless({ filename => "catalyst_130pix.gif", headers => bless({ "content-disposition" => "form-data; name=\"testfile\"; filename=\"catalyst_130pix.gif\"", "content-type" => "image/gif", }, "HTTP::Headers"), size => 5701, tempname => "/tmp/gmUe8sAzIl", type => "image/gif", }, "Catalyst::Request::Upload") Argument "9, 9" isn't numeric in subtraction (-) at /home/src/perl/catalyst/SVN/blib/lib/Catalyst/Engine.pm line 566. Argument "9, 9" isn't numeric in numeric gt (>) at /home/src/perl/repoperls/installed-perls/perl/ptLkj8l/perl-5.10.0@35013/lib/site_perl/5.11.0/HTTP/Body.pm line 186. Argument "9, 9" isn't numeric in numeric eq (==) at /home/src/perl/repoperls/installed-perls/perl/ptLkj8l/perl-5.10.0@35013/lib/site_perl/5.11.0/HTTP/Body/OctetStream.pm line 40. Argument "9, 9" isn't numeric in subtraction (-) at /home/src/perl/catalyst/SVN/blib/lib/Catalyst/Engine.pm line 566. Argument "9, 9" isn't numeric in subtraction (-) at /home/src/perl/catalyst/SVN/blib/lib/Catalyst/Engine.pm line 327. unlinking bless({ filename => "live_engine_request_uploads.t", headers => bless({ "content-disposition" => "form-data; name=\"testfile\"; filename=\"live_engine_request_uploads.t\"", "content-type" => "application/x-troff", }, "HTTP::Headers"), size => 12203, tempname => "/home/src/perl/catalyst/SVN/t/aggregate/MWCNnp93Co", type => "application/x-troff", }, "Catalyst::Request::Upload") t/aggregate/live_engine_request_uploads....ok All tests successful. Files=1, Tests=101, 3 wallclock secs ( 0.07 usr 0.01 sys + 2.42 cusr 0.17 csys = 2.67 CPU) Result: PASS What gives? -- andreas
Hmm, you can ignore those warnings about "9, 9", that's due to an LWP bug. Can you run the test manually as perl -Ilib t/aggregate/live_engine_request_uploads.t? Each of the files has a test to make sure it actually gets deleted. So I'm really surprised it passes all the tests if one of the files is being left behind.
The $request created in line 8752 of current live_engine_request_uploads.t is it.
On Sat Dec 06 02:03:10 2008, ANDK wrote: Show quoted text
> The $request created in line 8752 of current > live_engine_request_uploads.t is it.
mini Catalyst-Runtime-5.7015 $ wc -l t/live_engine_request_uploads.t 244 t/live_engine_request_uploads.t Erm, I think you have magnitude fail :/ I'd like to see this fixed, can you find the relevant test again (t/aggregate/live_engine_request_uploads.t in the current dist) and work out which request it was? TIA t0m
Tom, if an exception happens in the handle_request() eval then finalize is not called. It's in finalize where temp files are cleaned up. If temp files should be cleaned up regardless of exceptions in the engine then this is a bug with an easy solution. HTTP::Body in the most recent version added an option $body->cleanup(1) to remove the temp files on DESTROY. So Catalyst::Engine should probably set that after creating the HTTP::Body object (and then remove the finalize_uploads method). Frankly, I think HTTP::Body's approach is wrong. I think it should leave File::Temp with the default unlink => 1 option and leave the File::Temp object in the upload hash -- either by leaving "fh" in the hash or simply using the File::Temp object as the file name. That way the temp files will be unlinked as soon as the Request object ($req->_body) goes out of scope.
I have (finally) fixed this with r13156, which does much as Bill suggested in this RT. This will be in the next release and I'm closing this ticket. Thanks very much for the bug report.