Problème ImportExport avec le nouveau destructeur de Varien_Image_Adapter_Gd2 dans 1.9.2.0

23

Quelqu'un peut-il expliquer à quoi sert le code suivant introduit entre Magento CE 1.9.1.0 et 1.9.2.0?

class Varien_Image_Adapter_Gd2:

public function __construct()
{
    // Initialize shutdown function
    register_shutdown_function(array($this, 'destruct'));
}

/**
 * Destroy object image on shutdown
 */
public function destruct()
{
    @imagedestroy($this->_imageHandler);
}

Une fois ces deux fonctions ajoutées, notre importation d'images de la galerie de produits avec l'interface ImportExport a cessé de fonctionner. L'erreur est due à une limite de mémoire (qui s'avère être la limite maximale de taille de fichier ouvert).

Mon idée est que les fichiers ouverts par l'importation ne seront pas fermés correctement.

J'ai également vu qu'il y avait des destruct()fonctions vides introduites ( Mage_ImportExport_Model_Import_Adapter_Abstract) - mais les étendre pour qu'elles correspondent à la logique parente n'aide pas.

Achim Rosenhagen
la source

Réponses:

14

On dirait qu'ils ont essayé de s'assurer de détruire la ressource d'image, mais ont plutôt introduit une fuite de mémoire. Je ne peux pas penser à une raison valable pour ce code, pour être honnête, mais je peux expliquer ce qui a été changé:

A l'origine, imagedestroy()aurait été appelé dans le desctructor__destruct()

function __destruct()
{
    @imagedestroy($this->_imageHandler);
}

