From 5f921702d29fb4a11d73efaa9d2ca758c01c15f6 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 2 Aug 2016 03:57:52 -0700 Subject: [PATCH] Avoid stack overflow in LoadBalancer with CACHE_DB WAN/server cache * Add the ability to expose key BagOStuff attributes like whether the emulation SQL cache is being used. Several callers use hacks to detect this and can be updated later. * Fallback to a safe empty WAN cache in the CACHE_DB case. This fixes a regression from f4bf52e8438. * Also add this protection to the server cache, which could break similarly before too. * Also fix CACHE_ANYTHING by avoid the recursion risk from fc1d4d796024 by just checking the disabled service map. Bug: T123829 Bug: T141804 Change-Id: I17ee26138f69e01ec1aaddb55ab27caa4d542193 --- includes/Services/ServiceContainer.php | 8 +++++ includes/db/loadbalancer/LoadBalancer.php | 16 +++++++-- includes/libs/objectcache/BagOStuff.php | 35 ++++++++++++++++++- includes/libs/objectcache/CachedBagOStuff.php | 4 ++- includes/libs/objectcache/IExpiringStore.php | 7 ++++ .../libs/objectcache/MultiWriteBagOStuff.php | 1 + .../libs/objectcache/ReplicatedBagOStuff.php | 1 + includes/libs/objectcache/WANObjectCache.php | 11 +++++- includes/objectcache/ObjectCache.php | 8 ++--- includes/objectcache/SqlBagOStuff.php | 3 ++ 10 files changed, 84 insertions(+), 10 deletions(-) diff --git a/includes/Services/ServiceContainer.php b/includes/Services/ServiceContainer.php index b336795ee4..bad0ef9ce6 100644 --- a/includes/Services/ServiceContainer.php +++ b/includes/Services/ServiceContainer.php @@ -367,4 +367,12 @@ class ServiceContainer implements DestructibleService { return $service; } + /** + * @param string $name + * @return bool Whether the service is disabled + * @since 1.28 + */ + public function isServiceDisabled( $name ) { + return isset( $this->disabled[$name] ); + } } diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php index dbf031ebaa..b44b559b9e 100644 --- a/includes/db/loadbalancer/LoadBalancer.php +++ b/includes/db/loadbalancer/LoadBalancer.php @@ -138,8 +138,20 @@ class LoadBalancer { } } - $this->srvCache = ObjectCache::getLocalServerInstance(); - $this->wanCache = ObjectCache::getMainWANInstance(); + // Use APC/memcached style caching, but avoids loops with CACHE_DB (T141804) + // @TODO: inject these in via LBFactory at some point + $cache = ObjectCache::getLocalServerInstance(); + if ( $cache->getQoS( $cache::ATTR_EMULATION ) > $cache::QOS_EMULATION_SQL ) { + $this->srvCache = $cache; + } else { + $this->srvCache = new EmptyBagOStuff(); + } + $wCache = ObjectCache::getMainWANInstance(); + if ( $wCache->getQoS( $wCache::ATTR_EMULATION ) > $wCache::QOS_EMULATION_SQL ) { + $this->wanCache = $wCache; + } else { + $this->wanCache = WANObjectCache::newEmpty(); + } if ( isset( $params['trxProfiler'] ) ) { $this->trxProfiler = $params['trxProfiler']; diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php index 1a2711abe9..25a5a2628e 100644 --- a/includes/libs/objectcache/BagOStuff.php +++ b/includes/libs/objectcache/BagOStuff.php @@ -46,7 +46,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { /** @var array[] Lock tracking */ protected $locks = []; - /** @var integer */ + /** @var integer ERR_* class constant */ protected $lastError = self::ERR_NONE; /** @var string */ @@ -70,6 +70,9 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { /** @var bool */ private $dupeTrackScheduled = false; + /** @var integer[] Map of (ATTR_* class constant => QOS_* class constant) */ + protected $attrMap = []; + /** Possible values for getLastError() */ const ERR_NONE = 0; // no error const ERR_NO_RESPONSE = 1; // no response @@ -734,4 +737,34 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { public function makeKey() { return $this->makeKeyInternal( $this->keyspace, func_get_args() ); } + + /** + * @param integer $flag ATTR_* class constant + * @return integer QOS_* class constant + * @since 1.28 + */ + public function getQoS( $flag ) { + return isset( $this->attrMap[$flag] ) ? $this->attrMap[$flag] : self::QOS_UNKNOWN; + } + + /** + * Merge the flag maps of one or more BagOStuff objects into a "lowest common denominator" map + * + * @param BagOStuff[] $bags + * @return integer[] Resulting flag map (class ATTR_* constant => class QOS_* constant) + */ + protected function mergeFlagMaps( array $bags ) { + $map = []; + foreach ( $bags as $bag ) { + foreach ( $bag->attrMap as $attr => $rank ) { + if ( isset( $map[$attr] ) ) { + $map[$attr] = min( $map[$attr], $rank ); + } else { + $map[$attr] = $rank; + } + } + } + + return $map; + } } diff --git a/includes/libs/objectcache/CachedBagOStuff.php b/includes/libs/objectcache/CachedBagOStuff.php index 60ec9222f0..e70a51f93d 100644 --- a/includes/libs/objectcache/CachedBagOStuff.php +++ b/includes/libs/objectcache/CachedBagOStuff.php @@ -42,8 +42,10 @@ class CachedBagOStuff extends HashBagOStuff { * @param array $params Parameters for HashBagOStuff */ function __construct( BagOStuff $backend, $params = [] ) { - $this->backend = $backend; parent::__construct( $params ); + + $this->backend = $backend; + $this->attrMap = $backend->attrMap; } protected function doGet( $key, $flags = 0 ) { diff --git a/includes/libs/objectcache/IExpiringStore.php b/includes/libs/objectcache/IExpiringStore.php index 91e7934a09..62c4fa5ec3 100644 --- a/includes/libs/objectcache/IExpiringStore.php +++ b/includes/libs/objectcache/IExpiringStore.php @@ -42,4 +42,11 @@ interface IExpiringStore { const TTL_PROC_LONG = 30; // loose cache time that can survive slow web requests const TTL_INDEFINITE = 0; + + // Attribute and QoS constants; higher QOS values with the same prefix rank higher... + // Medium attributes constants related to emulation or media type + const ATTR_EMULATION = 1; + const QOS_EMULATION_SQL = 1; + // Generic "unknown" value that is useful for comparisons (e.g. always good enough) + const QOS_UNKNOWN = INF; } diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php index fe61470284..9dcfa7c55e 100644 --- a/includes/libs/objectcache/MultiWriteBagOStuff.php +++ b/includes/libs/objectcache/MultiWriteBagOStuff.php @@ -83,6 +83,7 @@ class MultiWriteBagOStuff extends BagOStuff { $this->caches[] = ObjectFactory::getObjectFromSpec( $cacheInfo ); } } + $this->mergeFlagMaps( $this->caches ); $this->asyncWrites = ( isset( $params['replication'] ) && diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php b/includes/libs/objectcache/ReplicatedBagOStuff.php index 5f2c509ed6..f2ba9de032 100644 --- a/includes/libs/objectcache/ReplicatedBagOStuff.php +++ b/includes/libs/objectcache/ReplicatedBagOStuff.php @@ -65,6 +65,7 @@ class ReplicatedBagOStuff extends BagOStuff { $this->readStore = ( $params['readFactory'] instanceof BagOStuff ) ? $params['readFactory'] : ObjectFactory::getObjectFromSpec( $params['readFactory'] ); + $this->attrMap = $this->mergeFlagMaps( [ $this->readStore, $this->writeStore ] ); } public function setDebug( $debug ) { diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index b5d014f673..4fd40e2fba 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -977,7 +977,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { /** * Get the "last error" registered; clearLastError() should be called manually - * @return int ERR_* constant for the "last error" registry + * @return int ERR_* class constant for the "last error" registry */ final public function getLastError() { if ( $this->lastRelayError ) { @@ -1019,6 +1019,15 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $this->procCache->clear(); } + /** + * @param integer $flag ATTR_* class constant + * @return integer QOS_* class constant + * @since 1.28 + */ + public function getQoS( $flag ) { + return $this->cache->getQoS( $flag ); + } + /** * Do the actual async bus purge of a key * diff --git a/includes/objectcache/ObjectCache.php b/includes/objectcache/ObjectCache.php index e1bb2db7c5..26f3356195 100644 --- a/includes/objectcache/ObjectCache.php +++ b/includes/objectcache/ObjectCache.php @@ -228,14 +228,12 @@ class ObjectCache { } } - try { - // Make sure we actually have a DB backend before falling back to CACHE_DB - MediaWikiServices::getInstance()->getDBLoadBalancer(); - $candidate = CACHE_DB; - } catch ( ServiceDisabledException $e ) { + if ( MediaWikiServices::getInstance()->isServiceDisabled( 'DBLoadBalancer' ) ) { // The LoadBalancer is disabled, probably because // MediaWikiServices::disableStorageBackend was called. $candidate = CACHE_NONE; + } else { + $candidate = CACHE_DB; } return self::getInstance( $candidate ); diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php index 98b6eb94da..c48880fe79 100644 --- a/includes/objectcache/SqlBagOStuff.php +++ b/includes/objectcache/SqlBagOStuff.php @@ -91,6 +91,9 @@ class SqlBagOStuff extends BagOStuff { */ public function __construct( $params ) { parent::__construct( $params ); + + $this->attrMap[self::ATTR_EMULATION] = self::QOS_EMULATION_SQL; + if ( isset( $params['servers'] ) ) { $this->serverInfos = []; $this->serverTags = []; -- 2.20.1