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

[DI] Broken entity manager service #30091

Closed
ro0NL opened this issue Feb 6, 2019 · 46 comments
Closed

[DI] Broken entity manager service #30091

ro0NL opened this issue Feb 6, 2019 · 46 comments

Comments

@ro0NL
Copy link
Contributor

ro0NL commented Feb 6, 2019

Symfony version(s) affected: 4.2.3

Updating symfony/dependency-injection (v4.2.2 => v4.2.3)

Somehow there are now 2 entity manager instances in my application, causing all sort of WTFs :)

See https://github.com/ro0NL/symfony-issue-30091

Before:

DefaultController.php on line 13:
EntityAwareFactory {#278 ▼
...
  -em: EntityManager {#233 …11}
}
DefaultController.php on line 13:
UserEmailRepository {#212 ▼
...
  -em: EntityManager {#233 …11}
...
}

Both entity managers are instance 233

After

DefaultController.php on line 13:
EntityAwareFactory {#278 ▼
...
  -em: EntityManager {#233 …11}
}
DefaultController.php on line 13:
UserEmailRepository {#298 ▼
...
  -em: EntityManager {#283 …11}
...
}

Now the instances differ. The only differenence is sf/di 4.2.2 vs 4.2.3. Respectively the first and last commit.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 7, 2019

Note that this is not a BC break, it's just a bug :)

@ro0NL ro0NL changed the title [DI] BC-Break entity manager service [DI] Broken entity manager service Feb 7, 2019
@jdelaune
Copy link
Contributor

jdelaune commented Feb 8, 2019

A lot of WTF indeed...

@virtualize
Copy link

Symfony 4.2.4 was just released and I can confirm the this horrible bug still exists. Getting two different EntityManagers when using constructor injection in two different services (in my case a controller and an event listener).
Downgrading to symfony/dependency-injection": "4.2.2" solves it temporarily

I wonder why not more people report here, I guess this breaks a lot of applications...

@ovgray
Copy link
Contributor

ovgray commented Mar 8, 2019

I seem to be suffering from the same bug on a project using Symfony 4.2.4.

In my case '@doctrine.orm.entity_manager' is injected into two services. For this comment I will call them Service A and Service B. Service A is injected into Service B. Within Service B, Service A is used to create, persist and return an entity without also flushing. Within Service A, just before it returns the entity, $this->entityManager->contains($entity) returns true, and spl_object_hash($this->entityManager) returns "0000000015d8e5160000000059204dbc".

Within Service B, after Service A returns $entity, $this->entityManager->contains($entity) returns false, and spl_object_hash($this->entityManager) returns "0000000015d8e61d0000000059204dbc", which differs by one character from the hash of the entity manager instance in Service A.

Edit:
I should add that no autowiring is involved. Both services are expressly configured in Resources/config/services.yml of a CommonBundle that I use in more than one project.

Edit:
However, I have not been able to create a demonstration project that shows this behaviour, so there may be something peculiar to the project described above that I have not yet been able to track down.

@ampaze
Copy link
Contributor

ampaze commented Mar 18, 2019

Any progress on this one? The bugs this causes are very difficult to relate to this issue, I guess thats why it is so silent here.

@ampaze
Copy link
Contributor

ampaze commented Mar 26, 2019

Downgrading to symfony/dependency-injection": "4.2.2" solves it temporarily

This does not seem to be the case for me. I was using symfony/symfony at 4.2.2 which solved this for me, using the symfony/* packages individually at 4.2.4 and only symfony/dependency-injection at 4.2.2 reintroduced the problem.

@fabienlem
Copy link

fabienlem commented Apr 8, 2019

Hello there,

I still experiment strange behaviours. I'm on symfony 3.4.24. I tried to downgrade DI component to 3.4.22 and 3.4.21 without any success. Is there another workaround ?

Thank you,

Fabien

@kaznovac
Copy link
Contributor

kaznovac commented Apr 8, 2019

seems like its introduced in #30046

@wiseguy1394
Copy link
Contributor

Error still exists for me in dependency-injection 4.2.6.
Lock the version to 4.2.2 solves the error temporarily.

@stof
Copy link
Member

stof commented Apr 18, 2019

Please provide us some clear reproducing cases. We have 2 conflicting reports here. @wiseguy1394 says that locking to 4.2.2 solves the issue, while @ampaze says that locking at 4.2.2 has the issue. This won't help us fix anything (and it might mean that you even have 2 different issues).

@nicolas-grekas
Copy link
Member

I have reproducer provided here and here, but they all involve complex applications and I didn't manage to reduce the case to a minimal service graph. All involve the Doctrine Configuration service... I need to find some time to fully concentrate on the topic to understand what's going on. It's non trivial :)

@kingchills
Copy link

Here is a case that is reproduce-able:

  1. Create project using composer composer create-project symfony/website-skeleton diTest
  2. Update composer.json to lock symfony/dependency-injection to 4.2.2 and run composer update
  3. Create 2 entities, one with at Many to one relationship to the other.
  4. Use maker to create CRUD for both entities
  5. Create a service and inject the EM in the constructor, this service should create a clone of the entity and persist and flush.
  6. Create a Doctrine Event Subscriber, inject your created service.
  7. Use the created cruds to create one of each entity, child then parent.
  8. Observe that three entities are created without issue.
  9. Update composer.json to lock DI package to 4.2.3
  10. Create single parent parent entity
  11. Observe ORMInvalidArgumentException::newEntitiesFoundThroughRelationships error.

Notes: Using step debugging I was able to set a breakpoint in the constructor of the service, and record the spl_object_id() of the EM being injected. And I set a second breakpoint in the controller, and recorded a second, different ID for the EM.

I have committed my test project for this at https://github.com/kingchills/di-test

@ampaze
Copy link
Contributor

ampaze commented Apr 23, 2019

Please provide us some clear reproducing cases. We have 2 conflicting reports here. @wiseguy1394 says that locking to 4.2.2 solves the issue, while @ampaze says that locking at 4.2.2 has the issue. This won't help us fix anything (and it might mean that you even have 2 different issues).

I think I have to revoke my report, don't have time to reinvestigate.

@jdelaune
Copy link
Contributor

Locking at 4.2.2 solves this issue for me

@benr77
Copy link
Contributor

benr77 commented Apr 24, 2019

I have also found that locking symfony/dependency-injection at 4.2.2 solves the issue.

@Ioni14
Copy link
Contributor

Ioni14 commented Apr 30, 2019

Hello,
I currently have this issue with symfony/dependency-injection v3.4.21 but not in v3.4.18. (I can't test v3.4.19 and v3.4.20)

I have a service BeneficiaryManager which depends on EntityManagerInterface that I inject in services.yaml.

I investigated and I found a condition that has been added in the build container since v3.4.21:

// in var/cache/test/ContainerUcir7yw/srcTestDebugProjectContainer.php
// ...
    /**
     * Gets the private 'App\Client\Manager\BeneficiaryManager' shared autowired service.
     *
     * @return \App\Client\Manager\BeneficiaryManager
     */
    protected function getBeneficiaryManagerService()
    {
        $a = ${($_ = isset($this->services['doctrine.orm.client_entity_manager']) ? $this->services['doctrine.orm.client_entity_manager'] : $this->getDoctrine_Orm_ClientEntityManagerService()) && false ?: '_'};

        if (isset($this->services['App\\Client\\Manager\\BeneficiaryManager'])) {
            return $this->services['App\\Client\\Manager\\BeneficiaryManager'];
        }

        return $this->services['App\\Client\\Manager\\BeneficiaryManager'] = new \App\Client\Manager\BeneficiaryManager($a, ${($_ = isset($this->services['property_accessor']) ? $this->services['property_accessor'] : $this->getPropertyAccessorService()) && false ?: '_'});
    }
// ...

Here the v3.4.18 version (it works):

    /**
     * Gets the private 'App\Client\Manager\BeneficiaryManager' shared autowired service.
     *
     * @return \App\Client\Manager\BeneficiaryManager
     */
    protected function getBeneficiaryManagerService()
    {
        return $this->services['App\Client\Manager\BeneficiaryManager'] = new \App\Client\Manager\BeneficiaryManager(${($_ = isset($this->services['doctrine.orm.client_entity_manager']) ? $this->services['doctrine.orm.client_entity_manager'] : $this->getDoctrine_Orm_ClientEntityManagerService()) && false ?: '_'}, ${($_ = isset($this->services['property_accessor']) ? $this->services['property_accessor'] : $this->getPropertyAccessorService()) && false ?: '_'});
    }

This function is called more than once in the 2 versions. I think (in v3.4.21): when it's called the second time, it recreates the entitymanager then overrides $this->services['doctrine.orm.client_entity_manager'] (done in the function $this->getDoctrine_Orm_ClientEntityManagerService()) and as we returns the App\\Client\\Manager\\BeneficiaryManager because it already exists (the new condition), it keeps the old entitymanager and has not injected the second.

In the v3.4.18, it recreates and overrides all times the App\\Client\\Manager\\BeneficiaryManager and has always the last instance of entitymanager.

Excuse me if I'm not clear, it's difficult to explain. ;)
I hope this can helps the fix.

@MWMathieu
Copy link

@nicolas-grekas Do you have some news about this bug ?
I lock symfony/dependency-injection at 4.2.2 and its work but its temporary fix.

@virtualize
Copy link

Just tried symfony/* 4.3-beta2. Still getting two different entity manager instances...

@virtualize
Copy link

virtualize commented May 24, 2019

Found a fix that works well (at least in my applications).

In every service, that needs an EntityManager flush to database, I inject the doctrine RegistryInterface instead of the EntityManagerInterface and then get the manager from the registry.
Which is basically the same that we do in controllers all the time.

use Symfony\Bridge\Doctrine\RegistryInterface;

/**
 * @var RegistryInterface
 */
protected $doctrine;

public function __construct(RegistryInterface $doctrine)
{
    $this->doctrine = $doctrine;
}

public function generate()
{
    $entity = new Entity()
    $em = $this->doctrine->getManager();
    $em->persist($entity);
    $em->flush();
}

@Ioni14
Copy link
Contributor

Ioni14 commented May 24, 2019

@virtualize I do the same thing: inject the RegistryInterface instead of EntityManagerInterface.

@virtualize
Copy link

@Ioni14 thanks for the hint. Using RegistryInterface instead of ManagerRegistry seem even more future proof, as ManagerRegistry was removed in DoctrineBundle 1.9 and the removal reverted in 1.9.1
https://github.com/doctrine/DoctrineBundle/releases/tag/1.9.0
I've updated my comment above.

@MWMathieu
Copy link

@virtualize You work with v4.2.8 ?

@virtualize
Copy link

virtualize commented May 24, 2019

@MWMathieu yes I do now. Works flawlessly. I removed my composer lock on symfony/dependency-injection at 4.2.2

@spackmat
Copy link
Contributor

Adding symfony/proxy-manager-bridge in my project solved this for me, as stated here by @dmaicher

@wiseguy1394
Copy link
Contributor

Injecting RegistryInterface instead of EntityManagerInterface is just a workaround. In bigger applications it is not possible to refactor all occurences of the EntityManagerInterface.
Also adding symfony/proxy-manager-bridge does not solve the issue.
Locking symfony/dependency-injection to 4.2.2 blocks the update to symfony 4.3.0.

@wiseguy1394
Copy link
Contributor

wiseguy1394 commented May 31, 2019

Injecting RegistryInterface instead of EntityManagerInterface is less effort than I thought. It only needs one Factory Class:

class OwnEntityManagerFactory
{
    /**
     * @param RegistryInterface $registry
     * @return \Doctrine\Common\Persistence\ObjectManager
     */
    public static function getEntityManagerFromRegistry(RegistryInterface $registry)
    {
        return $registry->getManager();
    }
}

and some config in services.yaml

App\Service\OwnEntityManager:
    decorates: Doctrine\ORM\EntityManagerInterface
    factory: ['@App\Service\OwnEntityManagerFactory',getEntityManagerFromRegistry]
    arguments: ['@Symfony\Bridge\Doctrine\RegistryInterface']

But I think it's still a workaround.

@Lctrs
Copy link
Contributor

Lctrs commented Jun 24, 2019

Minimal repro I could get if someone wants to look at it : https://github.com/Lctrs/symfony-issue-30091.

@maxailloud
Copy link
Contributor

Looks like @Lctrs made something that can be use to fix the issue.
Is it enough to work on @stof ?

This bug is really blocking.

@Lctrs
Copy link
Contributor

Lctrs commented Jun 24, 2019

From what I can see in the dumped container, it's a circular reference issue.

We have an initialization order like this : ReproduceIssueCommand -> EntityManager -> DbalConnection -> DoctrineSubscriber -> EntityManager.

So when the container instantiates the first EntityManager, it will initializes another one for the DoctrineSubscriber, because the first one is not yet in the array of initializated services.

Another workaround could be to convert all doctrine subscribers to doctrine listeners. It's the recommended ways to do it according to the documentation because doctrine listeners can be lazy loaded, so the issue would not occur with them.

@dmaicher
Copy link
Contributor

@Lctrs yes that is also what I mentioned here for a related issue:

#28869 (comment)

The question is: why is it not detected as a circular dependency? 😄

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jun 24, 2019

Because it's not :)
One of the edges in the loop is targetting a setter, which makes the loop breakable.

@weaverryan
Copy link
Member

I should have a failing test case shortly...

@maxailloud
Copy link
Contributor

Is there any workaround for this issue while waiting for a fix?
@wiseguy1394 solution is not working for me, and Injecting RegistryInterface instead of EntityManagerInterface is just not a temporary solution let's say.

@dmaicher
Copy link
Contributor

dmaicher commented Jul 3, 2019

Is there any workaround for this issue while waiting for a fix?
@wiseguy1394 solution is not working for me, and Injecting RegistryInterface instead of EntityManagerInterface is just not a temporary solution let's say.

you could try to make your listeners/services (where the EM is injected) lazy? See #30091 (comment)

@ro0NL
Copy link
Contributor Author

ro0NL commented Jul 3, 2019

if lazy is the answer now... can we close?

@maxailloud
Copy link
Contributor

The issue is not only linked to Listeners. I have it on regular services as well, just using two different services in one controller is enough to have the issue.
And for the lazy to be the answer you need a new dependency, i don't call that a solution.

@thomas-hiron
Copy link

I just moved my project from 4.2.2 to 4.3.2 and ran into this issue too.
Locking symfony/dependency-injection to 4.2.2 is working for me, but I'm stuck with SF 4.2.10.
Adding symfony/proxy-manager-bridge instead of locking seems to be working too.

I'll wait for the fix with the second solution! Even if I don't understand this component's purpose.
Can I have unwanted behaviors?

@Tobion
Copy link
Member

Tobion commented Jul 6, 2019

I ran into a Segfault due to PHP running in a loop. I had a \Doctrine\Common\EventSubscriber that had a dependency on a service that over several dependecies required a doctrine repository (\Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository). All autwired and with proxy-manager installed. It was not detected as a circular dependency.

@ro0NL
Copy link
Contributor Author

ro0NL commented Jul 19, 2019

Somehow the issue disappeared in my own app, both in 4.2.10 and 4.3.2. Maybe it's fixed on Doctrine's side.

@Lctrs
Copy link
Contributor

Lctrs commented Jul 19, 2019

Issue still occurs in my reproducer with 4.2.10 and 4.3.2.

@Thorry84
Copy link
Contributor

Any progress on this? Can confirm this still occurs with the latest versions (as of right now) and causes really strange and hard to debug issues.

nicolas-grekas added a commit that referenced this issue Jul 29, 2019
…rryan, nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Fix dumping Doctrine-like service graphs (bis)

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30091, #29637
| License       | MIT
| Doc PR        | -

Dumping the container while accounting for circular references is hard :)

Commits
-------

a37f3e0 [DI] Fix dumping Doctrine-like service graphs (bis)
ee49144 Failing test case for complex near-circular situation + lazy
@nicolas-grekas
Copy link
Member

Fix is now merged, please report back if you still encounter the issue (and open a new issue with a reproducer please if that happens.)

@maxcreation
Copy link

Is this also in 4.3.3 now? Or when is it going to be in 4.3 branch?

@nicolas-grekas
Copy link
Member

Fix will be part of 4.3.4 / 3.4.31

@fabienlem
Copy link

fabienlem commented Aug 2, 2019

I would like to provide you with additionnal feedback:

I have been able to reproduce this bug several times on my project. From what I have seen, this may occur when the EntityManager is injected both in a Doctrine EventSubscriber and in other "regular" services.

@fabienlem
Copy link

Hello there, bug seems fixed with symfony 3.4.31! 👍 Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests