Currently if two Symfony projects are hosted on the same server and used the same key for a lock using the defaultFlockStore, the two applications shared the same lock.

So I suggest adding a prefix in theFlockStore constructor to isolate the lock to the current project and avoid collision.

This prefix could be initialized with thekernel.secret parameter by default.

To avoid BC, I add theflock-exclusive lock key configuration.

I know this could be done by changing the path a theFlockStore, but this "exclusive lock" could be the default behaviour of Symfony in the next major release.

jdecool requested a review fromjderusse as acode owner

July 28, 2024 12:32

carsonbot added Status: Needs Review Deprecation Feature Lock labels

Jul 28, 2024

carsonbot added this to the7.2 milestone

Jul 28, 2024

[Lock] Add prefix argument to avoid collision with FlockStore

ec32409

jdecool force-pushed thelock-flock-collision branch from3ed7270 toec32409Compare

July 28, 2024 12:38

symfony deleted a comment fromcarsonbot

Jul 28, 2024

OskarStark changed the title~~[Lock] Add prefix argument to avoid collision with FlockStore~~[Lock] Add$prefix parameter to avoid collision withFlockStore

Aug 2, 2024

OskarStark reviewed

Aug 2, 2024

View reviewed changes

src/Symfony/Component/Lock/CHANGELOG.md

		7.2
		---

		* Add parameter `$prefix` to `FlockStore::__construct()`

Copy link

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change

	* Addparameter`$prefix` to`FlockStore::__construct()`
	* Add`$prefix` parameter to`FlockStore::__construct()`

src/Symfony/Component/Lock/Store/FlockStore.php

		*
		* @throws LockStorageException If the lock directory doesn’t exist or is not writable
		*/
		public function __construct(?string $lockPath = null)
		public function __construct(?string $lockPath = null, ?string $prefix = null)

Copy link

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change

	publicfunction __construct(?string$lockPath =null, ?string$prefix =null)
	publicfunction __construct(
	?string$lockPath =null,
	private ?string$prefix =null,
	)

We can use CPP (constructor property promotion here)

Copy link

Member

jderusse commentedAug 2, 2024

Didn't the$lockPath parameter solve the issue?

OskarStark added Status: Waiting feedback and removed Status: Needs Review labels

Aug 2, 2024

Copy link

ContributorAuthor

jdecool commentedAug 2, 2024

Didn't the $lockPath parameter solve the issue?

It does.

But by default, everything is written in the temp directory and if two Symfony projects use the default configuration they will share the lock.

The purpose of this PR is to provide a way to insulate by default the lock per Symfony instance (using theAPP_SECRET).

Copy link

ContributorAuthor

jdecool commentedSep 29, 2024

What about this PR ?

The idea is to allow multiple Symfony project to useflock LockStore on the same server without being block each other by default.

Copy link

Member

nicolas-grekas commentedOct 9, 2024

I would do it diffently: since by default we have a shared lock space, we should still do that, or we might break apps.
Then, for apps that need split lock spaces, each could define a dedicated $lockPath. We might make this easy by adding a namespace or prefix config option.
But since $lockPath already provides a way to do this split, we should leverage it. And I would definitely not use the secret for that (and I wouldn't split by default).