Skip to content

New circular reference errors that weren't there in Symfony DI version 3.3 #25044

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.

Sign up for GitHub

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

Jump to bottom
Closed
jagjeetdhaliwal opened this issue Nov 20, 2017 · 9 comments
Closed

New circular reference errors that weren't there in Symfony DI version 3.3 #25044

jagjeetdhaliwal opened this issue Nov 20, 2017 · 9 comments
Labels
Bug DependencyInjection Status: Needs Review Status: Waiting feedback

Comments

@jagjeetdhaliwal
Copy link

Q A
Bug report? yes
Feature request? no
BC Break report? yes
RFC? no
Symfony version 3.4.0 (Beta)

Just upgraded to the beta version (3.4) of the Dependency Injection (DI) package. On running the same test suite, we are getting new circular reference errors that weren't there in version 3.3. The codebase and test suite are exactly the same and only the DI package has been upgraded. Also on digging much deeper, the circular references don't really seem to exist. Just reverted back to the old Symfony DI version (3.3) using composer and everything is running smoothly again.

Please let me know if you need any more information!
@nicolas-grekas
Copy link
Member

We need a reproducer actually, the current description is unactionable if anyone else than you would like to work on it.

@Simperfit
Copy link
Contributor

Could you do the following, so we could get a reproducer ?

In case you need some help you can read this blog post : https://symfony.com/doc/current/contributing/code/reproducer.html

@nicolas-grekas
Copy link
Member

Might you also check #25055 see if it changes anything for you?

@jagjeetdhaliwal
Copy link
Author

Guys, will try it out and let you know soon! Thanks for the advice.

@nicolas-grekas
Copy link
Member

ping @jagjeetdhaliwal any news? would be great to have confirmation/information before 4.0.0 (ie next Friday)

@gedimin45
Copy link

Hello people!

We managed to reproduce the problem: https://github.com/ged15/symfony-34-di-bug/blob/master/src/AppBundle/Controller/DefaultController.php#L17-L18.

We are trying to get services by exploiting the fact that Symfony DI allows case-insensitive ID lookup. I know that is not a good idea but that worked in Symfony 3.3 and we do it for legacy reasons.

Let me or @jagjeetdhaliwal know if we can help further!

@NicoHaase
Copy link
Contributor

According to the changelog, case-insensitivity is deprecated since Sf 3.4, so you should probably go another way?

@gedimin45
Copy link

@NicoHaase we should, but, once again, legacy 😢 It is still a BC break.

@nicolas-grekas nicolas-grekas mentioned this issue Dec 4, 2017
Merged
@nicolas-grekas
Copy link
Member

thanks for the reproducer, fixed #25044

fabpot added a commit that referenced this issue Dec 4, 2017
@fabpot
bug #25313 [DI] Fix missing unset leading to false-positive circular …
8353d69 
…ref (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Fix missing unset leading to false-positive circular ref

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

Commits
-------

17d84f6 [DI] Fix missing unset leading to false-positive circular ref
@fabpot fabpot closed this as completed Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DependencyInjection Status: Needs Review Status: Waiting feedback
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@fabpot @nicolas-grekas @NicoHaase @xabbuh @Simperfit @gedimin45 @jagjeetdhaliwal @carsonbot