Edit report at https://bugs.php.net/bug.php?id=62106&edit=1
ID: 62106
User updated by: t dot glaser at tarent dot de
Reported by: t dot glaser at tarent dot de
Summary: zip can (and will, in real-life cases) leak huge
tempfiles
-Status: Feedback
+Status: Assigned
Type: Bug
Package: Zip Related
Operating System: Unix/POSIX, maybe Windows
PHP Version: master-Git-2012-05-22 (Git)
Assigned To: ab
Block user comment: N
Private report: N
New Comment:
Hum. Unfortunately, I cannot bring a self-compiled PHP into the production
system, especially as it runs Debian lenny still. But within some time, I
should be able to test the patch on Debian wheezy, on a test instance, by
using a self-compiled locally patched source package, as theyâre on 5.4
already.
Your testcase looks sound to exhibit this behaviour on all but the fastest
machines.
I have no idea what phpt is, sorry.
Previous Comments:
------------------------------------------------------------------------
[2012-07-10 14:20:04] [email protected]
Please test the supplied patch. Ideally you could do this with your productive
dev environment. Because cleanup happens on request shutdown, looks like it's
nearly impossible to test this with phpt. Consider the following piece of code:
<?php
set_time_limit(0);
ini_set('memory_limit', '768M');
$zip_fname = dirname(__FILE__) . DIRECTORY_SEPARATOR . 'bug62106.zip';
$s = 'a';
foreach (array(1024, 1024, 200) as $b) {
$s = str_repeat($s, $b);
}
$zip = new ZipArchive();
$r = $zip->open($zip_fname, ZipArchive::CREATE);
if ($r) {
$zip->addFromString('huhu.txt', $s);
set_time_limit(3);
$zip->close();
}
register_shutdown_function will not work here because it's a part of request.
But it's clearly to see - without patch there are temp files there, and with
the
patch the cleanup works. But the best were of course if you test it directly
with your app. If you have an idea how to test this with phpt, it would be also
great :)
------------------------------------------------------------------------
[2012-07-10 14:14:00] [email protected]
The following patch has been added/updated:
Patch Name: 62106.patch
Revision: 1341929640
URL:
https://bugs.php.net/patch-display.php?bug=62106&patch=62106.patch&revision=1341929640
------------------------------------------------------------------------
[2012-05-22 07:47:45] t dot glaser at tarent dot de
Description:
------------
Iâve been investigating a disc full case on some of our servers, which was
fixed by removing eight Gibibytes (!) of *.zip.?????? files (about a dozen
files) that looked like mkstemp(3)-generated, and investigated this problem.
To cut a long story short: an instance of the ZipArchive class does
its âactionâ when the close() method is called. (There should also be an
abort() method which frees all ressources and unlocks all files, for if an
error is detected in between open/addfile and further addfile calls or the
close call, but thatâs not the problem at hand, it just irritated me.) This
ends up being [php-src.git] / ext / zip / lib / zip_close.c (Iâve just looked
at gitweb master to reconfirm this), which has the following problem:
zip_close() forcibly allocates a temporary file, operates on that and then
atomically renames the temporary file to the desired output filename. This may
be a good thing in a regular Unix environment (and looking at the authors, I
know at least one of them from NetBSD®, so the code probably does come from a
Unix environment). HOWEVER, this is a PHP environment, not a regular Unix
environment, even if it may run on Unix, and PHP, when running in a webserver,
but also as CLI, asserts certain resource limits â execution time (not in
CLI), memory size, etc.
When the script is terminated due to such a limit (in our case, probably the
execution time limit in apache2 mod_php), the tempfile is not cleaned up.
Period. And when you were zipping up 400 MiB, and the termination happens when
300 MiB of that are already written to disc⦠well, not nice.
The cleanest way would probably be for the PHP core to offer an extension (C,
but PHP would also make sense) to register cleanup hooks that run when script
execution is terminated. This would be more generically usable than just a
method with which an extension could register tempfiles which were then
removed, as cleanup may involve other tasks. (As a general suggestion, cleanup
execution time could be limited to the greater of 5 seconds and a tenth of the
script execution time limit, and memory size should be similarly extended by a
small amount so it wonât terminate abnormally during the cleanup phase.)
Test script:
---------------
createNewestReleaseFilesAsZip method (at the end of the file) in:
https://evolvis.org/scm/viewvc.php/trunk/gforge_base/evolvisforge-5.1/src/common/frs/FRSPackage.class.php?revision=18116&root=evolvis&view=markup
Note how a later revision of the same file already contains some attempts at
error handling, and how it the lack of an abort method makes that awkward.
Expected result:
----------------
No tempfile leak. If no resource limit is hit, the .zip file is created; if
one is hit, the .zip file is either untouched or removed (unlinked).
Actual result:
--------------
If a resource limit is hit, a tempfile of arbitrary size (at least several
hundred Mebibytes confirmed) is leaked.
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=62106&edit=1
