Commit a16db5ac authored by alexpott's avatar alexpott

Issue #2531570 by mondrake, MattA, claudiu.cristea: Image objects should...

Issue #2531570 by mondrake, MattA, claudiu.cristea: Image objects should release memory when losing scope
parent 9e2450bb
......@@ -51,9 +51,9 @@ class Image implements ImageInterface {
*/
public function __construct(ImageToolkitInterface $toolkit, $source = NULL) {
$this->toolkit = $toolkit;
$this->getToolkit()->setImage($this);
if ($source) {
$this->source = $source;
$this->getToolkit()->setSource($this->source);
// Defer image file validity check to the toolkit.
if ($this->getToolkit()->parseFile()) {
$this->fileSize = filesize($this->source);
......
......@@ -32,11 +32,11 @@ abstract class ImageToolkitBase extends PluginBase implements ImageToolkitInterf
protected $configFactory;
/**
* Image object this toolkit instance is tied to.
* Path of the image file.
*
* @var \Drupal\Core\Image\ImageInterface
* @var string
*/
protected $image;
protected $source = '';
/**
* The image toolkit operation manager.
......@@ -84,18 +84,22 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
/**
* {@inheritdoc}
*/
public function setImage(ImageInterface $image) {
if ($this->image) {
public function setSource($source) {
// If a previous image has been loaded, there is no way to know if the
// toolkit implementation needs to perform any additional actions like
// freeing memory. Therefore, the source image cannot be changed once set.
if ($this->source) {
throw new \BadMethodCallException(__METHOD__ . '() may only be called once');
}
$this->image = $image;
$this->source = $source;
return $this;
}
/**
* {@inheritdoc}
*/
public function getImage() {
return $this->image;
public function getSource() {
return $this->source;
}
/**
......
......@@ -53,23 +53,27 @@
interface ImageToolkitInterface extends ContainerFactoryPluginInterface, PluginInspectionInterface, PluginFormInterface {
/**
* Sets the image object that this toolkit instance is tied to.
* Sets the source path of the image file.
*
* @param \Drupal\Core\Image\ImageInterface $image
* The image that this toolkit instance will be tied to.
* @param string $source
* The source path of the image file.
*
* @return \Drupal\Core\ImageToolkit\ImageToolkitInterface
* An instance of the current toolkit object.
*
* @throws \BadMethodCallException
* When called twice.
* After being set initially, the source image cannot be changed.
*/
public function setImage(ImageInterface $image);
public function setSource($source);
/**
* Gets the image object that this toolkit instance is tied to.
* Gets the source path of the image file.
*
* @return \Drupal\Core\Image\ImageInterface
* The image object that this toolkit instance is tied to.
* @return string
* The source path of the image file, or an empty string if the source is
* not set.
*/
public function getImage();
public function getSource();
/**
* Checks if the image is valid.
......
......@@ -13,10 +13,10 @@
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\ImageToolkit\ImageToolkitBase;
use Drupal\Core\ImageToolkit\ImageToolkitOperationManagerInterface;
use Drupal\Core\StreamWrapper\StreamWrapperInterface;
use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Drupal\Core\StreamWrapper\StreamWrapperInterface;
/**
* Defines the GD2 toolkit for image manipulation within Drupal.
......@@ -65,7 +65,7 @@ class GDToolkit extends ImageToolkitBase {
protected $streamWrapperManager;
/**
* Constructs a TestToolkit object.
* Constructs a GDToolkit object.
*
* @param array $configuration
* A configuration array containing information about the plugin instance.
......@@ -87,6 +87,17 @@ public function __construct(array $configuration, $plugin_id, array $plugin_defi
$this->streamWrapperManager = $stream_wrapper_manager;
}
/**
* Destructs a GDToolkit object.
*
* Frees memory associated with a GD image resource.
*/
public function __destruct() {
if (is_resource($this->resource)) {
imagedestroy($this->resource);
}
}
/**
* {@inheritdoc}
*/
......@@ -108,9 +119,13 @@ public static function create(ContainerInterface $container, array $configuratio
* @param resource $resource
* The GD image resource.
*
* @return $this
* @return \Drupal\system\Plugin\ImageToolkit\GDToolkit
* An instance of the current toolkit object.
*/
public function setResource($resource) {
if (!is_resource($resource) || get_resource_type($resource) != 'gd') {
throw new \InvalidArgumentException('Invalid resource argument');
}
$this->preLoadInfo = NULL;
$this->resource = $resource;
return $this;
......@@ -123,7 +138,7 @@ public function setResource($resource) {
* The GD image resource, or NULL if not available.
*/
public function getResource() {
if (!$this->resource) {
if (!is_resource($this->resource)) {
$this->load();
}
return $this->resource;
......@@ -167,7 +182,7 @@ protected function load() {
}
$function = 'imagecreatefrom' . image_type_to_extension($this->getType(), FALSE);
if (function_exists($function) && $resource = $function($this->getImage()->getSource())) {
if (function_exists($function) && $resource = $function($this->getSource())) {
$this->setResource($resource);
if (imageistruecolor($resource)) {
return TRUE;
......@@ -242,7 +257,7 @@ public function save($destination) {
* {@inheritdoc}
*/
public function parseFile() {
$data = @getimagesize($this->getImage()->getSource());
$data = @getimagesize($this->getSource());
if ($data && in_array($data[2], static::supportedTypes())) {
$this->setType($data[2]);
$this->preLoadInfo = $data;
......
......@@ -34,7 +34,7 @@ protected function arguments() {
protected function execute(array $arguments) {
// PHP installations using non-bundled GD do not have imagefilter.
if (!function_exists('imagefilter')) {
$this->logger->notice("The image '@file' could not be desaturated because the imagefilter() function is not available in this PHP installation.", array('@file' => $this->getToolkit()->getImage()->getSource()));
$this->logger->notice("The image '@file' could not be desaturated because the imagefilter() function is not available in this PHP installation.", array('@file' => $this->getToolkit()->getSource()));
return FALSE;
}
......
......@@ -94,7 +94,7 @@ protected function validateArguments(array $arguments) {
protected function execute(array $arguments) {
// PHP installations using non-bundled GD do not have imagerotate.
if (!function_exists('imagerotate')) {
$this->logger->notice('The image %file could not be rotated because the imagerotate() function is not available in this PHP installation.', array('%file' => $this->getToolkit()->getImage()->getSource()));
$this->logger->notice('The image %file could not be rotated because the imagerotate() function is not available in this PHP installation.', array('%file' => $this->getToolkit()->getSource()));
return FALSE;
}
......
......@@ -8,8 +8,8 @@
namespace Drupal\system\Tests\Image;
use Drupal\Core\Image\ImageInterface;
use \Drupal\simpletest\KernelTestBase;
use Drupal\Component\Utility\SafeMarkup;
use Drupal\simpletest\KernelTestBase;
/**
* Tests that core image manipulations work properly: scale, resize, rotate,
......@@ -401,6 +401,18 @@ function testManipulations() {
$this->assertTrue($image->isValid(), 'CreateNew with valid arguments validates the Image.');
}
/**
* Tests that GD resources are freed from memory.
*/
public function testResourceDestruction() {
$image = $this->imageFactory->get(drupal_get_path('module', 'simpletest') . '/files/image-test.png');
$res = $image->getToolkit()->getResource();
$this->assertTrue(is_resource($res), 'Successfully loaded image resource.');
// Force the toolkit to go out of scope.
$image = NULL;
$this->assertFalse(is_resource($res), 'Image resource was destroyed after losing scope.');
}
/**
* Tests loading an image whose transparent color index is out of range.
*/
......
......@@ -138,7 +138,7 @@ public function isValid() {
*/
public function parseFile() {
$this->logCall('parseFile', func_get_args());
$data = @getimagesize($this->getImage()->getSource());
$data = @getimagesize($this->getSource());
if ($data && in_array($data[2], static::supportedTypes())) {
$this->setType($data[2]);
$this->width = $data[0];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment