Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EntityManager find with pessimistic lock does not check for transaction when cached #7068

Closed
madwizard-thomas opened this issue Feb 15, 2018 · 4 comments
Assignees
Labels
Milestone

Comments

@madwizard-thomas
Copy link

When a pessimistic lock mode is passed to EntityManager::find it normally throws a TransactionRequiredException if no transaction is currently active.

However if the entity is currently in the identity map, it will refresh the entity from the database and return it but not check if a transaction is running. The refresh query has 'FOR UPDATE' (in case of pessimistic write) but this has no effect since there is no transaction.

To reproduce:

// $em is EntityManager, Order entity can be any entity
$em->find(Order::class, 1); // Load order id 1 in entity map
$em->find(Order::class, 1, LockMode::PESSIMISTIC_WRITE); // does not throw exception
$em->find(Order::class, 2, LockMode::PESSIMISTIC_WRITE); // throws TransactionRequiredException 

Tested with doctrine 2.5.14 but latest code seems to have the same issue.

@madwizard-thomas madwizard-thomas changed the title EntityManager find with pessmistic lock does not check for transaction when cached EntityManager find with pessimistic lock does not check for transaction when cached Feb 15, 2018
@Ocramius
Copy link
Member

Ocramius commented Feb 15, 2018 via email

@madwizard-thomas
Copy link
Author

I don't think so, but have only tested it with MySQL. It seems to be caused by the early return statement here before the transaction check here

BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
Contributing guidelines want surrounding spaces before and after the
exclamation mark but that would make the class code-style inconsistent
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
@BreiteSeite
Copy link
Contributor

Thanks @madwizard-thomas for the minimal code to reproduce. Helped a lot.

@Ocramius i added this as a test for the test-suite and also provided a fix in #7075

BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
Contributing guidelines want surrounding spaces before and after the
exclamation mark but that would make the class code-style inconsistent
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
Contributing guidelines want surrounding spaces before and after the
exclamation mark but that would make the class code-style inconsistent
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Feb 17, 2018
BreiteSeite pushed a commit to BreiteSeite/doctrine2 that referenced this issue Apr 15, 2018
Majkl578 pushed a commit to Majkl578/doctrine-orm that referenced this issue Jul 3, 2018
Majkl578 pushed a commit to Majkl578/doctrine-orm that referenced this issue Jul 3, 2018
Majkl578 pushed a commit to BreiteSeite/doctrine2 that referenced this issue Jul 3, 2018
Ocramius added a commit that referenced this issue Jul 3, 2018
[2.6] Fix for #7068: EntityManager::find() with pessimistic lock should check for transaction
@Ocramius Ocramius added the Bug label Jul 3, 2018
@Ocramius Ocramius added this to the 2.6.2 milestone Jul 3, 2018
@Ocramius Ocramius self-assigned this Jul 3, 2018
Ocramius added a commit that referenced this issue Jul 3, 2018
…d-with-pessimistic-lock-does-not-check-for-transaction

Fix for #7068: `EntityManager::find()` with pessimistic lock should check for transaction
@Ocramius
Copy link
Member

Ocramius commented Jul 3, 2018

Handled in #7075 and #7291

@Ocramius Ocramius closed this as completed Jul 3, 2018
maglnet pushed a commit to maglnet/doctrine2 that referenced this issue Oct 10, 2018
guilhermeblanco pushed a commit that referenced this issue May 30, 2019
guilhermeblanco pushed a commit that referenced this issue Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants