register_shutdown_function isn't run in SHORTINIT mode

[jtclarke]

I was noticing problems upgrading from 2.9.2 MU to 3.0.4 where some of my images weren't loading correctly through ms-files.php when I enabled the drop-in object-cache.php (in my case Ryan et al's memcache client).

I tracked the problem down to an inconsistency between MU and the new 3.0 codebase in the wp-settings.php file.

The problem is that if the SHORTINIT constant is set (as it is in ms-files.php), it returns false (at line 94) before we the shutdown function is registered (line 271). The shutdown function primarily takes care of object cash closing -- which needs to take place in both the short and regular load sequences.

donncha fixed this in the MU codebase at revision 1582 in the wordpress-mu SVN tree (between 2.6 and 2.7), but it looks like this change didn't get carried over in the 3.0 merge.

I recommend we restore the fix from 1582 (partially) by moving register_shutdown_function above the SHORTINIT check.

This should only affect multisite installations (the only ones that use SHORTINIT), but should fix the problem

[jtclarke]

wp-settings.php patch for SHORTINIT

[jtclarke]

the attached diff file resolves the issue with SHORTINIT.

please confirm.

[westi]

​http://trac.mu.wordpress.org/changeset/1582

[westi]

What part of ms-files.php is broken by this code being where it is now?

In other words how can I reproduce the issue that this change fixes - there is little history to say why this should be done :)

[nacin]

Sounds like this is for an object cache needing to leverage the wp_cache_close() call in the shutdown handler. Given the OP stating the Memcached plugin in use, and that the MU changeset gives props to Ryan, and that the Memcached plugin uses wp_cache_close()... I'm removing reporter-feedback. :)

Given this was something missing in the merge, and moving this has basically no ill side effects, this looks good.

[westi]

Replying to nacin:

Sounds like this is for an object cache needing to leverage the wp_cache_close() call in the shutdown handler. Given the OP stating the Memcached plugin in use, and that the MU changeset gives props to Ryan, and that the Memcached plugin uses wp_cache_close()... I'm removing reporter-feedback. :)

Given this was something missing in the merge, and moving this has basically no ill side effects, this looks good.

Yes but from a quick glance I don't see anything causing cache writes in ms-files.php

This has been gone for a long time so while it is probably prudent to move it to the place it was in mu - we should know why!

There was no explanation as to what needed this to move.

Hence the request for more info from the reporter.

[jtclarke]

Hey -- sorry I somehow wasn't getting requests for feedback.

The way I saw this was that on the sites I manage (MU with Batcache / Memcache), some (but not all) of the images were not being served correctly when Batcache was enabled and memcached was turned on.

I too am surprised nobody has encountered this before. Technically, the only way you would see problems is using 3.0 multisite (which uses ms-files.php and SHORTINIT) with Batcache/Memcache -- but I can't imagine we're the only ones doing that.

It's unclear why it would affect some files and not all. But I know that the problem was fixed if I turned off SHORTINIT -- which led me to dig deeper into the MU code and the placement of register_shutdown_function.

Let me provide some concrete steps to reproduce. Hopefully that'll clear things up. But it certainly was done in the MU codebase for a reason -- and the fact that it wasn't carried over to 3.0 was an oversight, not a choice.

[jtclarke]

Steps to reproduce:

Using a multisite installation of Wordpress 3.0.4, the TwentyTen theme and no plugins enabled (to eliminate any extraneous variables) and ensuring my system has the PHP5-Memcache extension:

1. Drop in Ryan et al's memcache client (object-cache.php.)
2. Start memcached
3. Begin creating new posts in wp-admin. With each new post, upload an image via the media uploader and try inserting as many sizes as are available into the post / Featured Image.

For me, even on the first post, what I see is that only some of the image sizes are served correctly. For example. I see the Full size and Post Thumb, but not the 150x150 thumbnail. See screenshot1.png. This affects the thumb in both wp-admin and on the frontend.

If I try loading the failing image directly from its url (eg. ​http://flavordev.local.mu/files/2011/02/6413719-150x150.jpg), the image does not load -- all I get is the url string.

When I remove object-cache.php, all images appear as they should. See screenshot2.png.

If I restore object-cache.php (making sure to clear my browser cache) the 150x150 disappears again. See screenshot3.png.

If I leave object-cache.php in place and institute the change I suggested in the diff file, everything serves as it should. See screenshot4.png.

Let me know if you need any more information.

[jtclarke]

Batcache enabled, wp-admin file upload

[jtclarke]

Batcache disabled, wp-admin file upload

[jtclarke]

Batcache re-enabled, wp-admin file upload

[jtclarke]

Batcache enabled, fix implemented -- wp-admin file upload

[jtclarke]

As a side note, I'm not the first person to see this issue. See: ​http://wordpress.org/support/topic/wordpress-30-multisite-mode-and-object-cache-with-memcached-not-showing-images?replies=4

It seems like Fernando solved the problem by installing a newer version of the PECL Memcache client. But the client he recommends is still in beta -- which isn't ideal.

The client I use on my environment is the latest stable (2.2.6) and with the mod suggested in the diff file, everything works as it should.

[jtclarke]

hello? anyone?

[jtclarke]

This is still a problem in 3.1. Modifying ticket accordingly.

[coffee2code]

The version field is used to report when the issue was first observed/reported. A comment to indicate that it still applies in 3.1 (as you've done) and the fact that the ticket remains opened is sufficient to keep the ticket up-to-date.

[jtclarke]

Could someone give this bug some love? It's done. It's fixed. it's been sitting here for 5 months.

[ryan]

Sorry. This slipped off the radar. The patch seems good, and I'm marking this for early inclusion into 3.3 since we missed the 3.2 boat.

[jtclarke]

sweet. thanks Ryan :)

[ryan]

In [18450]:

Call register_shutdown_function() for SHORTINIT. Props jtclarke. fixes #16389