WordPress.org

Make WordPress Core

Opened 12 days ago

Last modified 5 days ago

#48522 new defect (bug)

Attachment size not generated when large images uploaded

Reported by: vanyukov Owned by:
Milestone: 5.3.1 Priority: normal
Severity: normal Version: 5.3
Component: Media Keywords: reporter-feedback
Focuses: Cc:
PR Number:

Description

I've noticed that some images during upload do not generate attachment sizes at all, or generate the sizes partially. An example of such an image: https://upload.wikimedia.org/wikipedia/commons/f/ff/Pizigani_1367_Chart_10MB.jpg

Upload is successful, but there's no scaled image, as well as no attachments.

Change History (14)

This ticket was mentioned in Slack in #core-media by vanyukov. View the logs.


12 days ago

#2 @vanyukov
12 days ago

Just some more info. I've traced the issue to the thumbnail_image() failing inside the wp_create_image_subsizes() function with a "cache resources exhausted" error during the generation of the scaled image. Later on when WordPress tries to _wp_make_subsizes(), the image editor is still returned with an error. A possible solution would be to generate attachments from smallest to largest. That way ImageMagick is able to work... it's magic... and users will, at least, get some attachments generated. Now in 5.3, there is absolutely nothing being generated, compared to 5.2.4

Another possible solution would be to resample images to a smaller destination size? Or even make this dynamic? First try to resample to the default 5x size, if that fails, try the 4x and so on... That would probably require revisiting the DSSIM tests and making sure the resample does not change the image quality. But, again, from the users perspective - any image is better than nothing.

Will happily submit a patch to any of the above changes

#3 @johnbillion
12 days ago

  • Component changed from Upload to Media
  • Milestone changed from Awaiting Review to 5.3
  • Version set to trunk

Thank you for the report @vanyukov and the offer of help.

Moving to 5.3 for visibility.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


11 days ago

#5 @dinhtungdu
11 days ago

In my testing, the problem only occurs with ImageMagick as the media handler. Image sizes were generated normally with the default GD library. I have 11 image sizes generated when GD is the media handler, while it's only 5 sizes with ImageMagick.

The screenshot of VM when generating image sizes with ImageMagick:

https://i.imgur.com/XC2D766.png

My env:

  • Virtualbox Ubuntu 16.04.6 LTS
  • 1 Core
  • 1024GB RAM
  • PHP 7.3.9 with max_execution_time and max_input_time set to 30 (900 has the same result)
  • Latest WordPress svn revision.
  • ImageMagick 6.8.9-9
  • Imagick module version 3.4.4

#6 @azaozz
10 days ago

  • Keywords reporter-feedback added
  • Milestone changed from 5.3 to 5.3.1

Yes, image upload can still fail despite all the retry requests. The last one should be to delete the attachment and any existing sub-sizes that were created. Failing to delete it (to do cleanup) is a bug.

Testing with the linked image, it works properly here and creates all sub-sizes after 2-3 retries. Limiting the server resources makes it fail, but the attachment is deleted at the end. If it is not deleted in your tests, could you look in the last AJAX request ant/or possibly add some debugging in wp_ajax_media_create_image_subsizes() in wp-admin/includes/ajax-actions.php to try to figure out what's stopping it.

Moving this to 5.3.1 for now as it seems to be working as expected.

#7 follow-up: @vanyukov
8 days ago

@azaozz, the file upload doesn't fail. the wp_ajax_media_create_image_subsizes() is not triggered on my test install.

#8 in reply to: ↑ 7 @azaozz
7 days ago

Replying to vanyukov:

the wp_ajax_media_create_image_subsizes() is not triggered on my test install.

What requests do you see in the browser tools network tab?

There should be one to async-upload.php that will end with an HTTP 5.x.x error when the server runs out of resources. That should trigger an admin-ajax.php request which is the first retry attempt. If that also ends with HTTP 5.x.x error, another one should follow, up to four times. Finally if all retries fail there should be another admin-ajax.php request to do a "cleanup" of the failed upload. That should delete the newly created attachment and show the error message "...please scale down the image and try to upload again".

A possible solution would be to generate attachments from smallest to largest.

This is already the case. The image sub-sizes are sorted by "usage priority" in wp_create_image_subsizes(). However if generating a registered sub-size fails, the upload is deemed unsuccessful. This may change in the future but keep in mind that currently there are five attempts to generate the sub-sizes. If all fail, chances are that the image cannot be resizes as uploaded and best next step would be for the user to scale it down and try uploading again.

