-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Description
Bug Report
| Q | A |
|---|---|
| Version | 3.5.3 |
Summary
Every time a pessimistic lock is requested via find(), Doctrine queries the database, and if the object is already hydrated, it updates it using refresh().
Now imagine we have two independent sections of code that require a similar lock. Here's what happens:
- In section A, we obtained an object with a lock via find(); the object was obtained for the first time and was hydrated.
- In the same section A, we modified the object.
- Then we moved to section B and also obtained it with a lock via find()... which, due to refresh(), will overwrite the changes made to the object!
And there's no way to check whether a pessimistic lock was previously obtained on the entity instance.
Current behavior
Repeated calls to find($id, LockMode::PESSIMISTIC_WRITE) result in the loss of object state due to overwriting in https://github.com/doctrine/orm/blob/3.5.x/src/EntityManager.php#L342
Expected behavior
Preserve the lock state and do not call refresh() when the object has already acquired one.
If the object is already hydrated, but the lock is acquired for the first time, it is correct to call refresh() and reset its state; in such cases, all business logic should be executed after acquiring the lock.
How to reproduce
Prerequisites:
class ExampleEntity
{
// mapping skipped
public int $id;
public int $someProperty;
}
// ...populating database with ExampleEntity...Problem demonstration:
$em->beginTransaction();
// Section A
$entry = $em->find(ExampleEntity::class, $id, LockMode::PESSIMISTIC_WRITE);
$entry->someProperty = 2;
// No flush! Transaction remains open and is same.
// Section B
$entry = $em->find(ExampleEntity::class, $id, LockMode::PESSIMISTIC_WRITE);
assert($entry->someProperty === 2); // will failWhen I replace Section B with this - everything fine:
$entry = $em->find(ExampleEntity::class, $id); // no refresh, using identityMap
assert($entry->someProperty === 2); // will succeedIn the last case, we see that the behavior of the find() function matches what most of us expect, as we know it uses identityMap to avoid unnecessary queries and reuses the state of the managed object. The behavior demonstrated in Section B violates this contract.