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

Lazy services should not have their desctructor proxied #202

Open
func0der opened this issue Aug 5, 2023 · 4 comments
Open

Lazy services should not have their desctructor proxied #202

func0der opened this issue Aug 5, 2023 · 4 comments

Comments

@func0der
Copy link

func0der commented Aug 5, 2023

Feature Request

When you proxy a class that has a destructor in it, the proxy manager by default creates a proxy for the __destruct method.
Then without actually needing or using the class the destructor is called at the end of execution which loads the lazy loaded class.

There is a possibility to configure this behavior in the proxy manager by passing $proxyOptions['skipDestructor'], but there is currently no way to pass any proxy options to the proxy creation process from the service manager config.

There could be basically two routes here:

  • Either do not add the destructor by default
    • this could potentially break some code
    • Kinda makes sense since we are not calling the class actively and therefore should not need a destructor
      • Putting it like this also could make this a proxy manager feature: adding destructor dynamically or better only calling it when the class is actually initialized
  • Add the possibility to define $proxyOptions for each lazy services registered
Q A
New Feature yes
RFC yes
BC Break yes&no

Summary

Have destructor of lazy services dealt with in a way that does not call them without the class being ever used in that run of PHP.

@boesing
Copy link
Member

boesing commented Oct 10, 2023

Not sure if this is actually related to laminas-servicemanager or to laminas-code or ocramius/proxy-manager?
@Ocramius do you have an idea on how to handle this? Is this intended?

@boesing
Copy link
Member

boesing commented Oct 10, 2023

@func0der would you mind creating a PoC to get glympse? Most preferably with a failing unit test to understand the actual problem. If I understand correct, a __destruct is proxied to the instance even tho the service was never initialized in the first place? So on __destruct, the service will be initialized even tho we could early return in case the service was never loaded?

@func0der
Copy link
Author

@boesing Sorry, I currently do not have time for a PoC.

Easiest way for me to reproduce this was: Have class in the service manager that initializes a connection to Redis in the factory. For example \Redis::class => \RedisFactory::class. Make \Redis::class lazy loadable.
Build the service manager in an environment where you do not have a Redis server instance running, for example unit tests in CI or just stop it in your environment.
Run the tests.
Boom.

The __destruct of the proxy is called, the instance is initialized and it tries to connect to the redis server in RedisFactory.

@boesing
Copy link
Member

boesing commented Oct 11, 2023

You can provide that PoC whenever you want.

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

No branches or pull requests

2 participants