Skip to content

Unnecessary refresh() while acquiring pessimistick lock with find() leads to loosing changes done to entity #12260

@Arkemlar

Description

@Arkemlar

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 fail

When I replace Section B with this - everything fine:

$entry = $em->find(ExampleEntity::class, $id);  // no refresh, using identityMap
assert($entry->someProperty === 2); // will succeed

In 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions