Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image::encode() leaks memory #426

Open
itsgoingd opened this issue Sep 17, 2015 · 9 comments
Open

Image::encode() leaks memory #426

itsgoingd opened this issue Sep 17, 2015 · 9 comments

Comments

@itsgoingd
Copy link

Hey, I'm working on a batch script that processes lots of images in a loop, always crashing due to memory exhaustion caused by intervention/image. Here is a very simplified script that reproduces this problem:

use Intervention\Image\ImageManager;

$manager = new ImageManager(array('driver' => 'gd'));

for ($i = 0; $i < 1000; $i++) {
    $image = $manager->make(__DIR__ . '/image.png');
    $image->encode('jpg');
    echo "$i. " . memory_get_peak_usage(true) . "\n";
}

I've traced this problem to this assignment https://github.com/Intervention/image/blob/master/src/Intervention/Image/AbstractEncoder.php#L87, which I believe creates a circular reference between the Image and Encoder instances, causing the memory to never be freed.

Simply unsetting the $this->image reference after the image is encoded fixes this problem.

public function process(Image $image, $format = null, $quality = null)
{
        ...

        $this->setImage(null);

        return $image->setEncoded($this->result);
}
@olivervogel
Copy link
Member

I would use destroy() to free memory in each iteration. Unsetting the image after encoding, might not be always wanted.

@itsgoingd
Copy link
Author

That doesn't fix the problem, it still leaks, just a lot less. Better workaround is to call gc_collect_cycles() in each iteration, it's still just a workaround though, end user shouldn't be expected to do this.

@olivervogel
Copy link
Member

So this seems to be a problem of the GD function. Or is it the code from Intervention Image leaking memory?

@itsgoingd
Copy link
Author

This is an issue with Intervention Image itself (it can be reproduced with both gd and imagick), I believe what is happening is:

  • on Image::make call an Image and Driver instances are created and reference to a Driver instance is stored as Image::$driver
  • new Encoder instance is created in the Driver constructor and stored as Driver::$encoder
  • on Image::encode call the Image instance is passed to Encoder::process, where it is stored as Encoder::$image (I guess this is done for convenience, so the Image instance doesn't have to be passed as argument all over the place)
  • now when I'm done with saving the encoded image and I unset the Image instance in my client-code (and I expect the memory to be freed at that point) a reference to the Image instance still exists in Encoder::$image, which itself isn't freed because it's referenced in Driver::$encoder which is referenced in Image::$driver - basically creating circular reference between the Image and Encoder instances

That's why unsetting the Image reference in Encoder::process after it's no longer needed fixes the problem. Unsetting Image reference in my client-code in that case frees the memory as no other references for that object exist at that point.

I hope that makes at least some sense, I'm pretty awful at explaining stuff. :)

@abhimanyu003
Copy link

+1

@kleisauke
Copy link

I can confirm that $this->setImage(null); fixes also the problem for Imagick.

Test case using this picture (on the filesystem):

require 'vendor/autoload.php';

use Intervention\Image\ImageManager;

$manager = new ImageManager(array('driver' => 'imagick'));

for ($i = 0; $i < 10; $i++) {
    $image = $manager->make(__DIR__ . '/lichtenstein.jpg');
    $image->encode('png');
    $image->destroy();
    echo "$i. " . memory_get_peak_usage(true) . "\n";
}

Output without $this->setImage(null);:
0. 6553600

  1. 11796480
  2. 17039360
  3. 22282240
  4. 27525120
  5. 32768000
  6. 38010880
  7. 43253760
  8. 48496640
  9. 53739520

Output with $this->setImage(null);:
0. 6553600

  1. 7077888
  2. 7077888
  3. 7077888
  4. 7077888
  5. 7077888
  6. 7077888
  7. 7077888
  8. 7077888
  9. 7077888

But it still takes 20.45 seconds to process the whole script using the Imagick driver and 16.75 seconds using the GD driver.

Server details:
Intel Xeon E3-1225 V2 @ 3.20GHz - 16GB Ram
ImageMagick 6.9.2-10 Q16 x86_64 2015-12-26
GD 2.1.1
PHP Version 5.6.16

kleisauke added a commit to weserv/images that referenced this issue Dec 29, 2015
- Memory leak is fixed in vendor, see:
Intervention/image#426
- Loading time is still very long (previous commit did not fix it)
@itstudiocz
Copy link

Can this also be fixed on 2.2.2? It's the latest version for PHP 5.3, which we use.

@derekaug
Copy link

Ran into this when trying to batch optimize a bunch of jpegs today... $this->setImage(null); worked wonders.

@nvahalik
Copy link

nvahalik commented Jan 5, 2025

I am not entirely sure if this is what I ran into yesterday, but we had a script processing images and it caused system OOM after processing a few hundred images. We are using ImageMagick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants