Make the EntityManager resettable

[stof]

In Doctrine ORM 2.x, when an exception occurs during a flush, the EntityManager gets closed, because its unit of work is in a broken state.
But once it is closed, the object becomes totally unusable. This makes the EntityManager really hard to use in a context where you rely on dependency injection, because replacing the EntityManager with a new (non closed) instance would also mean replacing all the objects depending on it in the object graph (transitively).
This may not be a big issue for normal HTTP request handling (where each request is a separate process and the exception closing the EM tends to mark the end of the request), but it is critical for usage in a long-running process (for instance a RabbitMQ consumer).

The way we are dealing with it in 2.x is to never inject the EM directly, but to inject the ManagerRegistry instead, and to always get the EM from it (and never storing it in a property as it would reproduce the issue). We have the same restriction for entity repositories.
This allows replacing the entity manager (because there is a single object depending on it directly in the graph, which is the ManagerRegistry, and this class is designed to be able to reset the EM in it without replacing the registry itself). But it makes the architecture very fragile (it is easy to leak a place using the EM directly).

Btw, the AbstractManagerRegistry shipped in Doctrine Common lets implementation reset the manager (as the knowledge about retrieving the EM instance is part of the implementation work too). And the way this is implemented in Symfony today relies on being able to reset services in the DIC as if they were never instantiated, which is something we want to deprecated in Symfony 3.2. So we are currently looking at wrapping the EM in a proxy object to make it resettable by deinitializing the proxy (see symfony/symfony#19203 for the implementation). The drawback is that this forces to add overhead on calls to the EntityManager class. The advantage is that it solves the issue for object graph depending on the EM (but not for objects instantiated by Doctrine itself, like the entity repositories, because passing $this to the new instance does not play well with decorators) and that it avoids using our deprecated APIs.

So it would be really great that the EntityManager (actually the UnitOfWork IIRC) could be reset to reopen it (the unit of work would get cleared).

[stof]

this is something to be thought about for Doctrine 3

[Ocramius]

LGTM! 👍

[Ocramius]

@stof is there any actual need for the manager registry, btw? It seems like an abomination, at the moment...

[guilhermeblanco]

@stof last time I checked there were only one broken test if you completely swap UnitOfWork with a brand new instance as part of $em->clear();. We might take a look at it for D3, for sure! =)

[stof]

The manager registry is about 2 purposes:

• making it possible to support multiple entity managers in the same project easily, by having an easy way to get the needed entity manager among several ones (this is what allows the Symfony entity form type to support multiple entity managers by allowing to specify a manager name in an option instead of forcing to inject the entity manager instance in the higher form type when using a non-default one).
• allowing to workaround this issue by not depending on the EntityManager directly

Once the EM is resettable, the only use case becomes the support of multi-EM projects. I'm not sure how common this is (but I know it is used, according to the support I provided, even though I don't know whether it is the right solution for these projects)

[trickeyone]

I can say that for one of the projects at work, this is something that we really need. I've had to introduce a wrapped EntityManager that checks to see if the EntityManager is closed. If it is, then it creates new instance. Only detractor is that every single method for EntityManagerInterface has to have a call out to check to see if the EntityManager is closed before performing any of the operations.

Definitely looking forward to this being implemented in a future release!

[loiclavoie]

Is there any update on the status of this issue? @stof How did you solved/workaround it in your deamon?

[Majkl578]

Is there any update on the status of this issue?

Nothing so far, but it's scheduled for ORM 3.0.

[stof]

@loiclavoie until now, my workaround was to always inject he registry in my objects.
Now that I'm using Symfony 3.3, I can rely on the fact that Symfony itself wraps the EntityManager into a proxy to make it resettable (when you inject the EntityManager, it actually injects the proxy, and so resetting it can drop the internal instance and put the proxy back into uninitialized state).

