Skip to content

[DependencyInjection] circular references using setter #25240

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
ElectricMaxxx opened this issue Dec 1, 2017 · 8 comments
Closed

[DependencyInjection] circular references using setter #25240

ElectricMaxxx opened this issue Dec 1, 2017 · 8 comments
Labels
Bug DependencyInjection Status: Needs Review

Comments

@ElectricMaxxx
Copy link
Contributor

ElectricMaxxx commented Dec 1, 2017 •
edited
Loading

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 4.0

Hi,

i think i have an issue with circular references in our Symfony 4.0 build on DoctrinePhpcrBundle.

You can reproduce it it by:

git clone https://github.com/doctrine/DoctrinePHPCRBundle.git
git checkout allow_symfony_4.0
composer config minimum-stability dev
composer require symfony/symfony:^4.0 --no-update
composer update --prefer-dist

(same as travis does it)
Then you can call every console command (lives in testing component, kernel is here):

 export KERNEL_CLASS=Doctrine\\Bundle\\PHPCRBundle\\Tests\\Fixtures\\App\\Kernel && ./vendor/symfony-cmf/testing/bin/travis/../console doctrine:phpcr:init:dbal --drop -vvv

You should see the issue then. Currently i do no know why? Are we doing something wrong with the service definition or is it the setter for the TranslationStrategy. When debugging into i got the following image at the end where the exception is thrown:

selection_125

Notice:

  • The builds on Symfony 3.3, 3.4 and 2.8 are green
@stof
Copy link
Member

stof commented Dec 1, 2017

Well, you indeed have a circular reference here: you inject the strategy into the document manager, and the document manager into the strategy. This is very likely to break (writing the code able to instantiate such object graph is quite hard)

@stof
Copy link
Member

stof commented Dec 1, 2017

however, having it working on 3.4 and failing on 4.0 looks weird to me.

@nicolas-grekas
Copy link
Member

The difference must come from some services which have been inlined now that they are really private.
The PhpDumper has some anti-circular checks that might be obsolete now. I'll have a look.

@ElectricMaxxx
Copy link
Contributor Author

Uhhh you are right, did not recoginze that among the code.

@ElectricMaxxx ElectricMaxxx mentioned this issue Dec 1, 2017
Merged
@ElectricMaxxx
Copy link
Contributor Author

@stof you got a pattern/hint for me to resolve those references? both classes need each other as it seems. both classes should be created by the DI.

@Jean85 Jean85 mentioned this issue Dec 1, 2017
Closed
@nicolas-grekas nicolas-grekas mentioned this issue Dec 1, 2017
Merged
@nicolas-grekas
Copy link
Member

Fixed in #25247

fabpot added a commit that referenced this issue Dec 1, 2017
@fabpot
bug #25247 [DI] Fix false-positive circular exception (nicolas-grekas)
dccc5cc 
This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Fix false-positive circular exception

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

Commits
-------

adf1819 [DI] Fix false-positive circular exception
@fabpot fabpot closed this as completed Dec 1, 2017
@ElectricMaxxx
Copy link
Contributor Author

@nicolas-grekas will you release your fix also?

@nicolas-grekas
Copy link
Member

What does that mean?
The fix will be released with 3.4.1, which should happen in 4-5 weeks max.

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
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @nicolas-grekas @stof @xabbuh @ElectricMaxxx @carsonbot