MAGETWO-88237: Prevent running again already running cron group #12497

app/etc/di.xml

@@ -37,6 +37,7 @@
     <preference for="Magento\Framework\Locale\ListsInterface" type="Magento\Framework\Locale\TranslatedLists" />
     <preference for="Magento\Framework\Locale\AvailableLocalesInterface" type="Magento\Framework\Locale\Deployed\Codes" />
     <preference for="Magento\Framework\Locale\OptionInterface" type="Magento\Framework\Locale\Deployed\Options" />
+    <preference for="Magento\Framework\Lock\LockManagerInterface" type="Magento\Framework\Lock\Backend\Database" />
     <preference for="Magento\Framework\Api\AttributeTypeResolverInterface" type="Magento\Framework\Reflection\AttributeTypeResolver" />
     <preference for="Magento\Framework\Api\Search\SearchResultInterface" type="Magento\Framework\Api\Search\SearchResult" />
     <preference for="Magento\Framework\Api\Search\SearchCriteriaInterface" type="Magento\Framework\Api\Search\SearchCriteria"/>

dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/DatabaseTest.php

@@ -0,0 +1,50 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+/**
+ * \Magento\Framework\Lock\Backend\Database test case
+ */
+namespace Magento\Framework\Lock\Backend;
+
+class DatabaseTest extends \PHPUnit\Framework\TestCase
+{
+    /**
+     * @var \Magento\Framework\Lock\Backend\Database
+     */
+    private $model;
+
+    /**
+     * @var \Magento\Framework\ObjectManagerInterface
+     */
+    private $objectManager;
+
+    protected function setUp()
+    {
+        $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+        $this->model = $this->objectManager->create(\Magento\Framework\Lock\Backend\Database::class);
+    }
+
+    public function testLockAndUnlock()
+    {
+        $name = 'test_lock';
+
+        $this->assertFalse($this->model->isLocked($name));
+
+        $this->assertTrue($this->model->lock($name));
+        $this->assertTrue($this->model->isLocked($name));
+
+        $this->assertTrue($this->model->unlock($name));
+        $this->assertFalse($this->model->isLocked($name));
+    }
+
+    public function testUnlockWithoutExistingLock()
+    {
+        $name = 'test_lock';
+
+        $this->assertFalse($this->model->isLocked($name));
+        $this->assertFalse($this->model->unlock($name));
+    }
+}

lib/internal/Magento/Framework/Lock/Backend/Database.php

@@ -0,0 +1,158 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+namespace Magento\Framework\Lock\Backend;
+
+use Magento\Framework\App\DeploymentConfig;
+use Magento\Framework\App\ResourceConnection;
+use Magento\Framework\Config\ConfigOptionsListConstants;
+use Magento\Framework\Exception\AlreadyExistsException;
+use Magento\Framework\Exception\InputException;
+use Magento\Framework\Phrase;
+
+class Database implements \Magento\Framework\Lock\LockManagerInterface
+{
+    /** @var ResourceConnection */
+    private $resource;
+
+    /** @var DeploymentConfig */
+    private $deploymentConfig;
+
+    /** @var string Lock prefix */
+    private $prefix;
+
+    /** @var string|false Holds current lock name if set, otherwise false */
+    private $currentLock = false;
+
+    public function __construct(
+        ResourceConnection $resource,
+        DeploymentConfig $deploymentConfig,
+        string $prefix = null
+    ) {
+        $this->resource = $resource;
+        $this->deploymentConfig = $deploymentConfig;
+        $this->prefix = $prefix;
+    }
+
+    /**
+     * Sets a lock for name
+     *
+     * @param string $name lock name
+     * @param int $timeout How long to wait lock acquisition in seconds, negative value means infinite timeout
+     * @return bool
+     * @throws InputException
+     * @throws AlreadyExistsException
+     */
+    public function lock(string $name, int $timeout = -1): bool
+    {
+        $name = $this->addPrefix($name);
+
+        /**
+         * Before MySQL 5.7.5, only a single simultaneous lock per connection can be acquired.
+         * This limitation can be removed once MySQL minimum requirement has been raised,
+         * currently we support MySQL 5.6 way only.
+         */
+        if ($this->currentLock) {
+            throw new AlreadyExistsException(
+                new Phrase(
+                    'Current connection is already holding lock for $1, only single lock allowed',
+                    [$this->currentLock]
+                )
+            );
+        }
+
+        $result = (bool)$this->resource->getConnection()->query(
+            "SELECT GET_LOCK(?, ?);",
+            [(string)$name, (int)$timeout]
+        )->fetchColumn();
+
+        if ($result === true) {
+            $this->currentLock = $name;
+        }
+
+        return $result;
+    }
+
+    /**
+     * Releases a lock for name
+     *
+     * @param string $name lock name
+     * @return bool
+     * @throws InputException
+     */
+    public function unlock(string $name): bool
+    {
+        $name = $this->addPrefix($name);
+
+        $result = (bool)$this->resource->getConnection()->query(
+            "SELECT RELEASE_LOCK(?);",
+            [(string)$name]
+        )->fetchColumn();
+
+        if ($result === true) {
+            $this->currentLock = false;
+        }
+
+        return $result;
+    }
+
+    /**
+     * Tests of lock is set for name
+     *
+     * @param string $name lock name
+     * @return bool
+     * @throws InputException
+     */
+    public function isLocked(string $name): bool
+    {
+        $name = $this->addPrefix($name);
+
+        return (bool)$this->resource->getConnection()->query(
+            "SELECT IS_USED_LOCK(?);",
+            [(string)$name]
+        )->fetchColumn();
+    }
+
+    /**
+     * Adds prefix and checks for max length of lock name
+     *
+     * Limited to 64 characters in MySQL.
+     *
+     * @param string $name
+     * @return string $name
+     * @throws InputException
+     */
+    private function addPrefix(string $name): string
+    {
+        $name = $this->getPrefix() . '|' . $name;
+
+        if (strlen($name) > 64) {
+            throw new InputException(new Phrase('Lock name too long: %1...', [substr($name, 0, 64)]));
+        }
+
+        return $name;
+    }
+
+    /**
+     * Get installation specific lock prefix to avoid lock conflicts
+     *
+     * @return string lock prefix
+     */
+    private function getPrefix(): string
+    {
+        if ($this->prefix === null) {
+            $this->prefix = (string)$this->deploymentConfig->get(
+                ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTION_DEFAULT
+                . '/'
+                . ConfigOptionsListConstants::KEY_NAME,
+                ''
+            );
+        }
+
+        return $this->prefix;
+    }
+}

