MemcLockManager::getLocksOnServer() - Code Metrics - Inspection of "Daily Inspection: Merge "Use MAX_LOCK_TTL in Redis..." - wikimedia/mediawiki - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Branch — master (27c6e7)

unknown

created 2016-09-30 17:01 UTC

MemcLockManager::getLocksOnServer() C

↳ Parent: MemcLockManager

Complexity

Conditions	17
Paths	70

Size

Total Lines	74
Code Lines	43

Duplication

Lines	14
Ratio	18.92 %

Importance

Changes

Metric	Value
cc	17
eloc	43
nc	70
nop	2
dl	14
loc	74
rs	5.3488
c	0
b	0
f	0

How to fix Long Method Complexity

<?php
/**
 * Version of LockManager based on using memcached servers.
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program; if not, write to the Free Software Foundation, Inc.,
 * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 * http://www.gnu.org/copyleft/gpl.html
 *
 * @file
 * @ingroup LockManager
 */

/**
 * Manage locks using memcached servers.
 *
 * Version of LockManager based on using memcached servers.
 * This is meant for multi-wiki systems that may share files.
 * All locks are non-blocking, which avoids deadlocks.
 *
 * All lock requests for a resource, identified by a hash string, will map to one
 * bucket. Each bucket maps to one or several peer servers, each running memcached.
 * A majority of peers must agree for a lock to be acquired.
 *
 * @ingroup LockManager
 * @since 1.20
 */
class MemcLockManager extends QuorumLockManager {
	/** @var array Mapping of lock types to the type actually used */
	protected $lockTypeMap = [
		self::LOCK_SH => self::LOCK_SH,
		self::LOCK_UW => self::LOCK_SH,
		self::LOCK_EX => self::LOCK_EX
	];

	/** @var MemcachedBagOStuff[] Map of (server name => MemcachedBagOStuff) */
	protected $cacheServers = [];
	/** @var HashBagOStuff Server status cache */
	protected $statusCache;

	/**
	 * Construct a new instance from configuration.
	 *
	 * @param array $config Parameters include:
	 *   - lockServers  : Associative array of server names to "<IP>:<port>" strings.
	 *   - srvsByBucket : Array of 1-16 consecutive integer keys, starting from 0,
	 *                    each having an odd-numbered list of server names (peers) as values.
	 *   - memcConfig   : Configuration array for MemcachedBagOStuff::construct() with an
	 *                    additional 'class' parameter specifying which MemcachedBagOStuff
	 *                    subclass to use. The server names will be injected. [optional]
	 * @throws Exception
	 */
	public function __construct( array $config ) {
		parent::__construct( $config );

		// Sanitize srvsByBucket config to prevent PHP errors
		$this->srvsByBucket = array_filter( $config['srvsByBucket'], 'is_array' );
		$this->srvsByBucket = array_values( $this->srvsByBucket ); // consecutive

		$memcConfig = isset( $config['memcConfig'] ) ? $config['memcConfig'] : [];
		$memcConfig += [ 'class' => 'MemcachedPhpBagOStuff' ]; // default

		$class = $memcConfig['class'];
		if ( !is_subclass_of( $class, 'MemcachedBagOStuff' ) ) {
			throw new InvalidArgumentException( "$class is not of type MemcachedBagOStuff." );
		}

		foreach ( $config['lockServers'] as $name => $address ) {
			$params = [ 'servers' => [ $address ] ] + $memcConfig;
			$this->cacheServers[$name] = new $class( $params );
		}

		$this->statusCache = new HashBagOStuff();
	}

	protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
		$status = StatusValue::newGood();

		$memc = $this->getCache( $lockSrv );
		// List of affected paths
		$paths = call_user_func_array( 'array_merge', array_values( $pathsByType ) );
		$paths = array_unique( $paths );
		// List of affected lock record keys
		$keys = array_map( [ $this, 'recordKeyForPath' ], $paths );

