Skip to content

[DI] [6.2] Lazy object generation not working when same class is used for multiple lazy services #48507

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
keulinho opened this issue Dec 6, 2022 · 5 comments

Comments

@keulinho
Copy link
Contributor

keulinho commented Dec 6, 2022

Symfony version(s) affected

6.2

Description

When you have multiple lazy services that use the same class but are configured differently (one without a factory, one with a factory) the lazy object generation is broken.

The reason is that for the class in question only one ProxyObject is generated, which either uses the LazyGhostTrait or LazyProxytTrait, but not both. This proxy object is used for both services, but as one of those services uses a factory it expects an LazyProxy, the service that does not use a factory expects the LazyGhost. That means that one object cannot be instantiated.

Our specific use case is that we have several GuzzleClients as different services configured, as they use different middlewares and different configurations. Some of them are marked as lazy which leads to the problems described above.

How to reproduce

Having a service declaration like the following:

<service id="store_client" class="GuzzleHttp\Client" lazy="true">
    <factory service="StoreClientFactory" method="create"/>
</service>

<service id="app_system.guzzle" class="GuzzleHttp\Client" lazy="true">
    <argument type="collection">
        <argument key="timeout">5</argument>
         <argument key="connect_timeout">1</argument>
     </argument>
</service>

Will lead to the following proxy class being generated:

class Client_1efc5d2 extends \GuzzleHttp\Client implements \Symfony\Component\VarExporter\LazyObjectInterface
{
    use \Symfony\Component\VarExporter\LazyGhostTrait;

    private const LAZY_OBJECT_PROPERTY_SCOPES = [
        "\0".parent::class."\0".'config' => [parent::class, 'config', null],
        'config' => [parent::class, 'config', null],
    ];
}

Note: This example is greatly simplified, in our case this proxy is generated, i'm not sure but i suppose that it may also happen that a proxy will be generated that uses the LazyProxyTrait, but that will lead to the same problem down the line.

The generated service container will use the generated proxy in the following way:

    /**
     * Gets the private 'store_client' shared service.
     *
     * @return \GuzzleHttp\Client
     */
    protected function getStoreClientService($lazyLoad = true)
    {
        if (true === $lazyLoad) {
            return $this->privates['store_client'] = $this->createProxy('Client_1efc5d2', fn () => \Client_1efc5d2::createLazyProxy(fn () => $this->getStoreClientService(false)));
        }

        return (new StoreClientFactory())->create();
    }

    /**
     * Gets the private 'app_system.guzzle' shared service.
     *
     * @return \GuzzleHttp\Client
     */
    protected function getAppSystem_GuzzleService($lazyLoad = true)
    {
        if (true === $lazyLoad) {
            return $this->privates['app_system.guzzle'] = $this->createProxy('Client_1efc5d2', fn () => \Client_1efc5d2::createLazyGhost($this->getAppSystem_GuzzleService(...)));
        }

        return ($lazyLoad->__construct(['timeout' => 5, 'connect_timeout' => 1]) && false ?: $lazyLoad);
    }

Important: You can see that in the getter for the first service (that uses a factory) it is expected that the proxy class is a LazyProxy, but for the second service (without a factory) it expects a LazyGhost, but the auto generated proxy uses only one of those traits. This leads to problems in the creation of on of those services, because in fact the proxy does not implement the called method.

Possible Solution

One solution might be that in cases like the one described above the auto generated proxy class for the GuzzleClient uses both the LazyGhostTrait and the LazyProxyTrait. But i'm not sure if the traits are designed to be used in one class simultaneously or if the traits conflict with each other.

If the latter is the case at least two proxies should be generated for the same class in cases like the one above, one implementing the LazyGhostTrait and the other implementing the LazyObjectTrait.

Additional Context

No response

@stof
Copy link
Member

stof commented Dec 6, 2022

the right solution is to generate 2 different proxy classes

@keulinho
Copy link
Contributor Author

keulinho commented Dec 6, 2022

Taking a look at LazyObjectInterface it is probably wrong that the same proxy is used for multiple services if they have the same class correct? I thought reusing the same proxy might be some kind of optimization, but now it looks to me like that is the root cause of the bug in the first place. As also the LazyObjectInterface cannot really work with multiple instances right? So if each lazy service has it's own proxy the issue will be solved as well.

@nicolas-grekas
Copy link
Member

We need to vary the name of the generated class by "kind" (ghost/proxy) and generate two of them indeed when needed.
Up for giving it a try @keulinho?

@nicolas-grekas
Copy link
Member

I fixed this in #48522, thanks for reporting!

@keulinho
Copy link
Contributor Author

keulinho commented Dec 7, 2022

Nice, thanks for the quick fix! 👍

nicolas-grekas added a commit that referenced this issue Dec 7, 2022
… objects and virtual proxies (nicolas-grekas)

This PR was merged into the 6.2 branch.

Discussion
----------

[DependencyInjection] Generate different classes for ghost objects and virtual proxies

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #48507
| License       | MIT
| Doc PR        | -

Commits
-------

c5da2eb [DependencyInjection] Generate different classes for ghost objects and virtual proxies
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

4 participants