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
[Dependency Injection] No Circular reference warning for Symfony 3.4 & Doctrine #24775
Comments
I think something could to be done to improve the DX, what do you want to have in order to ease the debbuging ? |
It would be just great to see this |
Status: Needs Review |
Yea, this definitely "smells" weird to me (i.e. a bug) - especially with the different behavior on the different versions and public vs private. Someone needs to dig in a bit. |
@nicolas-grekas |
No idea yet, if someone can look at a reproducer or a fix, that'd be great :) |
|
@mimol91 could you try to work on a PR, fix the fix and a test case? That'd be the best way to prove your theory :) |
Sure, I will try |
Ok,
$instance->register(new \AppBundle\DoctrineListener(${($_ = isset($this->services['doctrine.orm.default_entity_manager']) ? $this->services['doctrine.orm.default_entity_manager'] : $this->get('doctrine.orm.default_entity_manager')) && false ?: '_'})); The important part is that if service is not registered in container it call What happens on SF3.4: $instance->register(new \AppBundle\DoctrineListener(${($_ = isset($this->services['doctrine.orm.default_entity_manager']) ? $this->services['doctrine.orm.default_entity_manager'] : $this->load(__DIR__.'/getDoctrine_Orm_DefaultEntityManagerService.php')) && false ?: '_'})); Similar as previous service does not exist, so How I think it can be fixed, $instance->register(new \AppBundle\DoctrineListener(${($_ = isset($this->services['doctrine.orm.default_entity_manager'])
? $this->services['doctrine.orm.default_entity_manager']
: $this->loading['doctrine.orm.default_entity_manager']
? $this->get('doctrine.orm.default_entity_manager')
: $this->load(__DIR__.'/getDoctrine_Orm_DefaultEntityManagerService.php')) && false ?: '_'})); I think its the root cause, would be great if someone would create PR with fix, becaue I am not sure how can I write tests for it, I do not feel comfortable with this phpDumper. |
😨 Is it possible extract some variables to make generated code more readable? |
The issue does not exist when changing require_once $cache->getPath();
$this->container = new $class(); ) has to be changed Ha. You can do check, but You will need to import |
@mimol91 can you provide a fork of the standard edition that reproduces the issue? It would help a lot. |
@nicolas-grekas 3.3 Version (working) 3.4 Version (no error) |
This is a variant of #19362, the reproducer provided there exhibits exactly the same issue. |
Fixed in #24822 (not by throwing an exception, but by handling properly the situation to create a fixed dependency tree.) |
…grekas) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Fix "almost-circular" dependencies handling | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19362, #24775 | License | MIT | Doc PR | - In a situation like the following one, we used to trigger a circular reference exception. But this was a false positive, as the reference is resolvable without hitting the circle. Fixing this exception could be considered as a new feature (because no existing config needs it, since it fails). But for 3.4, this should be considered a bug fix, as reported in #24775: not handling this situation now means creating broken service trees. ``` php $containerBuilder = new ContainerBuilder(); $container->register('foo', FooCircular::class)->setPublic(true) ->addArgument(new Reference('bar')); $container->register('bar', BarCircular::class) ->addMethodCall('addFoobar', array(new Reference('foobar'))); $container->register('foobar', FoobarCircular::class) ->addArgument(new Reference('foo')); $foo = $containerBuilder->get('foo'); ``` Commits ------- beb4df7 [DI] Fix "almost-circular" dependencies handling
Recently I had this error while clearing the cache : "Segmentation fault (core dumped)" |
I'm also having problem with circular reference on Symfony 4.1.4. I have a service (A) where some other services (B, C, D) are injected into it, one of them has Doctrine. If I try to inject service A into a Doctrine subscriber I get an exception:
|
@kgrieco that issue is closed since almost 1 year. It is unlikely that your issue is the same:
Please create a new issue rather than commenting on closed ones (you were lucky that I took attention to the email notification for this comment, as the notification is the only way someone could see it. We don't go read closed issues again otherwise) |
Just in case someone is struggling with this yet, I found that getting repository from services constructor throws "Segmentation fault (core dumped)" when clearing the cache. Removing Bad:
Good:
|
Recently I've experienced quite unexpected behavior while using doctrine (#24700) After some deep investigation it seems that reason is simple -> Circular references.
It seems that different Symfony version show or not different errors.
I've tested it on clean Symfony version
It can be simply recreated just just by creating empty class and adding this code to servies.yml
There are three possible solution how application will behave:
As you can see SF3.4 never mention about circular reference. Which I think still may occur ( or in my case cause some unexpected behavior)
SF 3.3 display (or not ) different error based of condition if service is public or state of cache.
And yes it's again example with Doctrine, however I think DI and circular references are part of Symfony.
(Doctrine version is this same for each of SF version)
Want more magic? Try to
install doctrine-migration-bundle
(DO NOT register it in AppKernel)Now if service is private the error about circular reference never shows!
I am totally aware that its not a 'common' case, but I am sure that this missing error about circular reference could save a lot of hours on debugging =). I do not mind if you close this issue, I just try to save other folks time, because they may have similar problem like I do.
The text was updated successfully, but these errors were encountered: