magento-lts icon indicating copy to clipboard operation
magento-lts copied to clipboard

memory leak when resizing multiple product images

Open ioweb-gr opened this issue 4 years ago • 6 comments

I've stumbled into this issue a while back and noted this from somewhere on the internet. Unfortunately I don't have the link anymore.

The issue is that when you loop through products and resize their main image you will at some point run out of memory. It seems the GD adapter is not freeing up memory correctly with it's custom shutdown function.

In my sites I used to override the adapter locally and it worked OK so I'm sharing the code I found. I'm not sure if you want to merge it though as it may interfere in other areas.

It will be good to have it noted down here anyways as a record that this problem exists and has a solution. Credits go to unknown person that wrote this down

ioweb-gr avatar Jun 30 '20 08:06 ioweb-gr

Thanks for the PR!

colinmollenhour avatar Jul 03 '20 22:07 colinmollenhour

Any other opinions to move this forward?

ADDISON74 avatar May 28 '22 18:05 ADDISON74

In this stackoverflow, there is a discussion about this issue. Someone commented about security concern with the __destruct() method.

kiatng avatar May 30 '22 06:05 kiatng

Interesting comments on Stackoverflow. The concern about security is coming from Magento Senior Piotr Kaminski. I would prefer the last solution offered in that post by logging the error too. It is implemented here:

https://github.com/sitewards/import_image_memory_leak_fix

ADDISON74 avatar May 30 '22 09:05 ADDISON74

To prevent security issues around serialization we could:

  1. disable serialization for this class
  2. make sure unserialization is safe (implementing e.g. __sleep and __wakeup) But to be honest, we solved serialization issues by implementing "allowed classes" everywhere where magento unserializes data, didn't we?

tmotyl avatar May 30 '22 10:05 tmotyl

Yeah I'm failing to see how this code could pose a security risk.. especially with "allowed classes".

colinmollenhour avatar Jun 09 '22 23:06 colinmollenhour