WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 3 weeks ago

#22661 reopened defect (bug)

Allow object caches to degrade gracefully

Reported by: markjaquith Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Cache API Keywords: has-patch
Focuses: Cc:

Description

Because of the way object caches are loaded, if a custom object cache can't run (say, Memcached or APC is not actually installed), it cannot gracefully degrade to the built-in object cache.

Witness this code in wp_start_object_cache():

 	if ( ! function_exists( 'wp_cache_init' ) ) {
		if ( file_exists( WP_CONTENT_DIR . '/object-cache.php' ) ) {
			require_once ( WP_CONTENT_DIR . '/object-cache.php' );
			$_wp_using_ext_object_cache = true;
		} else {
			require_once ( ABSPATH . WPINC . '/cache.php' );
			$_wp_using_ext_object_cache = false;
		}
		$first_init = true;
	} else if ( !$_wp_using_ext_object_cache && file_exists( WP_CONTENT_DIR . '/object-cache.php' ) ) {
		// Sometimes advanced-cache.php can load object-cache.php before it is loaded here.
		// This breaks the function_exists check above and can result in $_wp_using_ext_object_cache
		// being set incorrectly. Double check if an external cache exists.
		$_wp_using_ext_object_cache = true;
	}

So a custom object cache is loaded. If it wants to bail and defer to the built in object caching, it can do that by doing the include itself. But then WordPress sets $_wp_using_ext_object_cache = true; after that require. So WordPress thinks it is using an external object cache when it's actually not. This leads to oddness.

This can sometimes be hacked around by adding a callback to the very first WP action available that sets $_wp_using_ext_object_cache = false;, but that has issues: calls to the object cache might be made before that code can run, and add_action() is not always available at that point (because some advanced-cache.php drop-ins load the object cache really early. See Batcache.

Proposed solution: change the order of the require_once() and the setting of $_wp_using_ext_object_cache. That way, the external object cache can override the variable.

Attachments (2)

22661.patch (598 bytes) - added by SergeyBiryukov 7 weeks ago.
22661.2.patch (751 bytes) - added by SergeyBiryukov 4 weeks ago.

Download all attachments as: .zip

Change History (12)

comment:1 wonderboymusic17 months ago

this is mildly related: #21401

comment:2 scribu17 months ago

  • Cc scribu added

comment:3 aaroncampbell15 months ago

  • Cc aaroncampbell added

comment:4 wonderboymusic2 months ago

  • Resolution set to fixed
  • Status changed from new to closed

Unless I'm missing something, we did this here: [25289].

SergeyBiryukov7 weeks ago

comment:5 SergeyBiryukov7 weeks ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.9
  • Resolution fixed deleted
  • Status changed from closed to reopened

We still call wp_using_ext_object_cache( true ) after the require_once(), so this ticket was not addressed in [25289]. I guess 22661.patch should resolve this.

SergeyBiryukov4 weeks ago

comment:6 SergeyBiryukov4 weeks ago

Looks like 22661.patch might not work after [27634] (it was relying upon the initial null value).

22661.2.patch is another take. A custom object cache should be able to bail and defer to the built-in caching by including the file and calling wp_using_ext_object_cache( false ).

Last edited 4 weeks ago by SergeyBiryukov (previous) (diff)

comment:7 ircbot3 weeks ago

This ticket was mentioned in IRC in #wordpress-dev by DH-Shredder. View the logs.

comment:8 nacin3 weeks ago

[27634] can be reverted if necessary, but it felt like proper behavior to me. 22661.2.patch sounds good to me.

comment:9 nacin3 weeks ago

Well, there is some additional code here:

	} else if ( ! wp_using_ext_object_cache() && file_exists( WP_CONTENT_DIR . '/object-cache.php' ) ) {
		// Sometimes advanced-cache.php can load object-cache.php before it is loaded here.
		// This breaks the function_exists check above and can result in $_wp_using_ext_object_cache
		// being set incorrectly. Double check if an external cache exists.
		wp_using_ext_object_cache( true );
	}

So this wouldn't work in the case of Batcache loading object-cache.php earlier.

comment:10 nacin3 weeks ago

  • Milestone changed from 3.9 to Future Release

I'm going to punt this until it has an approach that handles all cases.

Note: See TracTickets for help on using tickets.