But anyway, this issue is quite hard to solve in Doctrine 2.x, which is why it is scheduled for 3.x (I haven't tried to check whether the 2.x architecture would allow us to make it resettable in a BC way)

[ahonnecke]

@trickeyone Would you be willing to share the code that you're using to wrap and restart the entity manger? I can't seem to get it to reopen properly after the flush fails.

[stof]

See https://github.com/symfony/symfony/blob/v3.3.9/src/Symfony/Bridge/Doctrine/ManagerRegistry.php, and then the implementation of lazy services based on ProxyManager

[Majkl578]

Update: develop (aka 3.0) already switched to creating new instance of UoW during clear(): d1b453d

[lisachenko]

This ORM Has Performed an Illegal Operation and Will Be Shut Down.

[trickeyone]

@ahonnecke It required overriding the MetadataFactory, ProxyFactory, and UnitOfWork. The main issue was that the UnitOfWork tracks all the retrieved and saved entities. When it is closed and invalidated, it can no longer be used, which is what makes the EntityManager no longer viable. I had to create a new UnitOfWork & ProxyFactory and overriding them on the new EntitiyManager instance. I also then restored the identityMap on the new UnitOfWork that was cached from the old instance.

It was a lot of work that could have, conceivably, been taken care of by re-retrieving the instances, but there were a lot of places that it wasn't feasible because of relational entities.

[Ocramius]

3.x includes this by replacing the UoW instance altogether

…

On 7 Feb 2018 01:07, "Josh England" ***@***.***> wrote: @ahonnecke <https://github.com/ahonnecke> It required overriding the MetadataFactory, ProxyFactory, and UnitOfWork. The main issue was that the UnitOfWork tracks all the retrieved and saved entities. When it is closed and invalidated, it can no longer be used, which is what makes the EntityManager no longer viable. I had to create a new UnitOfWork & ProxyFactory and overriding them on the new EntitiyManager instance. I also then restored the identityMap on the new UnitOfWork that was cached from the old instance. It was a lot of work that could have, conceivably, been taken care of by re-retrieving the instances, but there were a lot of places that it wasn't feasible because of relational entities. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5933 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakDkf9K23nX9PEXeKXUcCfvZWWdeQks5tSOlEgaJpZM4JLuU4> .

[stof]

restoring the identityMap of the UoW is precisely something which should not be done, as it would bring the broken objects into the new UoW, making it as unreliable than the old one.

[bentcoder]

Was this issue still meant to be open? I am asking because injecting EM into Repo/Any service was not a good idea previously. Is it still the case or not anymore? I mean is example below viable for Symfony 4+?

class UserRepository
{
    private $entityManager;
    private $objectRepository;

    public function __construct(EntityManagerInterface $entityManager)
    {
        $this->entityManager = $entityManager;
        $this->objectRepository = $this->entityManager->getRepository(User::class);
    }

    public function fin(): array
    {
        return $this->objectRepository
            ->createQueryBuilder('u')
            ->getQuery()
            ->getResult();
    }

    public function per(User $user): void
    {
        $this->entityManager->persist($user);
        $this->entityManager->flush();
    }
}

class UserService
{
    private $userRepository;

    public function __construct(UserRepository $userRepository)
    {
        $this->userRepository = $userRepository;
    }

    public function method()
    {
        // $this->userRepository->fin();
        // $this->userRepository->per(new User());
    }
}

[yaroslavbr]

@stof, could you please help regarding resetting EntityManager.

I am using Symfony 4.2 and Doctrine 2.6. I have my custom EntityManager which decorates default one and it is marked as lazy. So actually is a Proxy. When my EntityManager closes (as well as closing decorating one) I am resetting it. The problem is that the new Proxy is instantiated but with an old instance of default EntityManager which remains closed. During debugging compiled container I see there is a condition which takes the old one EM stored in the property otherwise it gets recreated if not exists.
$instance = new \Namespace\CustomEntityManager(($this->privates['Namespace\CustomEntityManager.inner'] ?? $this->getCustomEntityManager_InnerService()));

Is it expected behavior that closed EntityManager is injected in Proxy? Is it possible to enforce a new instance of default service is instantiated?

[stof]

@yaroslavbr DoctrineBundle relies on a "hack" to make the Doctrine ORM 2.x EntityManager resettable, by wrapping it in a proxy and resetting the proxy inner element.
But that will indeed not work if your own code decorates the entity manager, as the service will not be the proxy handled by DoctrineBundle. There is no solution for that, until we will have proper support for resetting in Doctrine itself (and won't need such hack anymore)

[yaroslavbr]

@stof, got you. Thanks for such a quick reply!

[oojacoboo]

What's the latest on this issue? This is a constant problem for us. We're using the Registry in all of our services, which allows for us to update the EntityManager with a new EntityManager, after a previous instance has been closed. However, there doesn't seem to be any way to deal with existing managed entities.

What is the hack/solution for all of this? Are there any docs? There are a lot of cases where you need to continue writing to the database after an exception and unwinding.

[stof]

There's no way to keep existing managed entities. When Doctrine closes the entity manager, it is because it cannot trust anymore the state of the unit of work.

This feature request about making it resettable without hack still involves clearing the unit of work to get rid of this untrusted state.

[VincentLanglet]

There's no way to keep existing managed entities. When Doctrine closes the entity manager, it is because it cannot trust anymore the state of the unit of work.

So currently, a code like

$project = $this->em->find($id);

// dothings with $project

$url = new Url('foo);
try {
    $this->em->persist($url);
    $this->em->flush();
} catch (\Exception $e) {
    $url = null;
    $this->registry->resetManager();
}

// do other things with $em and $project <= ERROR HERE

is not possible without any workaround ? (getting Entity App\Entity\Project@42 is not managed.)

It's not an easy task to know which entities were managed by the EntityManager in order to re-attached them to the new EntityManager...

[Ocramius]

No, on a closed entity manager, you need to get rid of EVERYTHING.

Your $project there could al ready be in inconsistent state.

[stof]

@VincentLanglet You could do $project = $this->em->find($id); again after your try/catch. If the EM was not reset in the meantime, find() will not hit the DB (as that id is already in the unit of work). But if you reset the entity manager, this will fetch a new Project object for a clean start.

Alternatively, you should organize your code so that it does not try to keep using the entity manager after an exception happened during flushing.

[stof]

Also, even if $project is not in an inconsistent state, $url would still be in the unit of work if it were not cleared, and so the next flush would likely fail again

[greg0ire]

Update: it does not look like d1b453d, or rather, d05e8e8 was backported to 3.0.x after I renamed develop to old-prototype-3.x

3.x includes this by replacing the UoW instance altogether

I don't know if @Ocramius was referring to d05e8e8 or something else here, but since the EntityManager::$unitOfWork is readonly, I suppose it didn't land on 3.0.x either.

The next steps for anybody willing to tackle this could be:

• attempt to cherry-pick d05e8e8 on 4.0.x;
• see if it still addresses the issue;
• see if it is still a breaking change (probably true).

[greg0ire]

I tried porting the commit in #11477, but now there are errors I don't understand. Maybe somebody can take a look?