Eventually these retries can be "exposed" to the users by having a "partial" status for attachments with some UI/a button to retry to create the missing sub-sizes. But that will bring more complexity and in most cases will still fail at the end. Thinking that for now that can be added by plugins. If such plugins become popular, can perhaps be merged to core :)

#9 follow-up: @uglyrobot
7 days ago

None of this answers why larger images like this sample fail all resizes on 5.3 when it works fine on 5.2 with the same server and ImageMagick configuration. We can bump up the default ImageMagick config to 2gb disk cache from default 1GB which seems to fix it but that seems excessive and likely must affect many other hosting providers who no doubt use the default config. There is some sort of performance regression with ImageMagick in 5.3.

As far as network requests, I just get a 200 response from the initial upload with attachment id, then the ajax response with the image row HTML. No http errors or php errors in logs. But no thumbs are generated, only the original image sits in the uploads directory. It fails silently.

#10 @uglyrobot
7 days ago

I'm just theorizing here that it's the new 5.3 really large original resize that fails, and that there is a bug that prevents the rest of the smaller thumbs (that would work in 5.2) to also get skipped due to:

Later on when WordPress tries to _wp_make_subsizes(), the image editor is still returned with an error

#11 in reply to: ↑ 9 @azaozz
7 days ago

Replying to uglyrobot:

There is some sort of performance regression with ImageMagick in 5.3.

As far as network requests, I just get a 200 response from the initial upload with attachment id, then the ajax response with the image row HTML. No http errors or php errors in logs. But no thumbs are generated, only the original image sits in the uploads directory. It fails silently.

Testing with the 10MB image using GD in 5.2 fails with (the dreaded) HTTP Error here. I don't currently have access to ImageMagick. Would you be able to "get to the bottom of it" and determine the exact ImageMagick error and where it comes from?

I'm just theorizing here that it's the new 5.3 really large original resize that fails...

Yes, it's possible it fails as the size is larger than the previous default sizes. If it fails at this point, the retries will still try to create the same -scaled image. However there are even larger "sub-sizes" added by themes/plugins, and generally wp-admin should be able to handle this kind of sub-sizes.

The bug in this case is that it fails silently and the user is not notified to scale down the image and try to upload it again. Would be great to catch the error and show that error message in the UI.

Last edited 7 days ago by azaozz (previous) (diff)

#12 @vanyukov
7 days ago

@azaozz,

thumbnail_image() is failing inside the wp_create_image_subsizes() function with a "cache resources exhausted" error. the async-upload.php never fails, because the resources do not run out during upload, they run out during an attempt to create the new big image and the error for that is not passed down to async-upload.php. I've attached a trace call:

https://i.imgur.com/4vsHSGrr.png

Also, I've asked in Slack, but haven't yet received a response. I'm not sure I understand how everyone is defaulting to GD during tests, when _wp_image_editor_choose() is set to return an instance of WP_Image_Editor_Imagick first. Am I missing something?

This ticket was mentioned in Slack in #core by vanyukov. View the logs.


5 days ago

#14 @vanyukov
5 days ago

As per discussion from Slack, I'm moving the discussion to trac.

I see three possible solutions to the problem.

  1. Adjust the resample size

In my tests, this change fixes the issue: inside wp-includes/class-wp-image-editor-imagick.php (around line 343) instead of:

$this->image->sampleImage( $dst_w * $sample_factor, $dst_h * $sample_factor );

try to dynamically adjust the $sample_factor:

do {
    try {
        unset( $e );
        $this->image->sampleImage( $dst_w * $sample_factor, $dst_h * $sample_factor );
    } catch ( Exception $e ) {
        $sample_factor--;
        continue;
    }

    break;
} while ( $sample_factor > 0 );

if ( isset( $e ) && $e instanceof ImagickException ) {
    throw $e;
}

My main concern is that this change might not be the best way to go forward without doing proper DSSIM tests to make sure that there is no visible change in images with the new resample size. The previous tests were done a while back and, I believe, the max-width tested was 1200px. Much has changed since then.

  1. Error handle this.

The problem with this approach - it does not fix the initial issue. There will still be no guaranteed way to resize large attachments.

  1. Fallback to GD

I can see that this is not an issue with GD. So maybe use GD as a default editor when both ImageMagick and GD are installed? Currently _wp_image_editor_choose() is set to return WP_Image_Editor_Imagick by default. Also an easy fix.

Note: See TracTickets for help on using tickets.