Fix memory leak

[shouze]

Priorto that fix, when I was adding 100 times the same file into a zip,
I had this kind of memory consumption:

php zip.php
100/100 [============================] 100%  1 sec/1 sec  28.0 MiB

After the fix I obtain a constant memory consumption, whatever the
number of times I add the file:

php zip.php
100/100 [============================] 100%  1 sec/1 sec  4.0 MiB

[NicolasCARPi]

Hello,

Thank you for your contribution.

Please refrain from using the variable name 'data', it is not descriptive enough. Can you change that?

For File.php you can return directly at line 464, no need for intermediate variable.

[shouze]

@NicolasCARPi done, so true about data 😉

[NicolasCARPi]

Can you also remove "data" here please? foreach files as file conveys better the intent.

[shouze]

@NicolasCARPi done

[NicolasCARPi]

@shouze Could you provide the code you used to monitor the memory usage please? I want to reproduce.

[shouze]

@NicolasCARPi yes of course, here's my test script to reproduce:

zip.php

<?php
require 'vendor/autoload.php';

const ITERATIONS_COUNT = 100;
const ZIP_FILENAME = 'test.zip';

use Symfony\Component\Console\Helper\ProgressBar;
use Symfony\Component\Console\Output\ConsoleOutput;

$fd = fopen(ZIP_FILENAME, 'w');
$opt = new ZipStream\Option\Archive();
$opt->setOutputStream($fd);
$zip = new ZipStream\ZipStream(ZIP_FILENAME, $opt);
$progressBar = new ProgressBar(new ConsoleOutput(), ITERATIONS_COUNT);
$progressBar->setFormat('debug');

for ($i = 1; $i <= ITERATIONS_COUNT; $i++) {
	$filename = sprintf('extracted/img-%0'.strlen(ITERATIONS_COUNT).'d.jpg', $i);
	$zip->addFileFromStream($filename, fopen('img.jpg', 'r'));
	$progressBar->advance();
}

$progressBar->finish();

$zip->finish();
fclose($fd);

composer.json

{
    "require": {
        "maennchen/zipstream-php": "dev-master",
        "symfony/console": "^4.1"
    },
    "scripts": {
	"test": [
		"rm -rf extracted",
		"php zip.php",
		"unzip test.zip",
		"ls -lha extracted/"
	]
    }
}

img.jpg

[NicolasCARPi]

Ok thank you. Looks good to me!

I'll let @maennchen merge it if he agrees.

[maennchen]

@NicolasCARPi Feel free to do so yourself. You‘ve examined the PR enough yourself.