Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[DI] fix taking lazy services into account when dumping the container#29247

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:3.4fromnicolas-grekas:di-fix34
Nov 20, 2018

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedNov 18, 2018
edited
Loading

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#29246
LicenseMIT
Doc PR-

This PR fixes issues found while working on#29246.
Itdoes fix the infinite loop,but replaces it by an exception (reopening#29078):

It's a requirement to specify a Metadata Driver and pass it to Doctrine\ORM\Configuration::setMetadataDriverImpl()

The full fix is not immediately accessible as it needs some core changes to the dumping logic. Requiringsymfony/proxy-manager-bridge works around the issue properly.

See#29251 for 4.2

@ro0NL
Copy link
Contributor

ro0NL commentedNov 18, 2018
edited
Loading

Does it mean some form of BC break happend along the way? Im curious as only as of 4.2 things became problematic.. before it worked like a charm. Did i rely on something unsupported actually?

@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedNov 18, 2018
edited
Loading

3.4 is equally affected (that's where this PR applies.) No BC break here, just a core issue with dumping complex service graphs that have varying side effects depending on the state of the fixes. With this PR, the dumped container should always be correct (ie no infinite loop) BUT it doesn't dump setters in the most optimized order when circular loops are involved. This doesn't play well with Doctrine's Configuration objects which expect to be fully defined before being injected.

@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedNov 18, 2018
edited
Loading

I found a workaround: disabling inlining of services with outgoing edges in lazy services is enough to fix the Doctrine use case - ie have a Configuration instance fully ready before injecting it.
In theory, the issue could come back again if the entity manager service is not declared lazy anymore. In practice, this was required with Symfony 2 - and I don't expect such complex graph + expectations to be common.

Should be good enough, PR ready.

For reference, the fully fixed behavior would involve dumping all services using the call stack (ie following references recursively when dumping - where the current logic stops at them) - for history and reference,#29252 contains a reproducer so that if fixing this is needed in the future, we can start from it.

@nicolas-grekasnicolas-grekas merged commit67d7623 intosymfony:3.4Nov 20, 2018
nicolas-grekas added a commit that referenced this pull requestNov 20, 2018
…e container (nicolas-grekas)This PR was merged into the 3.4 branch.Discussion----------[DI] fix taking lazy services into account when dumping the container| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#29246| License       | MIT| Doc PR        | -This PR fixes issues found while working on#29246.It *does* fix the infinite loop, ~but replaces it by an exception (reopening#29078)~:> ~It's a requirement to specify a Metadata Driver and pass it to Doctrine\ORM\Configuration::setMetadataDriverImpl()~The full fix is not immediately accessible as it needs some core changes to the dumping logic. Requiring `symfony/proxy-manager-bridge` works around the issue properly.See#29251 for 4.2Commits-------67d7623 [DI] fix taking lazy services into account when dumping the container
@nicolas-grekasnicolas-grekas deleted the di-fix34 branchNovember 20, 2018 16:40
This was referencedNov 26, 2018
This was referencedNov 26, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

4 participants

@nicolas-grekas@ro0NL@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp