Skip to content

Commit

Permalink
feature #23724 [Lock] Deprecate Filesystem/LockHandler (jderusse)
Browse files Browse the repository at this point in the history
This PR was merged into the 3.4 branch.

Discussion
----------

[Lock] Deprecate Filesystem/LockHandler

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        | symfony/symfony-docs#8243

This PR deprecate the `Filesystem\LockHandler` in favor of `Lock\SemaphoreStore` and `Lock\FlockStore`.

Commits
-------

67ecc71 Deprecate Filesystem/LockHandler
  • Loading branch information
fabpot committed Aug 1, 2017
2 parents c36262e + 67ecc71 commit 57a86fb
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 22 deletions.
7 changes: 7 additions & 0 deletions UPGRADE-3.4.md
Expand Up @@ -11,6 +11,13 @@ Debug

* Support for stacked errors in the `ErrorHandler` is deprecated and will be removed in Symfony 4.0.

Filesystem
----------

* The `Symfony\Component\Filesystem\LockHandler` class has been deprecated,
use the `Symfony\Component\Lock\Store\FlockStore` class
or the `Symfony\Component\Lock\Store\FlockStore\SemaphoreStore` class directly instead.

Finder
------

Expand Down
7 changes: 7 additions & 0 deletions UPGRADE-4.0.md
Expand Up @@ -147,6 +147,13 @@ ExpressionLanguage
class has been removed. You should use the `CacheItemPoolInterface` interface
instead.

Filesystem
----------

* The `Symfony\Component\Filesystem\LockHandler` has been removed,
use the `Symfony\Component\Lock\Store\FlockStore` class
or the `Symfony\Component\Lock\Store\FlockStore\SemaphoreStore` class directly instead.

Finder
------

Expand Down
31 changes: 20 additions & 11 deletions src/Symfony/Component/Console/Command/LockableTrait.php
Expand Up @@ -13,7 +13,10 @@

use Symfony\Component\Console\Exception\LogicException;
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Filesystem\LockHandler;
use Symfony\Component\Lock\Factory;
use Symfony\Component\Lock\Lock;
use Symfony\Component\Lock\Store\FlockStore;
use Symfony\Component\Lock\Store\SemaphoreStore;

