Skip to content
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

[HttpKernel] Prevent initialising lazy services during services reset #50241

Merged
merged 1 commit into from May 5, 2023

Conversation

tucksaun
Copy link
Contributor

@tucksaun tucksaun commented May 4, 2023

Q A
Branch? 6.2
Bug fix? yes-ish
New feature? no
Deprecations? no
Tickets none
License MIT
Doc PR

If one happens to have a service that is configured to be lazy AND also implements ResetInterface, when Symfony resets resettableService, it will call reset on the service which will trigger the initialization even though the service has not been actually used by the code, which defeats the purpose of having the service lazy.
To fix this I propose to skip resetting a service that is marked as LazyObjectInterface and has not been initialized (ie not used yet)

@carsonbot carsonbot added this to the 6.2 milestone May 4, 2023
@tucksaun tucksaun changed the title [HttpKernel] Prevent initialising lazy services when reseting them [HttpKernel] Prevent initialising lazy services during services reset May 4, 2023
@tucksaun

This comment was marked as resolved.

@stof
Copy link
Member

stof commented May 5, 2023

Do we also have the issue for lazy ghosts or no ?

@tucksaun
Copy link
Contributor Author

tucksaun commented May 5, 2023

Do we also have the issue for lazy ghosts or no ?

Good question. I would say it should be. But let me check that later on today

@nicolas-grekas
Copy link
Member

Thank you @tucksaun.

@nicolas-grekas nicolas-grekas merged commit f1af316 into symfony:6.2 May 5, 2023
9 checks passed
@tucksaun tucksaun deleted the fix/lazy-services-reset branch May 5, 2023 16:46
@tucksaun
Copy link
Contributor Author

tucksaun commented May 5, 2023

Do we also have the issue for lazy ghosts or no ?

Good question. I would say it should be. But let me check that later on today

Confirmed, the object was a lazy ghost when I encountered this issue

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.

None yet

4 participants