		// Lock all of the active lock record keys...
		if ( !$this->acquireMutexes( $memc, $keys ) ) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
			foreach ( $paths as $path ) {
				$status->fatal( 'lockmanager-fail-acquirelock', $path );
			}

			return $status;
		}

		// Fetch all the existing lock records...
		$lockRecords = $memc->getMulti( $keys );

		$now = time();
		// Check if the requested locks conflict with existing ones...
		foreach ( $pathsByType as $type => $paths ) {
			foreach ( $paths as $path ) {
				$locksKey = $this->recordKeyForPath( $path );
				$locksHeld = isset( $lockRecords[$locksKey] )
					? self::sanitizeLockArray( $lockRecords[$locksKey] )
					: self::newLockArray(); // init
				foreach ( $locksHeld[self::LOCK_EX] as $session => $expiry ) {
					if ( $expiry < $now ) { // stale?
						unset( $locksHeld[self::LOCK_EX][$session] );
					} elseif ( $session !== $this->session ) {
						$status->fatal( 'lockmanager-fail-acquirelock', $path );
					}
				}
				if ( $type === self::LOCK_EX ) {
					foreach ( $locksHeld[self::LOCK_SH] as $session => $expiry ) {
						if ( $expiry < $now ) { // stale?
							unset( $locksHeld[self::LOCK_SH][$session] );
						} elseif ( $session !== $this->session ) {
							$status->fatal( 'lockmanager-fail-acquirelock', $path );
						}
					}
				}
				if ( $status->isOK() ) {
					// Register the session in the lock record array
					$locksHeld[$type][$this->session] = $now + $this->lockTTL;
					// We will update this record if none of the other locks conflict
					$lockRecords[$locksKey] = $locksHeld;
				}
			}
		}

		// If there were no lock conflicts, update all the lock records...
		if ( $status->isOK() ) {
			foreach ( $paths as $path ) {
				$locksKey = $this->recordKeyForPath( $path );
				$locksHeld = $lockRecords[$locksKey];
				$ok = $memc->set( $locksKey, $locksHeld, self::MAX_LOCK_TTL );
				if ( !$ok ) {
					$status->fatal( 'lockmanager-fail-acquirelock', $path );
				} else {
					$this->logger->debug( __METHOD__ . ": acquired lock on key $locksKey.\n" );
				}
			}
		}

		// Unlock all of the active lock record keys...
		$this->releaseMutexes( $memc, $keys );
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

		return $status;
	}

	protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
		$status = StatusValue::newGood();

		$memc = $this->getCache( $lockSrv );
		// List of affected paths
		$paths = call_user_func_array( 'array_merge', array_values( $pathsByType ) );
		$paths = array_unique( $paths );
		// List of affected lock record keys
		$keys = array_map( [ $this, 'recordKeyForPath' ], $paths );

		// Lock all of the active lock record keys...
		if ( !$this->acquireMutexes( $memc, $keys ) ) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
			foreach ( $paths as $path ) {
				$status->fatal( 'lockmanager-fail-releaselock', $path );
			}

			return $status;
		}

		// Fetch all the existing lock records...
		$lockRecords = $memc->getMulti( $keys );

		// Remove the requested locks from all records...
		foreach ( $pathsByType as $type => $paths ) {
			foreach ( $paths as $path ) {
				$locksKey = $this->recordKeyForPath( $path ); // lock record
				if ( !isset( $lockRecords[$locksKey] ) ) {
					$status->warning( 'lockmanager-fail-releaselock', $path );
					continue; // nothing to do
				}
				$locksHeld = $this->sanitizeLockArray( $lockRecords[$locksKey] );
				if ( isset( $locksHeld[$type][$this->session] ) ) {
					unset( $locksHeld[$type][$this->session] ); // unregister this session
					$lockRecords[$locksKey] = $locksHeld;
				} else {
					$status->warning( 'lockmanager-fail-releaselock', $path );
				}
			}
		}

		// Persist the new lock record values...
		foreach ( $paths as $path ) {
			$locksKey = $this->recordKeyForPath( $path );
			if ( !isset( $lockRecords[$locksKey] ) ) {
				continue; // nothing to do
			}
			$locksHeld = $lockRecords[$locksKey];
			if ( $locksHeld === $this->newLockArray() ) {
				$ok = $memc->delete( $locksKey );
			} else {
				$ok = $memc->set( $locksKey, $locksHeld, self::MAX_LOCK_TTL );
			}
			if ( $ok ) {
				$this->logger->debug( __METHOD__ . ": released lock on key $locksKey.\n" );
			} else {
				$status->fatal( 'lockmanager-fail-releaselock', $path );
			}
		}

		// Unlock all of the active lock record keys...
		$this->releaseMutexes( $memc, $keys );
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

		return $status;
	}

	/**
	 * @see QuorumLockManager::releaseAllLocks()
	 * @return StatusValue
	 */
	protected function releaseAllLocks() {
		return StatusValue::newGood(); // not supported
	}

	/**
	 * @see QuorumLockManager::isServerUp()
	 * @param string $lockSrv
	 * @return bool
	 */
	protected function isServerUp( $lockSrv ) {
		return (bool)$this->getCache( $lockSrv );
	}

	/**
	 * Get the MemcachedBagOStuff object for a $lockSrv
	 *
	 * @param string $lockSrv Server name
	 * @return MemcachedBagOStuff|null
	 */
	protected function getCache( $lockSrv ) {
		if ( !isset( $this->cacheServers[$lockSrv] ) ) {
			throw new InvalidArgumentException( "Invalid cache server '$lockSrv'." );
		}

		$online = $this->statusCache->get( "online:$lockSrv" );
		if ( $online === false ) {
			$online = $this->cacheServers[$lockSrv]->set( __CLASS__ . ':ping', 1, 1 );
			if ( !$online ) { // server down?
				$this->logger->warning( __METHOD__ . ": Could not contact $lockSrv." );
			}
			$this->statusCache->set( "online:$lockSrv", (int)$online, 30 );
		}

		return $online ? $this->cacheServers[$lockSrv] : null;
	}

	/**
	 * @param string $path
	 * @return string
	 */
	protected function recordKeyForPath( $path ) {
		return implode( ':', [ __CLASS__, 'locks', $this->sha1Base36Absolute( $path ) ] );
	}

	/**
	 * @return array An empty lock structure for a key
	 */
	protected function newLockArray() {
		return [ self::LOCK_SH => [], self::LOCK_EX => [] ];
	}

	/**
	 * @param array $a
	 * @return array An empty lock structure for a key
	 */
	protected function sanitizeLockArray( $a ) {
		if ( is_array( $a ) && isset( $a[self::LOCK_EX] ) && isset( $a[self::LOCK_SH] ) ) {
			return $a;
		}

		$this->logger->error( __METHOD__ . ": reset invalid lock array." );

		return $this->newLockArray();
	}

	/**
	 * @param MemcachedBagOStuff $memc
	 * @param array $keys List of keys to acquire
	 * @return bool
	 */
	protected function acquireMutexes( MemcachedBagOStuff $memc, array $keys ) {
		$lockedKeys = [];

		// Acquire the keys in lexicographical order, to avoid deadlock problems.
		// If P1 is waiting to acquire a key P2 has, P2 can't also be waiting for a key P1 has.
		sort( $keys );

		// Try to quickly loop to acquire the keys, but back off after a few rounds.
		// This reduces memcached spam, especially in the rare case where a server acquires
		// some lock keys and dies without releasing them. Lock keys expire after a few minutes.
		$loop = new WaitConditionLoop(
			function () use ( $memc, $keys, &$lockedKeys ) {
				foreach ( array_diff( $keys, $lockedKeys ) as $key ) {
					if ( $memc->add( "$key:mutex", 1, 180 ) ) { // lock record
						$lockedKeys[] = $key;
					}
				}

				return array_diff( $keys, $lockedKeys )
					? WaitConditionLoop::CONDITION_CONTINUE
					: true;
			},
			3.0 // timeout
		);
		$loop->invoke();

		if ( count( $lockedKeys ) != count( $keys ) ) {
			$this->releaseMutexes( $memc, $lockedKeys ); // failed; release what was locked
			return false;
		}

		return true;
	}

	/**
	 * @param MemcachedBagOStuff $memc
	 * @param array $keys List of acquired keys
	 */
	protected function releaseMutexes( MemcachedBagOStuff $memc, array $keys ) {
		foreach ( $keys as $key ) {
			$memc->delete( "$key:mutex" );
		}
	}

	/**
	 * Make sure remaining locks get cleared for sanity
	 */
	function __destruct() {
		while ( count( $this->locksHeld ) ) {
			foreach ( $this->locksHeld as $path => $locks ) {
				$this->doUnlock( [ $path ], self::LOCK_EX );
				$this->doUnlock( [ $path ], self::LOCK_SH );
			}
		}
	}
}


