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

Proxy generating __sleep with uninitialized properties (PHP7.4.2+) #886

Closed
bizley opened this issue Mar 18, 2020 · 13 comments
Closed

Proxy generating __sleep with uninitialized properties (PHP7.4.2+) #886

bizley opened this issue Mar 18, 2020 · 13 comments

Comments

@bizley
Copy link

bizley commented Mar 18, 2020

Proxy generator is creating __sleep method with uninitialized typed properties which is a critical bug since PHP 7.4.2 (https://bugs.php.net/bug.php?id=79002) ending in message:

Typed property Proxies_CG_\App\Entity\EntityName::$ must not be accessed before initialization (in __sleep)

I've already reported it (not knowing the source of error hence the wrong repo) in doctrine/orm#8030 but it looks like it's stuck there forgotten.

@bizley bizley changed the title Proxy generating __sleep with uninitialized properties Proxy generating __sleep with uninitialized properties (PHP7.4.2+) Mar 18, 2020
@2ec0b4
Copy link

2ec0b4 commented Mar 23, 2020

I'm stuck with the same error. I manage to reproduce it with the smallest Symfony project. Here it is: https://github.com/2ec0b4/symfony-proxy-issue

@sebastianlew
Copy link

sebastianlew commented Mar 25, 2020

I wanted to make PR fixing that problem, but i think it isn't as simple as we think :)

Few ideas i had:

  1. Maybe solution is to serialize just identifier if entity is not initialized yet. I tested it, it seems work in most cases, but what with case when we serialize uninitialized entity, and then try to unserialize and access different property? Same problem, we also get an error that property is not initialized.

  2. Second solution is to initialize whole entity in _sleep before we serialize it. But i think this can really affect performance, so the first one sounds better for me. The question is if it is possible and how it could run in wider context if we serialize just identifier when entity is not initialized by doctrine

@nicolas-grekas
Copy link
Member

To me, a fix should be considered at the PHP level, I opened https://bugs.php.net/79447 🤞

@sebastianlew
Copy link

@nicolas-grekas That’s why I wrote

I wanted to make PR fixing that problem, but i think it isn't as simple as we think :)

cause i didn’t see any perfect solution to solve this problem directly in doctrine 😁

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 23, 2020

php/php-src#5396 will be part of PHP 7.4.6, thus nothing to do here.

@sebastianlew
Copy link

@nicolas-grekas I saw that problem escalated a little and also your PR was closed on php src. Will that problem be fixed soon in php src? What's the state of this problem for now ?

@nicolas-grekas
Copy link
Member

Give PHP 7.4.6 a try, it's been released yesterday.

@sebastianlew
Copy link

Everything works fine now 🎉 , thank you @nicolas-grekas

@Ocramius
Copy link
Member

Probably best to add an exclusion range for 7.4.0~7.4.5

@alcaeus
Copy link
Member

alcaeus commented May 15, 2020

Probably best to add an exclusion range for 7.4.0~7.4.5

That won't help: any release without this exclusion is still eligible but also suffers from the problem. This is not our problem to fix.

@Ocramius
Copy link
Member

It would still make upgrading to latest version clear and blocked off until 7.4.6 is installed (good).

This has been done for similar reasons in:

Overall, it provides guidance for downstream, and this package is indeed still broken in 7.4.0~7.4.5.

@sebastianlew
Copy link

sebastianlew commented May 15, 2020

I think that first step also could be improvement of the tests in doctrine/common cause despite this problem tests have been passing for e.g. tests/Doctrine/Tests/Common/Proxy/LazyLoadableObjectWithTypedProperties.php (I'll try to do it if nobody do it first)

EDIT: I know that problem was fixed in php-src directly, and doctrine/common shouldn't check functionalities that are in php-src, but still in my opinion this test doesn't verify a lot of cases

@beberlei
Copy link
Member

beberlei commented Dec 3, 2020

Closing as its a PHP bug.

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

No branches or pull requests

7 participants