Skip to content

Commit

Permalink
Prevent UnitOfWork::commit() reentrance
Browse files Browse the repository at this point in the history
  • Loading branch information
@mpdude
mpdude committed Aug 9, 2023
1 parent 597a63a commit e281238
Showing 1 changed file with 127 additions and 109 deletions.
236 changes: 127 additions & 109 deletions lib/Doctrine/ORM/UnitOfWork.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
use Doctrine\Persistence\PropertyChangedListener;
use Exception;
use InvalidArgumentException;
use LogicException;
use RuntimeException;
use Throwable;
use UnexpectedValueException;
Expand Down Expand Up @@ -328,6 +329,13 @@ class UnitOfWork implements PropertyChangedListener
/** @var ReflectionPropertiesGetter */
private $reflectionPropertiesGetter;

/**
* Set to `true` while the UnitOfWork has entered the `commit()` method
*
* @var bool
*/
private $pendingCommit = false;

/**
* Initializes a new UnitOfWork instance, bound to the given EntityManager.
*/
Expand Down Expand Up @@ -363,146 +371,156 @@ public function __construct(EntityManagerInterface $em)
*/
public function commit($entity = null)
{
if ($entity !== null) {
Deprecation::triggerIfCalledFromOutside(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/8459',
'Calling %s() with any arguments to commit specific entities is deprecated and will not be supported in Doctrine ORM 3.0.',
__METHOD__
);
if ($this->pendingCommit) {
throw new LogicException('The UnitOfWork is currently in the commit phase. You must not call UnitOfWork::commit() or EntityManager::flush() at this time, since commit phases cannot safely be nested.');

Check warning on line 375 in lib/Doctrine/ORM/UnitOfWork.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/UnitOfWork.php#L375 

Added line #L375 was not covered by tests
}

$connection = $this->em->getConnection();
$this->pendingCommit = true;

if ($connection instanceof PrimaryReadReplicaConnection) {
$connection->ensureConnectedToPrimary();
}

// Raise preFlush
if ($this->evm->hasListeners(Events::preFlush)) {
$this->evm->dispatchEvent(Events::preFlush, new PreFlushEventArgs($this->em));
}

// Compute changes done since last commit.
if ($entity === null) {
$this->computeChangeSets();
} elseif (is_object($entity)) {
$this->computeSingleEntityChangeSet($entity);
} elseif (is_array($entity)) {
foreach ($entity as $object) {
$this->computeSingleEntityChangeSet($object);
}
}

if (
! ($this->entityInsertions ||
$this->entityDeletions ||
$this->entityUpdates ||
$this->collectionUpdates ||
$this->collectionDeletions ||
$this->orphanRemovals)
) {
$this->dispatchOnFlushEvent();
$this->dispatchPostFlushEvent();
try {
if ($entity !== null) {
Deprecation::triggerIfCalledFromOutside(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/8459',
'Calling %s() with any arguments to commit specific entities is deprecated and will not be supported in Doctrine ORM 3.0.',
__METHOD__
);
}

$this->postCommitCleanup($entity);
$connection = $this->em->getConnection();

return; // Nothing to do.
}
if ($connection instanceof PrimaryReadReplicaConnection) {
$connection->ensureConnectedToPrimary();

Check warning on line 393 in lib/Doctrine/ORM/UnitOfWork.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/UnitOfWork.php#L393 

Added line #L393 was not covered by tests
}

$this->assertThatThereAreNoUnintentionallyNonPersistedAssociations();
// Raise preFlush
if ($this->evm->hasListeners(Events::preFlush)) {
$this->evm->dispatchEvent(Events::preFlush, new PreFlushEventArgs($this->em));
}

if ($this->orphanRemovals) {
foreach ($this->orphanRemovals as $orphan) {
$this->remove($orphan);
// Compute changes done since last commit.
if ($entity === null) {
$this->computeChangeSets();
} elseif (is_object($entity)) {
$this->computeSingleEntityChangeSet($entity);
} elseif (is_array($entity)) {
foreach ($entity as $object) {
$this->computeSingleEntityChangeSet($object);
}
}
}

$this->dispatchOnFlushEvent();
if (
!($this->entityInsertions

Check failure on line 413 in lib/Doctrine/ORM/UnitOfWork.php

View workflow job for this annotation

GitHub Actions / coding-standards / Coding Standards (8.2) 

Expected 1 space(s) after NOT operator; 0 found
|| $this->entityDeletions
|| $this->entityUpdates
|| $this->collectionUpdates
|| $this->collectionDeletions
|| $this->orphanRemovals)
) {
$this->dispatchOnFlushEvent();
$this->dispatchPostFlushEvent();

$conn = $this->em->getConnection();
$conn->beginTransaction();
$this->postCommitCleanup($entity);

try {
// Collection deletions (deletions of complete collections)
foreach ($this->collectionDeletions as $collectionToDelete) {
// Deferred explicit tracked collections can be removed only when owning relation was persisted
$owner = $collectionToDelete->getOwner();
return; // Nothing to do.
}

if ($this->em->getClassMetadata(get_class($owner))->isChangeTrackingDeferredImplicit() || $this->isScheduledForDirtyCheck($owner)) {
$this->getCollectionPersister($collectionToDelete->getMapping())->delete($collectionToDelete);
$this->assertThatThereAreNoUnintentionallyNonPersistedAssociations();

if ($this->orphanRemovals) {
foreach ($this->orphanRemovals as $orphan) {
$this->remove($orphan);
}
}

if ($this->entityInsertions) {
// Perform entity insertions first, so that all new entities have their rows in the database
// and can be referred to by foreign keys. The commit order only needs to take new entities
// into account (new entities referring to other new entities), since all other types (entities
// with updates or scheduled deletions) are currently not a problem, since they are already
// in the database.
$this->executeInserts();
}
$this->dispatchOnFlushEvent();

if ($this->entityUpdates) {
// Updates do not need to follow a particular order
$this->executeUpdates();
}
$conn = $this->em->getConnection();
$conn->beginTransaction();

// Extra updates that were requested by persisters.
// This may include foreign keys that could not be set when an entity was inserted,
// which may happen in the case of circular foreign key relationships.
if ($this->extraUpdates) {
$this->executeExtraUpdates();
}
try {
// Collection deletions (deletions of complete collections)
foreach ($this->collectionDeletions as $collectionToDelete) {
// Deferred explicit tracked collections can be removed only when owning relation was persisted
$owner = $collectionToDelete->getOwner();

// Collection updates (deleteRows, updateRows, insertRows)
// No particular order is necessary, since all entities themselves are already
// in the database
foreach ($this->collectionUpdates as $collectionToUpdate) {
$this->getCollectionPersister($collectionToUpdate->getMapping())->update($collectionToUpdate);
}
if ($this->em->getClassMetadata(get_class($owner))->isChangeTrackingDeferredImplicit() || $this->isScheduledForDirtyCheck($owner)) {
$this->getCollectionPersister($collectionToDelete->getMapping())->delete($collectionToDelete);
}
}

// Entity deletions come last. Their order only needs to take care of other deletions
// (first delete entities depending upon others, before deleting depended-upon entities).
if ($this->entityDeletions) {
$this->executeDeletions();
}
if ($this->entityInsertions) {
// Perform entity insertions first, so that all new entities have their rows in the database
// and can be referred to by foreign keys. The commit order only needs to take new entities
// into account (new entities referring to other new entities), since all other types (entities
// with updates or scheduled deletions) are currently not a problem, since they are already
// in the database.
$this->executeInserts();
}

// Commit failed silently
if ($conn->commit() === false) {
$object = is_object($entity) ? $entity : null;
if ($this->entityUpdates) {
// Updates do not need to follow a particular order
$this->executeUpdates();
}

throw new OptimisticLockException('Commit failed', $object);
}
} catch (Throwable $e) {
$this->em->close();
// Extra updates that were requested by persisters.
// This may include foreign keys that could not be set when an entity was inserted,
// which may happen in the case of circular foreign key relationships.
if ($this->extraUpdates) {
$this->executeExtraUpdates();
}

if ($conn->isTransactionActive()) {
$conn->rollBack();
}
// Collection updates (deleteRows, updateRows, insertRows)
// No particular order is necessary, since all entities themselves are already
// in the database
foreach ($this->collectionUpdates as $collectionToUpdate) {
$this->getCollectionPersister($collectionToUpdate->getMapping())->update($collectionToUpdate);
}

$this->afterTransactionRolledBack();
// Entity deletions come last. Their order only needs to take care of other deletions
// (first delete entities depending upon others, before deleting depended-upon entities).
if ($this->entityDeletions) {
$this->executeDeletions();
}

throw $e;
}
// Commit failed silently
if ($conn->commit() === false) {
$object = is_object($entity) ? $entity : null;

$this->afterTransactionComplete();
throw new OptimisticLockException('Commit failed', $object);
}
} catch (Throwable $e) {
$this->em->close();

// Unset removed entities from collections, and take new snapshots from
// all visited collections.
foreach ($this->visitedCollections as $coid => $coll) {
if (isset($this->pendingCollectionElementRemovals[$coid])) {
foreach ($this->pendingCollectionElementRemovals[$coid] as $key => $valueIgnored) {
unset($coll[$key]);
if ($conn->isTransactionActive()) {
$conn->rollBack();
}

$this->afterTransactionRolledBack();

throw $e;
}

$coll->takeSnapshot();
}
$this->afterTransactionComplete();

// Unset removed entities from collections, and take new snapshots from
// all visited collections.
foreach ($this->visitedCollections as $coid => $coll) {
if (isset($this->pendingCollectionElementRemovals[$coid])) {
foreach ($this->pendingCollectionElementRemovals[$coid] as $key => $valueIgnored) {
unset($coll[$key]);
}
}

$this->dispatchPostFlushEvent();
$coll->takeSnapshot();
}

$this->postCommitCleanup($entity);
$this->dispatchPostFlushEvent();

$this->postCommitCleanup($entity);
} finally {
$this->pendingCommit = false;
}
}

/** @param object|object[]|null $entity */
Expand Down

0 comments on commit e281238

Please sign in to comment.