Problem/Motivation

There are couple of issues with the current way of spawning the ImageMagick processes on the shell via proc_open. See related issues.

Proposed resolution

See if using the Symfony process component the issues get fixed. Commit to 8.x-2.x, but since that component is in Drupal core's composer.json, if the approach works then it could be backported to 8.x-1.x.

Remaining tasks

Test thoroughly, especially on Windows machines.

User interface changes

none

API changes

none

Data model changes

none

CommentFileSizeAuthor
#14 revert-2911289-14.patch3.37 KBjunaidpv
#8 2911289-8.patch15.07 KBmondrake
#8 interdiff_5-8.txt2.59 KBmondrake
#5 2911289-5.patch13.85 KBmondrake
#5 interdiff_4-5.txt874 bytesmondrake
#4 2911289-4.patch13.79 KBmondrake
#4 interdiff_3-4.txt2.07 KBmondrake
#3 interdiff_2-3.txt11.86 KBmondrake
#3 2911289-3.patch13.24 KBmondrake
#2 2911289-2.patch2.77 KBmondrake

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2911289-2.patch2.77 KB
mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new 2911289-3.patch13.24 KB
new interdiff_2-3.txt11.86 KB

More elaborated patch, with tests.

Using the Symfony process component also allows us to set a timeout for the process, that is terminated when 60 seconds pass without response from the ImageMagick binary. Hopefully this addresses also the related issues that are about processes that pile up forever.

Tests are running fine on a TravisCI build on Trusty, both with Imagemagick and Graphicsmagick installed, https://travis-ci.org/mondrake/imagemagick/builds/279841170

Would be great to have feedback if this also works on Windows boxes.

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new interdiff_3-4.txt2.07 KB
new 2911289-4.patch13.79 KB
mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new interdiff_4-5.txt874 bytes
new 2911289-5.patch13.85 KB

Code style cleanup (also #4)

Status: Needs review » Needs work

The last submitted patch, 5: 2911289-5.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review

#5 is green

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new interdiff_5-8.txt2.59 KB
new 2911289-8.patch15.07 KB

Adjusted tests to allow testing on Windows through AppVeyor https://ci.appveyor.com/project/mondrake/imagemagick/build/1.0.80

  • mondrake committed fc8e585 on 8.x-2.x 
    Issue #2911289 by mondrake: Use Symfony process component instead of...
mondrake’s picture

mondrake
Location 🇮🇹
commented
Version: 8.x-2.x-dev » 8.x-1.x-dev
Status: Needs review » Patch (to be ported)

Committed to 8.x-2.x

Leaving open for a backport to 8.x-1.x

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +release notes

Note to self: in next release mention the 60 sec timeout

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Patch (to be ported) » Fixed

Backport is unlikely at this stage.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

junaidpv’s picture

junaidpv
Location Kannur, Kerala
commented
StatusFileSize
new revert-2911289-14.patch3.37 KB

ImageMagick module causes to hang site when media entities are created as well as when uploading images in theme settings page.
I noticed image style images generation are very slow.

Also, we get lot of this warning with latest ImageMagic module 8.x-2.5:

Warning: proc_get_status() has been disabled for security reasons in Symfony\Component\Process\Process->updateStatus() (line 1430 of ../vendor/symfony/process/Process.php) 
#0 ../core/includes/bootstrap.inc(587): _drupal_error_handler_real(2, 'proc_get_status...', '/data/disk/o537...', 1430, Array) 

#1 [internal function]: _drupal_error_handler(2, 'proc_get_status...', '/data/disk/o537...', 1430, Array) 
#2 ../vendor/symfony/process/Process.php(1430): proc_get_status(Resource id #630) 
#3 ../vendor/symfony/process/Process.php(359): Symfony\Component\Process\Process->updateStatus(false) 
#4 ../vendor/symfony/process/Process.php(210): Symfony\Component\Process\Process->start(NULL, NULL) 
#5 ../modules/imagemagick/src/ImagemagickExecManager.php(379): Symfony\Component\Process\Process->run() 
#6 ../modules/imagemagick/src/ImagemagickExecManager.php(452): Drupal\imagemagick\ImagemagickExecManager->runOsShell('locale', '-a', 'locale', '') 
#7 ../modules/imagemagick/src/Plugin/ImageToolkit/ImagemagickToolkit.php(380): Drupal\imagemagick\ImagemagickExecManager->getInstalledLocales() 
#8 ../core/modules/system/src/Form/ImageToolkitForm.php(94): Drupal\imagemagick\Plugin\ImageToolkit\ImagemagickToolkit->buildConfigurationForm(Array, Object(Drupal\Core\Form\FormState)) 
#9 [internal function]: Drupal\system\Form\ImageToolkitForm->buildForm(Array, Object(Drupal\Core\Form\FormState)) 
#10 ../core/lib/Drupal/Core/Form/FormBuilder.php(519): call_user_func_array(Array, Array) 
#11 ../core/lib/Drupal/Core/Form/FormBuilder.php(276): Drupal\Core\Form\FormBuilder->retrieveForm('system_image_to...', Object(Drupal\Core\Form\FormState)) 
#12 ../core/lib/Drupal/Core/Controller/FormController.php(93): Drupal\Core\Form\FormBuilder->buildForm('system_image_to...', Object(Drupal\Core\Form\FormState)) 
#13 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch)) 
#14 ../core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array) 
#15 ../core/lib/Drupal/Core/Render/Renderer.php(582): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() 
#16 ../core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure)) 
#17 ../core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) 
#18 ../vendor/symfony/http-kernel/HttpKernel.php(151): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() 
#19 ../vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) 
#20 ../core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#21 ../core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#22 ../core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#23 ../core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#24 ../core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#25 ../core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#26 ../vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#27 ../core/lib/Drupal/Core/DrupalKernel.php(693): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) 
#28 ../index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request)) 
#29 {main}.

Reverting changes in fixes all the issues I mentioned above.