/**
* Basic lock feature for commands.
Expand All @@ -22,7 +25,8 @@
*/
trait LockableTrait
{
private $lockHandler;
/** @var Lock */
private $lock;

/**
* Locks a command.
Expand All @@ -31,18 +35,23 @@ trait LockableTrait
*/
private function lock($name = null, $blocking = false)
{
if (!class_exists(LockHandler::class)) {
throw new RuntimeException('To enable the locking feature you must install the symfony/filesystem component.');
if (!class_exists(SemaphoreStore::class)) {
throw new RuntimeException('To enable the locking feature you must install the symfony/lock component.');
}

if (null !== $this->lockHandler) {
if (null !== $this->lock) {
throw new LogicException('A lock is already in place.');
}

$this->lockHandler = new LockHandler($name ?: $this->getName());
if (SemaphoreStore::isSupported($blocking)) {
$store = new SemaphoreStore();
} else {
$store = new FlockStore(sys_get_temp_dir());
}

if (!$this->lockHandler->lock($blocking)) {
$this->lockHandler = null;
$this->lock = (new Factory($store))->createLock($name ?: $this->getName());
if (!$this->lock->acquire($blocking)) {
$this->lock = null;

return false;
}
Expand All @@ -55,9 +64,9 @@ private function lock($name = null, $blocking = false)
*/
private function release()
{
if ($this->lockHandler) {
$this->lockHandler->release();
$this->lockHandler = null;
if ($this->lock) {
$this->lock->release();
$this->lock = null;
}
}
}
14 changes: 11 additions & 3 deletions src/Symfony/Component/Console/Tests/Command/LockableTraitTest.php
Expand Up @@ -13,7 +13,9 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\Filesystem\LockHandler;
use Symfony\Component\Lock\Factory;
use Symfony\Component\Lock\Store\FlockStore;
use Symfony\Component\Lock\Store\SemaphoreStore;

class LockableTraitTest extends TestCase
{
Expand All @@ -39,8 +41,14 @@ public function testLockReturnsFalseIfAlreadyLockedByAnotherCommand()
{
$command = new \FooLockCommand();

$lock = new LockHandler($command->getName());
$lock->lock();
if (SemaphoreStore::isSupported(false)) {
$store = new SemaphoreStore();
} else {
$store = new FlockStore(sys_get_temp_dir());
}

$lock = (new Factory($store))->createLock($command->getName());
$lock->acquire();

$tester = new CommandTester($command);
$this->assertSame(1, $tester->execute(array()));
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Console/composer.json
Expand Up @@ -25,13 +25,13 @@
"symfony/http-kernel": "~2.8|~3.0|~4.0",
"symfony/event-dispatcher": "~2.8|~3.0|~4.0",
"symfony/dependency-injection": "~3.3|~4.0",
"symfony/filesystem": "~2.8|~3.0|~4.0",
"symfony/lock": "~3.4|~4.0",
"symfony/process": "~3.3|~4.0",
"psr/log": "~1.0"
},
"suggest": {
"symfony/event-dispatcher": "",
"symfony/filesystem": "",
"symfony/lock": "",
"symfony/process": "",
"psr/log": "For using the console logger"
},
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/Filesystem/LockHandler.php
Expand Up @@ -11,7 +11,11 @@

namespace Symfony\Component\Filesystem;

@trigger_error(sprintf('The %s class is deprecated since version 3.4 and will be removed in 4.0. Use %s or %s instead.', LockHandler::class, SemaphoreStore::class, FlockStore::class), E_USER_DEPRECATED);

use Symfony\Component\Filesystem\Exception\IOException;
use Symfony\Component\Lock\Store\FlockStore;
use Symfony\Component\Lock\Store\SemaphoreStore;

/**
* LockHandler class provides a simple abstraction to lock anything by means of
Expand All @@ -25,6 +29,8 @@
* @author Grégoire Pineau <lyrixx@lyrixx.info>
* @author Romain Neutron <imprec@gmail.com>
* @author Nicolas Grekas <p@tchwork.com>
*
* @deprecated since version 3.4, to be removed in 4.0. Use Symfony\Component\Lock\Store\SemaphoreStore or Symfony\Component\Lock\Store\FlockStore instead.
*/
class LockHandler
{
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/Filesystem/Tests/LockHandlerTest.php
Expand Up @@ -16,6 +16,9 @@
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\Filesystem\LockHandler;

/**
* @group legacy
*/
class LockHandlerTest extends TestCase
{
/**
Expand Down
19 changes: 17 additions & 2 deletions src/Symfony/Component/Lock/Store/SemaphoreStore.php
Expand Up @@ -24,9 +24,24 @@
*/
class SemaphoreStore implements StoreInterface
{
public static function isSupported()
/**
* Returns whether or not the store is supported.
*
* @param bool|null $blocking When not null, checked again the blocking mode.
*
* @return bool
*/
public static function isSupported($blocking = null)
{
return extension_loaded('sysvsem');
if (!extension_loaded('sysvsem')) {
return false;
}

if ($blocking === false && \PHP_VERSION_ID < 50601) {
return false;
}

return true;
}

public function __construct()
Expand Down
Expand Up @@ -35,7 +35,7 @@ abstract protected function getStore();
public function testBlockingLocks()
{
// Amount a microsecond used to order async actions
$clockDelay = 50000;
$clockDelay = 200000;

if (\PHP_VERSION_ID < 50600 || defined('HHVM_VERSION_ID')) {
$this->markTestSkipped('The PHP engine does not keep resource in child forks');
Expand All @@ -49,7 +49,7 @@ public function testBlockingLocks()

if ($childPID1 = pcntl_fork()) {
// give time to fork to start
usleep(2 * $clockDelay);
usleep(1 * $clockDelay);

try {
// This call should failed given the lock should already by acquired by the child #1
Expand All @@ -69,8 +69,8 @@ public function testBlockingLocks()
} else {
try {
$store->save($key);
// Wait 3 ClockDelay to let parent process to finish
usleep(3 * $clockDelay);
// Wait 2 ClockDelay to let parent process to finish
usleep(2 * $clockDelay);
$store->delete($key);
exit(0);
} catch (\Exception $e) {
Expand Down

0 comments on commit 57a86fb

Please sign in to comment.