Le destructeur est appelé chaque fois que le garbage collector PHP détruit des objets inutilisés (c'est-à-dire des objets en mémoire qui ne sont plus référencés).

Maintenant, imagedestroy()est appelé à la place dans une fonction d'arrêt et comme il s'agit d'un rappel d'une méthode de l' Varien_Image_Adapter_Gd2objet, il ne peut même pas être récupéré jusqu'à la fin. De cette façon, toutes les ressources d'image restent ouvertes jusqu'à la fin de l'exécution du script.

Fabian Schmengler
la source
Merci pour l'explication - c'est ce à quoi j'ai pensé. Donc, dans l'ensemble, ce code introduit rend la majorité des importations inutiles sur 1.9.2. dans mes yeux. J'espère que cela sera corrigé bientôt. Un conseil où ouvrir un rapport de bogue?
Achim Rosenhagen
6

Avoir les mêmes problèmes avec mon Magento 1.9.2.0 ...

Je ne reçois que cela fonctionne , en changeant Varien_Image_Adapter_Gd2 dans la /lib/Varien/Image/Adapter/Gd2.phpmanière suivante:

public function __construct()
{
    // Initialize shutdown function
    // register_shutdown_function(array($this, 'destruct'));
}

/**
 * Destroy object image on shutdown
 */
public function __destruct()
{
    @imagedestroy($this->_imageHandler);
}
  • supprimer la ligne avec register_shutdown_function (ou commenter)
  • changer le nom de la fonction destruct en __destruct

J'ai remis memory_limit à 1G (auparavant, j'ai augmenté jusqu'à 32 Go) et maintenant cela fonctionne ...

Ce projet met en œuvre ladite procédure de manière conviviale pour Modman. Installez-le simplement avec le compositeur et vous êtes prêt à partir.

dkr
la source
Cela ne répond pas vraiment à la question. Si vous avez une autre question, vous pouvez la poser en cliquant sur Poser une question . Vous pouvez également ajouter une prime pour attirer davantage l'attention sur cette question une fois que vous avez suffisamment de réputation .
Rajeev K Tomy
Oui, cela ne répond pas à la question mais aidera les personnes qui ont besoin d'une solution temporaire et sans discussion
dkr
correction d'un problème de consommation de mémoire lors de l'importation. Intéressant, Magento teste-t-il en quelque sorte ce qu'ils publient?
klipach
Cela ne résout pas seulement le problème d'importation. Cela résout une grande mémoire consommée par le processus qui crée / recrée le cache et les versions redimensionnées pour les images de chaque produit. Si je télécharge des images png dans mes produits, sans ce "hack" je ne peux pas travailler et j'obtiens beaucoup d'erreurs de mémoire épuisées.
Simbus82
Aujourd'hui, j'ai trouvé cette suggestion. Je l'ai implémenté et la fuite de mémoire a disparu. J'ai ensuite créé ce github.com/borasocom-team/magento-gd2-memoryleak pour l'installer de manière propre.
Dr.Gianluigi Zane Zanettini
5

Cela faisait partie de la résolution des problèmes de sécurité avec la désérialisation. Les méthodes magiques comme __destruct ont des problèmes inhérents à la sérialisation.

Nous avons vu des exploits proposés qui utilisaient la sérialisation et __destruct pour créer des fichiers dans le système de fichiers - et ce changement (vous verrez des changements plus similaires dans d'autres endroits) a été fait pour éviter cela.

Cela provoque-t-il une fuite de mémoire ou utilise-t-il simplement plus de mémoire jusqu'à la fin du script?

/security/77549/is-php-unserialize-exploitable-without-any-interesting-methods

Piotr Kaminski
la source
Merci pour le contexte. Ce changement particulier a-t-il été fait pour empêcher un exploit spécifique ou juste pour être sûr?
Fabian Schmengler
Et non, cela fait probablement que le script consomme plus de mémoire, pas une véritable fuite de mémoire
Fabian Schmengler
Cela provoque une énorme fuite de mémoire, en particulier lors de l'importation d'images, car il gardera tous les fichiers d'image ouverts jusqu'à la fin du traitement d'importation. De cette façon, nous ne pouvons importer qu'environ 50 produits (alors qu'avant, nous pouvons faire une importation de> 2k de manière apparente). J'ai exécuté mon test sur une machine virtuelle locale avec 8 Go de RAM et les fichiers source font tous environ 300 Ko. Avant le changement, la mémoire utilisée par PHP rermains à 1k pendant toute l'importation.
Achim Rosenhagen
fschmengler a raison - ce n'est peut-être pas une «fuite de mémoire» mais la consommation monte jusqu'à la colline ;-)
Achim Rosenhagen
1
@Alex merci pour le conseil. Je l'inverse patché. Maintenant, la fuite de mémoire a disparu, mais aucune solution pour l'avenir.
Arne du
4

J'ai donc soulevé un bogue avec Magento, y compris une "solution" qui devrait traiter les problèmes d'utilisation de la mémoire dans le processus d'importation d'image.

La solution peut être trouvée sur github sous https://github.com/sitewards/import_image_memory_leak_fix mais l'idée de base est.

Correction de l' Mage_Catalog_Helper_Image::validateUploadFileappel de la destructméthode sur le processeur d'image. Malheureusement, il semble que la valeur par défaut Varien_Imagene traite pas avec un destructdonc nous avons dû ajouter notre propre classe qui le fait.

<?php
/**
 * @category    Sitewards
 * @package     Sitewards_ImportImageMemoryLeakFix
 * @copyright   Copyright (c) Sitewards GmbH (http://www.sitewards.com/)
 */
class Sitewards_ImportImageMemoryLeakFix_Model_Destructable_Image extends Varien_Image
{
    /**
     * Constructor,
     * difference from original constructor - we register a destructor here.
     *
     * @param string $sFileName
     * @param Varien_Image_Adapter $oAdapter Default value is GD2
     */
    public function __construct($sFileName = null, $oAdapter = Varien_Image_Adapter::ADAPTER_GD2)
    {
        parent::__construct($sFileName, $oAdapter);

        // Initialize shutdown function
        register_shutdown_function(array($this, 'destruct'));
    }

    /**
     * Destroy object image on shutdown
     */
    public function destruct()
    {
        $oAdapter = $this->_getAdapter();
        if (method_exists($oAdapter, 'destruct')) {
            $oAdapter->destruct();
        } else {
            Mage::log('Image can not be destructed properly, adapter doesn\'t support the method.');
        }
    }
}

Et puis une réécriture de l'aide.

<?xml version="1.0"?>
<config>
    <modules>
        <Sitewards_ImportImageMemoryLeakFix>
            <version>0.1.0</version>
        </Sitewards_ImportImageMemoryLeakFix>
    </modules>
    <global>
        <models>
            <sitewards_importimagememoryleakfix>
                <class>Sitewards_ImportImageMemoryLeakFix_Model</class>
            </sitewards_importimagememoryleakfix>
        </models>
        <helpers>
            <catalog>
                <rewrite>
                    <image>Sitewards_ImportImageMemoryLeakFix_Helper_Catalog_Helper_Image</image>
                </rewrite>
            </catalog>
        </helpers>
    </global>
</config>

Et la nouvelle fonction appelle la nouvelle classe d'images destructibles.

<?php
/**
 * @category    Sitewards
 * @package     Sitewards_ImportImageMemoryLeakFix
 * @copyright   Copyright (c) Sitewards GmbH (http://www.sitewards.com/)
 */
class Sitewards_ImportImageMemoryLeakFix_Helper_Catalog_Helper_Image extends Mage_Catalog_Helper_Image
{
    /**
     * Check - is this file an image
     *
     * Difference from original method - we destroy the image object here,
     * i.e. we are not wasting memory, without that fix product import with images
     * easily goes over 4Gb on memory with just couple hundreds of products.
     *
     * @param string $sFilePath
     *
     * @return bool
     * @throws Mage_Core_Exception
     */
    public function validateUploadFile($sFilePath) {
        if (!getimagesize($sFilePath)) {
            Mage::throwException($this->__('Disallowed file type.'));
        }

        /** @var Sitewards_ImportImageMemoryLeakFix_Model_Destructable_Image $oImageProcessor */
        $oImageProcessor = Mage::getModel('sitewards_importimagememoryleakfix/destructable_image', $sFilePath);
        $sMimeType       = $oImageProcessor->getMimeType();
        $oImageProcessor->destruct();

        return $sMimeType !== null;
    }
}
David Manners
la source