1		<?php
2		/**
3		* Version of LockManager based on using memcached servers.
4		*
5		* This program is free software; you can redistribute it and/or modify
6		* it under the terms of the GNU General Public License as published by
7		* the Free Software Foundation; either version 2 of the License, or
8		* (at your option) any later version.
9		*
10		* This program is distributed in the hope that it will be useful,
11		* but WITHOUT ANY WARRANTY; without even the implied warranty of
12		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13		* GNU General Public License for more details.
14		*
15		* You should have received a copy of the GNU General Public License along
16		* with this program; if not, write to the Free Software Foundation, Inc.,
17		* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
18		* http://www.gnu.org/copyleft/gpl.html
19		*
20		* @file
21		* @ingroup LockManager
22		*/
23
24		/**
25		* Manage locks using memcached servers.
26		*
27		* Version of LockManager based on using memcached servers.
28		* This is meant for multi-wiki systems that may share files.
29		* All locks are non-blocking, which avoids deadlocks.
30		*
31		* All lock requests for a resource, identified by a hash string, will map to one
32		* bucket. Each bucket maps to one or several peer servers, each running memcached.
33		* A majority of peers must agree for a lock to be acquired.
34		*
35		* @ingroup LockManager
36		* @since 1.20
37		*/
38		class MemcLockManager extends QuorumLockManager {
39		/** @var array Mapping of lock types to the type actually used */
40		protected $lockTypeMap = [
41		self::LOCK_SH => self::LOCK_SH,
42		self::LOCK_UW => self::LOCK_SH,
43		self::LOCK_EX => self::LOCK_EX
44		];
45
46		/** @var MemcachedBagOStuff[] Map of (server name => MemcachedBagOStuff) */
47		protected $cacheServers = [];
48		/** @var HashBagOStuff Server status cache */
49		protected $statusCache;
50
51		/**
52		* Construct a new instance from configuration.
53		*
54		* @param array $config Parameters include:
55		* - lockServers : Associative array of server names to "<IP>:<port>" strings.
56		* - srvsByBucket : Array of 1-16 consecutive integer keys, starting from 0,
57		* each having an odd-numbered list of server names (peers) as values.
58		* - memcConfig : Configuration array for MemcachedBagOStuff::construct() with an
59		* additional 'class' parameter specifying which MemcachedBagOStuff
60		* subclass to use. The server names will be injected. [optional]
61		* @throws Exception
62		*/
63		public function __construct( array $config ) {
64		parent::__construct( $config );
65
66		// Sanitize srvsByBucket config to prevent PHP errors
67		$this->srvsByBucket = array_filter( $config['srvsByBucket'], 'is_array' );
68		$this->srvsByBucket = array_values( $this->srvsByBucket ); // consecutive
69
70		$memcConfig = isset( $config['memcConfig'] ) ? $config['memcConfig'] : [];
71		$memcConfig += [ 'class' => 'MemcachedPhpBagOStuff' ]; // default
72
73		$class = $memcConfig['class'];
74		if ( !is_subclass_of( $class, 'MemcachedBagOStuff' ) ) {
75		throw new InvalidArgumentException( "$class is not of type MemcachedBagOStuff." );
76		}
77
78		foreach ( $config['lockServers'] as $name => $address ) {
79		$params = [ 'servers' => [ $address ] ] + $memcConfig;
80		$this->cacheServers[$name] = new $class( $params );
81		}
82
83		$this->statusCache = new HashBagOStuff();
84		}
85
86		protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
87		$status = StatusValue::newGood();
88
89		$memc = $this->getCache( $lockSrv );
90		// List of affected paths
91		$paths = call_user_func_array( 'array_merge', array_values( $pathsByType ) );
92		$paths = array_unique( $paths );
93		// List of affected lock record keys
94		$keys = array_map( [ $this, 'recordKeyForPath' ], $paths );
95
96		// Lock all of the active lock record keys...
97		if ( !$this->acquireMutexes( $memc, $keys ) ) {
		0 ignored issues – show Bug introduced 2016-02-23 18:13 UTC by Report Bug Copy Issue Report It seems like `$memc` defined by `$this->getCache($lockSrv)` on line `89` can be `null`; however, `MemcLockManager::acquireMutexes()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
98		foreach ( $paths as $path ) {
99		$status->fatal( 'lockmanager-fail-acquirelock', $path );
100		}
101
102		return $status;
103		}
104
105		// Fetch all the existing lock records...
106		$lockRecords = $memc->getMulti( $keys );
107
108		$now = time();
109		// Check if the requested locks conflict with existing ones...
110		foreach ( $pathsByType as $type => $paths ) {
111		foreach ( $paths as $path ) {
112		$locksKey = $this->recordKeyForPath( $path );
113		$locksHeld = isset( $lockRecords[$locksKey] )
114		? self::sanitizeLockArray( $lockRecords[$locksKey] )
115		: self::newLockArray(); // init
116	View Code Duplication	foreach ( $locksHeld[self::LOCK_EX] as $session => $expiry ) {
117		if ( $expiry < $now ) { // stale?
118		unset( $locksHeld[self::LOCK_EX][$session] );
119		} elseif ( $session !== $this->session ) {
120		$status->fatal( 'lockmanager-fail-acquirelock', $path );
121		}
122		}
123		if ( $type === self::LOCK_EX ) {
124	View Code Duplication	foreach ( $locksHeld[self::LOCK_SH] as $session => $expiry ) {
125		if ( $expiry < $now ) { // stale?
126		unset( $locksHeld[self::LOCK_SH][$session] );
127		} elseif ( $session !== $this->session ) {
128		$status->fatal( 'lockmanager-fail-acquirelock', $path );
129		}
130		}
131		}
132		if ( $status->isOK() ) {
133		// Register the session in the lock record array
134		$locksHeld[$type][$this->session] = $now + $this->lockTTL;
135		// We will update this record if none of the other locks conflict
136		$lockRecords[$locksKey] = $locksHeld;
137		}
138		}
139		}
140
141		// If there were no lock conflicts, update all the lock records...
142		if ( $status->isOK() ) {
143		foreach ( $paths as $path ) {
144		$locksKey = $this->recordKeyForPath( $path );
145		$locksHeld = $lockRecords[$locksKey];
146		$ok = $memc->set( $locksKey, $locksHeld, self::MAX_LOCK_TTL );
147		if ( !$ok ) {
148		$status->fatal( 'lockmanager-fail-acquirelock', $path );
149		} else {
150		$this->logger->debug( __METHOD__ . ": acquired lock on key $locksKey.\n" );
151		}
152		}
153		}
154
155		// Unlock all of the active lock record keys...
156		$this->releaseMutexes( $memc, $keys );
		0 ignored issues – show Bug introduced 2016-02-23 18:13 UTC by Report Bug Copy Issue Report It seems like `$memc` defined by `$this->getCache($lockSrv)` on line `89` can be `null`; however, `MemcLockManager::releaseMutexes()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
157
158		return $status;
159		}
160
161		protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
162		$status = StatusValue::newGood();
163
164		$memc = $this->getCache( $lockSrv );
165		// List of affected paths
166		$paths = call_user_func_array( 'array_merge', array_values( $pathsByType ) );
167		$paths = array_unique( $paths );
168		// List of affected lock record keys
169		$keys = array_map( [ $this, 'recordKeyForPath' ], $paths );
170
171		// Lock all of the active lock record keys...
172		if ( !$this->acquireMutexes( $memc, $keys ) ) {
		0 ignored issues – show Bug introduced 2016-02-23 18:13 UTC by Report Bug Copy Issue Report It seems like `$memc` defined by `$this->getCache($lockSrv)` on line `164` can be `null`; however, `MemcLockManager::acquireMutexes()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
173		foreach ( $paths as $path ) {
174		$status->fatal( 'lockmanager-fail-releaselock', $path );
175		}
176
177		return $status;
178		}
179
180		// Fetch all the existing lock records...
181		$lockRecords = $memc->getMulti( $keys );
182
183		// Remove the requested locks from all records...
184		foreach ( $pathsByType as $type => $paths ) {
185		foreach ( $paths as $path ) {
186		$locksKey = $this->recordKeyForPath( $path ); // lock record
187		if ( !isset( $lockRecords[$locksKey] ) ) {
188		$status->warning( 'lockmanager-fail-releaselock', $path );
189		continue; // nothing to do
190		}
191		$locksHeld = $this->sanitizeLockArray( $lockRecords[$locksKey] );
192		if ( isset( $locksHeld[$type][$this->session] ) ) {
193		unset( $locksHeld[$type][$this->session] ); // unregister this session
194		$lockRecords[$locksKey] = $locksHeld;
195		} else {
196		$status->warning( 'lockmanager-fail-releaselock', $path );
197		}
198		}
199		}
200
201		// Persist the new lock record values...
202		foreach ( $paths as $path ) {
203		$locksKey = $this->recordKeyForPath( $path );
204		if ( !isset( $lockRecords[$locksKey] ) ) {
205		continue; // nothing to do
206		}
207		$locksHeld = $lockRecords[$locksKey];
208		if ( $locksHeld === $this->newLockArray() ) {
209		$ok = $memc->delete( $locksKey );
210		} else {
211		$ok = $memc->set( $locksKey, $locksHeld, self::MAX_LOCK_TTL );
212		}
213		if ( $ok ) {
214		$this->logger->debug( __METHOD__ . ": released lock on key $locksKey.\n" );
215		} else {
216		$status->fatal( 'lockmanager-fail-releaselock', $path );
217		}
218		}
219
220		// Unlock all of the active lock record keys...
221		$this->releaseMutexes( $memc, $keys );
		0 ignored issues – show Bug introduced 2016-02-23 18:13 UTC by Report Bug Copy Issue Report It seems like `$memc` defined by `$this->getCache($lockSrv)` on line `164` can be `null`; however, `MemcLockManager::releaseMutexes()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
222
223		return $status;
224		}
225
226		/**
227		* @see QuorumLockManager::releaseAllLocks()
228		* @return StatusValue
229		*/
230		protected function releaseAllLocks() {
231		return StatusValue::newGood(); // not supported
232		}
233
234		/**
235		* @see QuorumLockManager::isServerUp()
236		* @param string $lockSrv
237		* @return bool
238		*/
239		protected function isServerUp( $lockSrv ) {
240		return (bool)$this->getCache( $lockSrv );
241		}
242
243		/**
244		* Get the MemcachedBagOStuff object for a $lockSrv
245		*
246		* @param string $lockSrv Server name
247		* @return MemcachedBagOStuff\|null
248		*/
249		protected function getCache( $lockSrv ) {
250		if ( !isset( $this->cacheServers[$lockSrv] ) ) {
251		throw new InvalidArgumentException( "Invalid cache server '$lockSrv'." );
252		}
253
254		$online = $this->statusCache->get( "online:$lockSrv" );
255		if ( $online === false ) {
256		$online = $this->cacheServers[$lockSrv]->set( __CLASS__ . ':ping', 1, 1 );
257		if ( !$online ) { // server down?
258		$this->logger->warning( __METHOD__ . ": Could not contact $lockSrv." );
259		}
260		$this->statusCache->set( "online:$lockSrv", (int)$online, 30 );
261		}
262
263		return $online ? $this->cacheServers[$lockSrv] : null;
264		}
265
266		/**
267		* @param string $path
268		* @return string
269		*/
270		protected function recordKeyForPath( $path ) {
271		return implode( ':', [ __CLASS__, 'locks', $this->sha1Base36Absolute( $path ) ] );
272		}
273
274		/**
275		* @return array An empty lock structure for a key
276		*/
277		protected function newLockArray() {
278		return [ self::LOCK_SH => [], self::LOCK_EX => [] ];
279		}
280
281		/**
282		* @param array $a
283		* @return array An empty lock structure for a key
284		*/
285		protected function sanitizeLockArray( $a ) {
286		if ( is_array( $a ) && isset( $a[self::LOCK_EX] ) && isset( $a[self::LOCK_SH] ) ) {
287		return $a;
288		}
289
290		$this->logger->error( __METHOD__ . ": reset invalid lock array." );
291
292		return $this->newLockArray();
293		}
294
295		/**
296		* @param MemcachedBagOStuff $memc
297		* @param array $keys List of keys to acquire
298		* @return bool
299		*/
300		protected function acquireMutexes( MemcachedBagOStuff $memc, array $keys ) {
301		$lockedKeys = [];
302
303		// Acquire the keys in lexicographical order, to avoid deadlock problems.
304		// If P1 is waiting to acquire a key P2 has, P2 can't also be waiting for a key P1 has.
305		sort( $keys );
306
307		// Try to quickly loop to acquire the keys, but back off after a few rounds.
308		// This reduces memcached spam, especially in the rare case where a server acquires
309		// some lock keys and dies without releasing them. Lock keys expire after a few minutes.
310		$loop = new WaitConditionLoop(
311		function () use ( $memc, $keys, &$lockedKeys ) {
312		foreach ( array_diff( $keys, $lockedKeys ) as $key ) {
313		if ( $memc->add( "$key:mutex", 1, 180 ) ) { // lock record
314		$lockedKeys[] = $key;
315		}
316		}
317
318		return array_diff( $keys, $lockedKeys )
319		? WaitConditionLoop::CONDITION_CONTINUE
320		: true;
321		},
322		3.0 // timeout
323		);
324		$loop->invoke();
325
326		if ( count( $lockedKeys ) != count( $keys ) ) {
327		$this->releaseMutexes( $memc, $lockedKeys ); // failed; release what was locked
328		return false;
329		}
330
331		return true;
332		}
333
334		/**
335		* @param MemcachedBagOStuff $memc
336		* @param array $keys List of acquired keys
337		*/
338		protected function releaseMutexes( MemcachedBagOStuff $memc, array $keys ) {
339		foreach ( $keys as $key ) {
340		$memc->delete( "$key:mutex" );
341		}
342		}
343
344		/**
345		* Make sure remaining locks get cleared for sanity
346		*/
347		function __destruct() {
348		while ( count( $this->locksHeld ) ) {
349		foreach ( $this->locksHeld as $path => $locks ) {
350		$this->doUnlock( [ $path ], self::LOCK_EX );
351		$this->doUnlock( [ $path ], self::LOCK_SH );
352		}
353		}
354		}
355		}
356

wikimedia / mediawiki

Branch — master (27c6e7)

MemcLockManager::getLocksOnServer() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like