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

Can you elaborate on this requirement?

Im concerned that using theInMemoryStore 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 twoLockFactory::createLockFromKey, one using Mysql and the other using Postgres

I moved the state into the store since the tests assert thatexists() is bound to theKey instance, whilesave() is bound to theKey::$resource. Does this work?

1. They should not share the same instance ofInMemoryStore

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 triggersPDOException: 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 onconnection_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 theconnection_id previously affected to another process. Which makes the key used inself::$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 thestoreB->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 ofInMemoryStore

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 the1 when the lock is held, andnull when it's not (safer to assume not1 value)

agree

consider usingexecute with an argument for binding (theKey name has been already 'forced' to a string byletLockName) - 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 bothRELEASE_LOCK() calls return 1 (success). Before MySQL 5.7, the secondGET_LOCK() releases the first lock ('lock1') and the secondRELEASE_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 whenbinlog_format is set to STATEMENT.

please investigate the isolation of this lock acquire.

i'm not sure theIF statement is atomic nor serialized, so i've posted the question on sfhttps://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 bygetLockName

...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. TheSTATEMENT 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) : 1select get_lock('foo',0) : 0select release_lock('foo') : 1select get_lock('foo',0) : 1select get_lock('foo',0) : 0

maybe rename it toMysqlLocksStore 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 yourexists 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 yourexists check tries to avoid also)