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

[From PHP-DI] __destruct magic method being called before lazy Proxy creates value #669 #468

Open
odahcam opened this issue Jun 14, 2019 · 10 comments
Labels

Comments

@odahcam
Copy link

odahcam commented Jun 14, 2019

Disclaimer: this issue was originally submitted here: PHP-DI/PHP-DI#669 . The context is about PHP-DI.

When the class gets destroyed in its life cycle, the __destruct method gets called and, if there's one implementation in the original class, the proxy class tries to execute the base class implementation. But looks like the proxy class is getting destroyed before creating a instance for the base class and, as there's no check for null values, we get the following error:

[Wed Jun 12 00:52:18 2019] ::1:59011 [500]: / - Uncaught Error: Call to a member function __destruct() on null in <Project-path>\var\proxies\ProxyManagerGeneratedProxy__PM__<CLASS_NAME>Generated8d9f8c89dc680a18cbf6f65f21952d52.php:55
Stack trace:
#0 <Project-path>\vendor\php-di\php-di\src\Proxy\ProxyFactory.php(60): ProxyManagerGeneratedProxy\__PM__\<CLASS_NAME>\Generated8d9f8c89dc680a18cbf6f65f21952d52->__destruct()
#1 <Project-path>\vendor\php-di\php-di\src\Proxy\ProxyFactory.php(74): DI\Proxy\ProxyFactory->createProxy('<CLASS_NAME>', Object(Closure))
#2 <Project-path>\vendor\php-di\php-di\src\Compiler\ObjectCreationCompiler.php(140): DI\Proxy\ProxyFactory->generateProxyClass('<CLASS_NAME>')
#3 <Project-path>\vendor\php-di\php-di\src\Compiler\ObjectCreationCompiler.php(39): DI\Compiler\Obje in <Project-path>\var\proxies\ProxyManagerGeneratedProxy__PM_<CLASS_NAME>Generated8d9f8c89dc680a18cbf6f65f21952d52.php on line 55

The project path and class full qualified names were hidden because it's a private project, where the class is related to database connectivity.

That class just access authorization data and creates a new connection instance. The __destruct method implementation exists to ensure the connection will be closed when the class gets destroyed.

In this scenario the class was never used and that's why I believe it's getting destructed before creating an instance for the original class.

@Ocramius
Copy link
Owner

This is very likely a duplicate of #373

@odahcam
Copy link
Author

odahcam commented Jun 15, 2019

Yes, looks like it is, but I would like to discuss it again if possible, starting with: why the instance inside the proxy-generated class is never checked for null values before being called?

I think this is a serious problem, the proxy class should check if the instance value exists before calling it. The proxy should do that at least in the magic methods, cause there's no way to guarantee the magic method is being called by the user code or by some PHP magic.

Just my thoughts. I'm confident it matters for @mnapoli too, as I'm experiencing this trough his lib.

@Ocramius
Copy link
Owner

In theory, the initializer should have been called. The only reason why this could fail is a failure to initialise the property: that should lead to an exception to be thrown, but due to the weird semantics of __destruct, it gets called anyway.

Do you maybe have an example of the class that was proxied, and its initializer?

@odahcam
Copy link
Author

odahcam commented Jun 15, 2019

This is the proxied class, attention in Injectable:

<?php

declare(strict_types=1);

namespace SOME_NAMESPACE;

/**
 * @Injectable(lazy=true)
 */
class CLASS_NAME implements INTERFACE_IMPLEMENTATION
{
    /**
     * {@inheritdoc}
     */
    public function __construct()
    {
        // code omitted
    }

    /**
     * {@inheritdoc}
     */
    public function closeConnection(): void
    {
        // code omitted
    }

    /**
     * {@inheritdoc}
     */
    public function __destruct()
    {
        $this->closeConnection();
    }
}

Injectable is a property read by PHP-DI, that uses this class and the proxy to create lazy injection functionality. In this case, I don't know where the proxy is really generated, maybe @mnapoli can explain to us better.

@Ocramius
Copy link
Owner

Can __construct for this object throw? I think you could try writing a test in https://github.com/Ocramius/ProxyManager/tree/2.4.0/tests/language-feature-scripts

I don't know which kind of proxy is being used in the upstream library though.

@odahcam
Copy link
Author

odahcam commented Jun 15, 2019

Well, construct is just setting a property, like:

<?php

class {
    /**
     * {@inheritdoc}
     */
    public function __construct(
        ContainerInterface $container
    ) {
        $this->container = $container;
        // The connection configuration.
        $this->connectionParams = [/* data */] ?? [/* other data */ ];
    }
}

I would say it doesn't throws, because no integrations tests failed while in development, but when in production, where the PHP-DI container gets compiled, the error shows up.

@anologicon
Copy link

Do we have any news about this issue ?

@Ocramius
Copy link
Owner

@anologicon I'd need an example where this fails in an integration test, as noted in #468 (comment)

@Ocramius
Copy link
Owner

Also needed: proxy type in use.

@odahcam
Copy link
Author

odahcam commented Jun 29, 2019

I don't know if I've implemented it right, but there it is: #471

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

No branches or pull requests

3 participants