This should be changed to3.4

good catch@apfelbox

Since a lock is an unmanaged resource, there is an exeception safety issue right here. Despite this is not a bug in the lock component itself, you should promote exception safety in examples IMO.

It can be fixed with a simple try / finally

$lock =$factory->createLock('pdf-invoice-generation');if ($lock->acquire()) {try {// ...    }finally {$lock->release();    }}

You can even add an helper method to avoid this boilerplate

$lock =$factory->createLock('pdf-invoice-generation');$lock->with(function() {// ...});

With the following implementation

publicfunctionwith($callback){if (!$this->acquire()) {returnfalse;    }try {$callback();    }finally {$this->release();    }returntrue;}

You're right, this chapter was covered inBlocking Locks but it's better to promote it everywhere.

Based on my experience in my company, we've spent lot of time and headaches using distributed locks for use cases where more reliable tools are best suited.
I mean:

• for high concurrency use cases, atomic write operations are better when available.
• for low concurrency use cases, OCC is better when available.

For example, to avoid concurrent business transactions on the same object within a DBMS,Doctrine's OCC is the best solution.

So it would be fine to explain it right here IMO.

Indeed, a chapter about other altenatives would be relevant. I will add it when this PR will be merged given this PR had already been reviewed.

But in my opinion, it really depend of the use case:

for high concurrency use cases, atomic write operations are better when available.

True for concurrency between read and write, but wrong for concurrency between writes.

for low concurrency use cases, OCC is better when available.

True when computing the changset does not affect other systems:

• must be free (otherwise you pay twice to update the object) (ie. call to a billable API)
• must be quick (in a scalable environment, it's close to the previous point given you pay for the CPU time)
• other system can be rollbacked (ie. Don't call non-safe operation on third API, don't send email during the changset computation)

BTW, I am interested by what you said:we've spent lot of time and headaches. Can you give me exemples/use case and how this component can be improved to fix such issue?

True for concurrency between read and write, but wrong for concurrency between writes.

For both AFAIK. For example, when your business transaction simply consists in incrementing something in your data store, most DBMS (relational or not) does support this kind of atomic write. If several atomic writes are required, ACID-capable DBMS is required. This is applicable as long as the whole transaction can be executed on DBMS side.

True when computing the changset does not affect other systems:

Yes, not in all cases indeed, that's what I mean by saying "when available" (i.e. applicable).

BTW, I am interested by what you said: we've spent lot of time and headaches. Can you give me exemples/use case and how this component can be improved to fix such issue?

This component seems to be quite close from our own internal locking implementation. The main issue I've spotted (I may be wrong) is the one present herehttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/Lock/Store/CombinedStore.php#L88 as described heresymfony/symfony#22132 (review) (lack of onwership tracking to catch program flaws).

Issues can also arise at infrastructure level, and some hints may be added in the doc. For example, about the Redis setup, the user has to be aware of the HA vs consistency trade-off with this kind of backend. If consistency (lock must works as expected or not be working) is more important than availability (lock is always working, whatever its correctness) then a single Redis instance with maximum durability and without replication (even synchronous) should be preferred AFAIK.

Some other minor points:

• you provide a decorator which add blocking behavior to a non-blocking implementation, this looks great. But since this is implemented with a spinlock, the lack of fairness (no FIFO semantic) might be documented.
• I don't get what kind of problem solves CombinedStore. Some examples would be great.

Issues can also arise at infrastructure level, and some hints may be added in the doc

I'm working on an other PR to add a lot of tips/warning about reliability and weakness of each store.

I don't get what kind of problem solves CombinedStore

Combine several stores together to provide an implementation of the Redlock pattern and offer an highest availability.