Allowed memory size of 67108864 bytes exhausted in framework/filesystem/GD.php

[CodingMonkeyXYZ]

PHP Fatal error: Allowed memory size of 67108864 bytes exhausted (tried to allocate 11328 bytes) in ...../httpdocs/framework/filesystem/GD.php on line 51,

Steps to reproduce:

1. Upload say 20 camera-sized HQ images
2. Flush the cache with ?flush=all.
3. Go to files in the Silverstripe CMS menu and list the content of the Uploads folder (where the images get saved). The system tries to generate the thumbnails all at once and dies trying.

If it were a smaller amount, I'd just say 'add more memory' but when the server ends up allocating 128Meg of a 512M limit, it's getting silly - especially when we're planning on having more than a few sites on there.

I'm guessing this might be caused by the framework not freeing up the memory for the image after creating the thumbnail, or something. It wouldn't take too many HQ images to quickly exceed the limit. This might be as simple as adding an 'imagedestroy' somewhere...

[Firesphere]

I think this is more a GD problem, resizing all HQ images takes a LOT of memory for GD. I would suggest to upload the HQ images as "normal" files, and upload resized images yourself.
Another option is to use a less RAM-heavy image handler, like Imagick.

[CodingMonkeyXYZ]

Yes but isn't the problem that the memory isn't freed after creating a thumbnail for each HQ image though? As in rather than doing a create, free, create, free, etc, it's just doing a whole bunch of creates and thereby eating up a lot of memory?

[kinglozzer]

This issue (or at least, a very similar issue) has been discussed before, not sure what the current state of it is: https://groups.google.com/forum/m/#!topic/silverstripe-dev/B57a3KYeAVQ. Original pull request was here: #2569.

Would adding a GDBackend->__destruct() with imagedestroy($this->getImageResource()) handle this? Or are images intentionally kept in memory in case we need to resampled the same resource more than once?

[CodingMonkeyXYZ]

Ah Thanks, I did search here but not the old site. I haven't read through all that thread on the old system in detail yet but there's a lot of talk there discussing things like limiting the upload size to solve the problem. This IMHO is the wrong approach - that should (ideally) be a configurable option for the system administrator to enable them to cap file sizes for disk quota reasons - not to work around an apparent bug in the code. In this day an age, there's really no reason not to let customers upload images directly from a digital camera (hosting quotas allowing). Actually, the images I used were more like 'screen size' rather than digital camera size (so much smaller).

I think if we can, freeing up the memory would be a solution. I can't see why we'd need to keep them in memory - at least not for the job of creating and viewing thumbnails for a directory. I'm just not sure where to alter that at the moment. Maybe I'll take a look on Monday.

[kinglozzer]

The main outcome of that thread was how to prevent crashes when resizing from occurring more than once - if PHP hit its memory limits, don't try resizing that image again (preventing unrecoverable errors).

The discussion about actually preventing these issues was centered around calculating if there's enough memory available to resampled the image without crashing.

Would be interesting to see some sort of benchmark for image resampling with vs. without imagedestroy(), might try this if I get time this weekend. @chillu do you have any feedback on this? Any potential issues with using imagedestroy() in a destructor?

[kinglozzer]

Well, I just set up a quick, very basic test for this and it didn't seem to make any noticeable difference.

public function Foo() {
    $before = memory_get_usage();

    foreach ($this->Images() as $img) {
        $resampled = $img->SetWidth(1920)->getURL();
        $resampled2 = $img->CroppedImage(1920, 1080)->getURL();
        $resampled3 = $img->SetHeight(1080)->getURL();
        $resampled4 = $img->SetHeight(1214)->getURL();
        $resampled5 = $img->SetHeight(1251)->getURL();
    }

    $result = memory_get_usage() - $before;

    foreach ($this->Images() as $img) {
        $img->deleteFormattedImages();
    }

    return $result;
}

The difference was a matter of bytes - in fact, adding a destructor with imagedestroy() increased memory usage. I also checked with memory_get_peak_usage() and saw the same result. Can anyone see a flaw with that quick test, or does this really make no difference?

[dhensby]

PHP's garbage collection is probably taking care of it - when there are no more references the the image resource it'll automatically be destroyed - so adding it to the __destruct will make no difference

You want to be destroying the image resource as soon as it's no longer needed.

[jonom]

Think this can be closed, fixed for 3.2 thanks to Loz's work in #2859.

[tractorcow]

Thanks @jonom :D and @kinglozzer