laminas-servicemanager icon indicating copy to clipboard operation
laminas-servicemanager copied to clipboard

Lazy services should not have their desctructor proxied

Open func0der opened this issue 1 year ago • 4 comments

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.

func0der avatar Aug 05 '23 01:08 func0der

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 avatar Oct 10 '23 21:10 boesing

@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?

boesing avatar Oct 10 '23 21:10 boesing

@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.

func0der avatar Oct 11 '23 13:10 func0der

You can provide that PoC whenever you want.

boesing avatar Oct 11 '23 18:10 boesing