Skip to content

[DependencyInjection] PhpDumper::isFrozen inconsistency #19704

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

Closed
wants to merge 2 commits into from
Closed

[DependencyInjection] PhpDumper::isFrozen inconsistency #19704

wants to merge 2 commits into from

Conversation

allflame
Copy link
Contributor

@allflame allflame commented Aug 22, 2016

Q A
Branch? 3.1
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

There is a bug (from my prespective) regarding cached container generated by the PhpDumper. ProjectServiceContainer::isFrozen call will be forwarded to the Container::isFrozen method which relies on instance of the parameter bag property. In the cached ProjectServiceContainer parameter bag is undefined afetr intialization hence calls to the isFrozen will result in false unless getParameterBag will be called and then calls to isFrozen will return true onwards.
This can actually break some compatibility, although it's a bug from my prespective from the very beginning

@nicolas-grekas
Copy link
Member

Looks like a bug to me also, not a bc break.
👍 on 2.7

@stof
Copy link
Member

stof commented Aug 22, 2016

👍 on 2.7

@fabpot
Copy link
Member

fabpot commented Aug 22, 2016

Thank you @allflame.

fabpot added a commit that referenced this pull request Aug 22, 2016
…llflame)

This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #19704).

Discussion
----------

[DependencyInjection] PhpDumper::isFrozen inconsistency

| Q             | A
| ------------- | ---
| Branch?       | 3.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |
There is a bug (from my prespective) regarding cached container generated by the PhpDumper. ProjectServiceContainer::isFrozen call will be forwarded to the Container::isFrozen method which relies on instance of the parameter bag property. In the cached ProjectServiceContainer parameter bag is undefined afetr intialization hence calls to the isFrozen will result in false unless getParameterBag will be called and then calls to isFrozen will return true onwards.
This can actually break some compatibility, although it's a bug from my prespective from the very beginning

Commits
-------

7c0a62c [DependencyInjection] PhpDumper::isFrozen inconsistency
@fabpot fabpot closed this Aug 22, 2016
@allflame
Copy link
Contributor Author

@fabpot can we get a backmerge to 3*? Or what should I do to get this fix there?

@nicolas-grekas
Copy link
Member

@allflame it will come to 3.* as part of the regular merges from 2.7

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

Successfully merging this pull request may close these issues.

5 participants