PostgreSqlStore uses the InMemoryStore to avoid duplicate lock on the same connection.

Can you elaborate on this requirement?

Im concerned that using the InMemoryStore can cause unexpected behavior in situations where either:

1. A combined store is used with with Mysql and Postgres
2. A the same key is passed to two LockFactory::createLockFromKey, one using Mysql and the other using Postgres

I moved the state into the store since the tests assert that exists() is bound to the Key instance, while save() is bound to the Key::$resource. Does this work?

1. They should not share the same instance of InMemoryStore

spl_object_id is not reliable.

The above code should work

$key = new Key('foo');
$store->save($key);

$key = clone($key);
$this->assertTrue($store->exists($key));

This line triggers PDOException: SQLSTATE[HY000] [2002] No such file or directory on my local machine

Replaced the MYSQL_HOST with MYSQL_DSN envar so you can test using a socket dsn. I think that's what your error is related to.

That's wrong, the implementation must assert that the lock is still present in the backend. (Ie the database restart, the network is down, ...)

This wont be possible since GET_LOCK is bound to the connection so its always released when the connection is terminated.

Well.. getLockId is based on connection_id() which is (for MySQL 8 at least) an auto-increment that is reset when server starts.

This means, that each time the server restarts, the application might retrieve the connection_id previously affected to another process. Which makes the key used in self::$locksAcquired untrustable.

I think this should do it.

I cant add this test case to the since it requires rebooting mysql but I did run it manually to confirm proper handling even if the connection id is the same

class ManualMysqlStore extends MysqlStoreTest
{
    public function testMysqlClosedConnection()
    {
        // manually restart mysql to reset conn id counter
        $pdo = $this->getPdo();
        $storeA = new MysqlStore($pdo);
        $key = new Key('foo');
        $storeA->save($key);

        $connIdA = $pdo->query('SELECT CONNECTION_ID()')->fetchColumn();

        while (true) {
            try {
                $this->assertTrue($storeA->exists($key));
                sleep(1);
                // manually restart mysql to reset conn id counter
            } catch (\PDOException $e) {
                // mysql has gone away
                break;
            }
        }

        $pdo = $this->getPdo();
        $storeB = new MysqlStore($pdo);
        $connIdB = $pdo->query('SELECT CONNECTION_ID()')->fetchColumn();

        $this->assertFalse($storeB->exists($key));
        $this->assertSame($connIdA, $connIdB);
    }
}

The test is incorrect. You are testing both:

• 2 instances of Key
• 2 instances of store

You don't know why the storeB->save throws an exception.

For the record the below code is OK:

$resource = uniqid(__METHOD__, true);
$key1 = new Key($resource);
$key2 = new Key($resource);

$storeA->save($key1);
$storeb->save($key1);

Because stores are stateless, and you are allowed to use 2 instances of store to manipulate the same lock.

Because stores are stateless, and you are allowed to use 2 instances of store to manipulate the same lock.

makes sense I see where you're going now

1. They should not share the same instance of InMemoryStore

this prepares the statement each time the method is called, consider moving it to after the connection is established and capturing statement in an object property

IF statement is technically not needed, return the result of comparison

you would get the 1 when the lock is held, and null when it's not (safer to assume not 1 value)

agree

consider using execute with an argument for binding (the Key name has been already 'forced' to a string by letLockName) - it's easier to read without semantic drawbacks

agree - did the others too

some mysql peculates should be documented (and maybe this store should prevent instantiation on unsupported version or scenario):

https://dev.mysql.com/doc/refman/5.7/en/locking-functions.html

•   SELECT GET_LOCK('lock1',10);
    SELECT GET_LOCK('lock2',10);
    SELECT RELEASE_LOCK('lock2');
    SELECT RELEASE_LOCK('lock1');
  

  In MySQL 5.7 or later, the second [GET_LOCK()] (https://dev.mysql.com/doc/refman/5.7/en/locking-functions.html#function_get-lock) acquires a second lock and both RELEASE_LOCK() calls return 1 (success). Before MySQL 5.7, the second GET_LOCK() releases the first lock ('lock1') and the second RELEASE_LOCK() returns NULL (failure) because there is no 'lock1' to release.

• MySQL 5.7 and later enforces a maximum length on lock names of 64 characters. Previously, no limit was enforced.

• This function is [All lock functions are] unsafe for statement-based replication. A warning is logged if you use this function when binlog_format is set to STATEMENT.

please investigate the isolation of this lock acquire.

i'm not sure the IF statement is atomic nor serialized, so i've posted the question on sf https://stackoverflow.com/questions/71957583/is-mysql-if-statement-atomic

MySQL 5.7 and later enforces a maximum length on lock names of 64 characters.

Handled by getLockName

...unsafe for statement-based replication

After some digging, I don't believe this is relevant to the use case.

1. The locks are "locked on a server-wide basis." so i dont one would expect a replica to respect locks on the master.
2. The STATEMENT based replication warning is applicable if these various functions are used within DML (e.g. insert into foo values(is_used_lock('bar')) so a replica would get a different value when replicating that statement (since the lock situation on the replica is not the same as the master).

please investigate the isolation of this lock acquire.

These functions do not interact with transactions per "Locks obtained with GET_LOCK() are not released when transactions commit or roll back."

Heres a quick demo:

$pdoA = ...
$pdoB = ...

$dump = function (\PDO $pdo, string $sql) {
    printf("%s : %s\n",
         $sql,
        var_export($pdo->query($sql)->fetchColumn(), true)
    );
};

$dump($pdoA, "select get_lock('foo',0)");
$dump($pdoB, "select get_lock('foo',0)");
$dump($pdoA, "select release_lock('foo')");

$pdoA->beginTransaction();
$dump($pdoA, "select get_lock('foo',0)");
$dump($pdoB, "select get_lock('foo',0)");

select get_lock('foo',0) : 1
select get_lock('foo',0) : 0
select release_lock('foo') : 1
select get_lock('foo',0) : 1
select get_lock('foo',0) : 0

maybe rename it to MysqlLocksStore to indicate Mysql user locks are used (not to be confused with data storage in Pdo store

Hi, thanks for your work!, I have one feedback:
I think that is necessary to throw different type of exception if user already has LOCK, i sow the implementation in vendor/symfony/lock/Lock.php:69, is better to ignore it if the user/connection already has the lock, or i am wrong?

a Lock already acquired by this connection should not throw a LockConflictedException (btw, this is what your exists check tries to avoid also)

hashing only longer names will mean that there is 2 resources that share the same MySQL lock name

a Lock already acquired by this connection should not throw a LockConflictedException (btw, this is what your exists check tries to avoid also)