Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Bridge\ProxyManager] Dont call __destruct() on non-instantiated services#23729
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
c366fc7 to6fb67d5Compare| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| protected function getGenerator() : ProxyGeneratorInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You can't use feature from PHP 7 on branch that supports version 5.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed
6baa81f tob98fc0aCompare| */ | ||
| class LazyLoadingValueHolderFactoryV1 extends BaseFactory | ||
| { | ||
| private $generatorV1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why does it have a different name here ? Except for the property name, the code is the same in v1 and V2.
nicolas-grekasAug 1, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Because in v0.4, the property exists and is protected, thus can't be made private...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
we could just use a name available in both versions
b98fc0a to2d79ffaComparefabpot commentedAug 3, 2017
Thank you@nicolas-grekas. |
…tiated services (nicolas-grekas)This PR was merged into the 2.7 branch.Discussion----------[Bridge\ProxyManager] Dont call __destruct() on non-instantiated services| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -While working on making#23678 green, I discovered that if a lazy service implements `__destruct`, then that service is not lazy anymore: it is created at destruct time.That behavior is documented atOcramius/ProxyManager#258 (+related issues).While I may understand why this behavior is the default for ProxyManager, it does not fit our "lazy-services" use case to me. Typically, nobody wants a database connection to be created to destruct the uninitialized lazy-proxy.Blocks#23678Commits-------2d79ffa [Bridge\ProxyManager] Dont call __destruct() on non-instantiated services
Uh oh!
There was an error while loading.Please reload this page.
While working on making#23678 green, I discovered that if a lazy service implements
__destruct, then that service is not lazy anymore: it is created at destruct time.That behavior is documented atOcramius/ProxyManager#258 (+related issues).
While I may understand why this behavior is the default for ProxyManager, it does not fit our "lazy-services" use case to me. Typically, nobody wants a database connection to be created to destruct the uninitialized lazy-proxy.
Blocks#23678