lib/internal/Magento/Framework/Lock/LockManagerInterface.php

@@ -0,0 +1,44 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+namespace Magento\Framework\Lock;
+
+/**
+ * Interface of a lock manager
+ *
+ * @api
+ */
+interface LockManagerInterface
+{
+    /**
+     * Sets a lock
+     *
+     * @param string $name lock name
+     * @param int $timeout How long to wait lock acquisition in seconds, negative value means infinite timeout
+     * @return bool
+     * @api
+     */
+    public function lock(string $name, int $timeout = -1): bool;
+
+    /**
+     * Releases a lock
+     *
+     * @param string $name lock name
+     * @return bool
+     * @api
+     */
+    public function unlock(string $name): bool;
+
+    /**
+     * Tests if lock is set
+     *
+     * @param string $name lock name
+     * @return bool
+     * @api
+     */
+    public function isLocked(string $name): bool;
+}

lib/internal/Magento/Framework/Lock/README.md

@@ -0,0 +1,8 @@
+# Lock library
+
+Lock library provides mechanism to acquire Magento system-wide lock. Default implementation is based on MySQL locks, where any locks are automatically released on connection close.
+
+The library provides interface *LockManagerInterface* which provides following methods:
+* *lock* - Acquires a named lock
+* *unlock* - Releases a named lock
+* *isLocked* - Tests if a named lock exists

lib/internal/Magento/Framework/Lock/Test/Unit/Backend/DatabaseTest.php

@@ -0,0 +1,93 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\Framework\Lock\Test\Unit\Backend;
+
+use Magento\Framework\Lock\Backend\Database;
+
+class DatabaseTest extends \PHPUnit\Framework\TestCase
+{
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\ResourceConnection
+     */
+    private $resource;
+
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\DB\Adapter\AdapterInterface
+     */
+    private $connection;
+
+    /**
+     * @var \PHPUnit_Framework_MockObject_MockObject|\Zend_Db_Statement_Interface
+     */
+    private $statement;
+
+    /**
+     * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
+     */
+    private $objectManager;
+
+    /** @var Database $database */
+    private $database;
+
+    protected function setUp()
+    {
+        $this->connection = $this->getMockBuilder(\Magento\Framework\DB\Adapter\AdapterInterface::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+        $this->resource = $this->getMockBuilder(\Magento\Framework\App\ResourceConnection::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->statement = $this->getMockBuilder(\Zend_Db_Statement_Interface::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $this->resource->expects($this->any())
+            ->method('getConnection')
+            ->willReturn($this->connection);
+
+        $this->connection->expects($this->any())
+            ->method('query')
+            ->willReturn($this->statement);
+
+        $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
+
+        /** @var Database $database */
+        $this->database = $this->objectManager->getObject(
+            Database::class,
+            ['resource' => $this->resource]
+        );
+    }
+
+    public function testLock()
+    {
+        $this->statement->expects($this->once())
+            ->method('fetchColumn')
+            ->willReturn(true);
+
+        $this->assertTrue($this->database->lock('testLock'));
+    }
+
+    /**
+     * @expectedException \Magento\Framework\Exception\InputException
+     */
+    public function testlockWithTooLongName()
+    {
+        $this->database->lock('BbXbyf9rIY5xuAVdviQJmh76FyoeeVHTDpcjmcImNtgpO4Hnz4xk76ZGEyYALvrQu');
+    }
+
+    /**
+     * @expectedException \Magento\Framework\Exception\AlreadyExistsException
+     */
+    public function testlockWithAlreadyAcquiredLockInSameSession()
+    {
+        $this->statement->expects($this->any())
+            ->method('fetchColumn')
+            ->willReturn(true);
+
+        $this->database->lock('testLock');
+        $this->database->lock('differentLock');
+    }
+}