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

Problem with Proxies on PHP 7.4.2 with typed properties #8030

Closed
bizley opened this issue Feb 19, 2020 · 25 comments
Closed

Problem with Proxies on PHP 7.4.2 with typed properties #8030

bizley opened this issue Feb 19, 2020 · 25 comments
Labels
Milestone

Comments

@bizley
Copy link

bizley commented Feb 19, 2020

(I'm sorry if this is wrong place to report that issue, please transfer me to the proper one if this is the case)

We encountered this problem on the Behat testing phase with generated entity proxies. There is fatal error:

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

This happened when we upgraded PHP to 7.4.2 (everything was ok on 7.4.1). We used typed properties before the upgrade as well. After playing with the entity for a bit we discovered that now every typed object property must be initialized immediately (previously they were initialized in the constructor) like:

private ?DateTimeInterface $property = null;

public function __construct()
{
    $this->property = new DateTime();
}

because this doesn't work anymore:

private DateTimeInterface $property;

public function __construct()
{
    $this->property = new DateTime();
}

This is was already reported in Symfony symfony/symfony#35574 but closed.

Installed Doctrine package Version
doctrine/annotations v1.8.0
doctrine/cache 1.10.0
doctrine/collections 1.6.4
doctrine/common 2.12.0
doctrine/data-fixtures 1.4.2
doctrine/dbal v2.10.1
doctrine/doctrine-bundle 2.0.7
doctrine/doctrine-migrations-bundle 2.1.2
doctrine/event-manager 1.1.0
doctrine/inflector 1.3.1
doctrine/instantiator 1.3.0
doctrine/lexer 1.2.0
doctrine/migrations 2.2.1
doctrine/mongodb-odm 2.0.5
doctrine/mongodb-odm-bundle 4.1.0
doctrine/orm v2.7.1
doctrine/persistence 1.3.6
doctrine/reflection v1.1.0
@ostrolucky
Copy link
Member

I don't see any DoctrineBundle issue here, moving to ORM

@ostrolucky ostrolucky transferred this issue from doctrine/DoctrineBundle Feb 19, 2020
@beberlei beberlei added the Bug label Feb 19, 2020
@beberlei
Copy link
Member

@bizley you are wrong though, it is still possible to set the property in __construct or everywhere else, its not possible to access it if its not initialized. We probably need to add guards in __sleep.

However why is sleep called anywys, please provide the whole stack trace for the error.

@bizley
Copy link
Author

bizley commented Feb 19, 2020

I'm not saying it's not possible - I'm saying that it's triggering fatal error for proxy object, and yes, for the reason you mentioned. I just don't understand why it's suddenly happening in 7.4.2.
I will provide stack trace in few minutes, need to recreate the environment.

@bizley
Copy link
Author

bizley commented Feb 19, 2020

  Behat\Testwork\Call\Exception\FatalThrowableError: Fatal error: Typed property Proxies\__CG__\App\Entity\OrderTask::$ must not be accessed before initialization (in __sleep) in vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:626
  Stack trace:
  #0 vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php(626): serialize()
  #1 vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php(327): Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->getCacheKey()
  #2 vendor/api-platform/core/src/Serializer/AbstractItemNormalizer.php(222): Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize()
  #3 vendor/api-platform/core/src/Serializer/ItemNormalizer.php(69): ApiPlatform\Core\Serializer\AbstractItemNormalizer->denormalize()
  #4 vendor/symfony/serializer/Serializer.php(191): ApiPlatform\Core\Serializer\ItemNormalizer->denormalize()
  #5 vendor/symfony/serializer/Serializer.php(142): Symfony\Component\Serializer\Serializer->denormalize()
  #6 vendor/api-platform/core/src/EventListener/DeserializeListener.php(106): Symfony\Component\Serializer\Serializer->deserialize()
  #7 vendor/symfony/event-dispatcher/EventDispatcher.php(304): ApiPlatform\Core\EventListener\DeserializeListener->onKernelRequest()
  #8 vendor/symfony/event-dispatcher/EventDispatcher.php(264): Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}()
  #9 vendor/symfony/event-dispatcher/EventDispatcher.php(239): Symfony\Component\EventDispatcher\EventDispatcher->doDispatch()
  #10 vendor/symfony/event-dispatcher/EventDispatcher.php(73): Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
  #11 vendor/symfony/http-kernel/HttpKernel.php(122): Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
  #12 vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
  #13 vendor/symfony/http-kernel/Kernel.php(201): Symfony\Component\HttpKernel\HttpKernel->handle()
  #14 vendor/symfony/http-kernel/Client.php(65): Symfony\Component\HttpKernel\Kernel->handle()
  #15 vendor/symfony/framework-bundle/Client.php(131): Symfony\Component\HttpKernel\Client->doRequest()
  #16 vendor/symfony/browser-kit/Client.php(404): Symfony\Bundle\FrameworkBundle\Client->doRequest()
  #17 vendor/behatch/contexts/src/HttpCall/Request/BrowserKit.php(71): Symfony\Component\BrowserKit\Client->request()
  #18 [internal function]: Behatch\HttpCall\Request\BrowserKit->send()
  #19 vendor/behatch/contexts/src/HttpCall/Request.php(31): call_user_func_array()
  #20 vendor/behatch/contexts/src/Context/RestContext.php(34): Behatch\HttpCall\Request->__call()
  #21 tests/Functional/Features/Context/RestContext.php(83): Behatch\Context\RestContext->iSendARequestTo()
  #22 [internal function]: App\Tests\Functional\Features\Context\RestContext->iSendARequestToWithBody()
  #23 vendor/behat/behat/src/Behat/Testwork/Call/Handler/RuntimeCallHandler.php(109): call_user_func_array()
  #24 vendor/behat/behat/src/Behat/Testwork/Call/Handler/RuntimeCallHandler.php(64): Behat\Testwork\Call\Handler\RuntimeCallHandler->executeCall()
  #25 vendor/behat/behat/src/Behat/Testwork/Call/CallCenter.php(140): Behat\Testwork\Call\Handler\RuntimeCallHandler->handleCall()
  #26 vendor/behat/behat/src/Behat/Testwork/Call/CallCenter.php(96): Behat\Testwork\Call\CallCenter->handleCall()
  #27 vendor/behat/behat/src/Behat/Behat/Tester/Runtime/RuntimeStepTester.php(125): Behat\Testwork\Call\CallCenter->makeCall()
  #28 vendor/behat/behat/src/Behat/Behat/Tester/Runtime/RuntimeStepTester.php(73): Behat\Behat\Tester\Runtime\RuntimeStepTester->testDefinition()
  #29 vendor/behat/behat/src/Behat/Behat/Hook/Tester/HookableStepTester.php(74): Behat\Behat\Tester\Runtime\RuntimeStepTester->test()
  #30 vendor/behat/behat/src/Behat/Behat/EventDispatcher/Tester/EventDispatchingStepTester.php(84): Behat\Behat\Hook\Tester\HookableStepTester->test()
  #31 vendor/behat/behat/src/Behat/Behat/Tester/StepContainerTester.php(59): Behat\Behat\EventDispatcher\Tester\EventDispatchingStepTester->test()
  #32 vendor/behat/behat/src/Behat/Behat/Tester/Runtime/RuntimeScenarioTester.php(76): Behat\Behat\Tester\StepContainerTester->test()
  #33 vendor/behat/behat/src/Behat/Behat/Hook/Tester/HookableScenarioTester.php(74): Behat\Behat\Tester\Runtime\RuntimeScenarioTester->test()
  #34 vendor/behat/behat/src/Behat/Behat/EventDispatcher/Tester/EventDispatchingScenarioTester.php(114): Behat\Behat\Hook\Tester\HookableScenarioTester->test()
  #35 vendor/behat/behat/src/Behat/Behat/Tester/Runtime/IsolatingScenarioTester.php(69): Behat\Behat\EventDispatcher\Tester\EventDispatchingScenarioTester->test()
  #36 vendor/behat/behat/src/Behat/Behat/Tester/Runtime/RuntimeFeatureTester.php(84): Behat\Behat\Tester\Runtime\IsolatingScenarioTester->test()
  #37 vendor/behat/behat/src/Behat/Behat/Hook/Tester/HookableFeatureTester.php(72): Behat\Behat\Tester\Runtime\RuntimeFeatureTester->test()
  #38 vendor/behat/behat/src/Behat/Behat/EventDispatcher/Tester/EventDispatchingFeatureTester.php(82): Behat\Behat\Hook\Tester\HookableFeatureTester->test()
  #39 vendor/behat/behat/src/Behat/Testwork/Tester/Runtime/RuntimeSuiteTester.php(63): Behat\Behat\EventDispatcher\Tester\EventDispatchingFeatureTester->test()
  #40 vendor/behat/behat/src/Behat/Testwork/Hook/Tester/HookableSuiteTester.php(73): Behat\Testwork\Tester\Runtime\RuntimeSuiteTester->test()
  #41 vendor/behat/behat/src/Behat/Testwork/EventDispatcher/Tester/EventDispatchingSuiteTester.php(83): Behat\Testwork\Hook\Tester\HookableSuiteTester->test()
  #42 vendor/behat/behat/src/Behat/Testwork/Tester/Runtime/RuntimeExercise.php(71): Behat\Testwork\EventDispatcher\Tester\EventDispatchingSuiteTester->test()
  #43 vendor/behat/behat/src/Behat/Testwork/EventDispatcher/Tester/EventDispatchingExercise.php(81): Behat\Testwork\Tester\Runtime\RuntimeExercise->test()
  #44 vendor/behat/behat/src/Behat/Testwork/Ordering/OrderedExercise.php(80): Behat\Testwork\EventDispatcher\Tester\EventDispatchingExercise->test()
  #45 vendor/behat/behat/src/Behat/Testwork/Tester/Cli/ExerciseController.php(149): Behat\Testwork\Ordering\OrderedExercise->test()
  #46 vendor/behat/behat/src/Behat/Testwork/Tester/Cli/ExerciseController.php(108): Behat\Testwork\Tester\Cli\ExerciseController->testSpecifications()
  #47 vendor/behat/behat/src/Behat/Testwork/Cli/Command.php(63): Behat\Testwork\Tester\Cli\ExerciseController->execute()
  #48 vendor/symfony/console/Command/Command.php(255): Behat\Testwork\Cli\Command->execute()
  #49 vendor/symfony/console/Application.php(1012): Symfony\Component\Console\Command\Command->run()
  #50 vendor/symfony/console/Application.php(272): Symfony\Component\Console\Application->doRunCommand()
  #51 vendor/behat/behat/src/Behat/Testwork/Cli/Application.php(124): Symfony\Component\Console\Application->doRun()
  #52 vendor/symfony/console/Application.php(148): Behat\Testwork\Cli\Application->doRun()
  #53 vendor/behat/behat/bin/behat(34): Symfony\Component\Console\Application->run()
  #54 {main}

Here is __sleep from Proxies\__CG__\App\Entity\OrderTask:

public function __sleep()
{
    if ($this->__isInitialized__) {
        return ['__isInitialized__', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'id', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'order', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'status', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'createdAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'promisedAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'completeAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'assignAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'activeAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'destinationAddress', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'courier', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'type', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'distance', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'isActive', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'endLatitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'endLongitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'startLatitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'startLongitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'endLocationAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'primaryEstimatedDistance', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'primaryEstimatedDuration', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimatedDuration', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimatedDistance', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'lastEstimationAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'ratio', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'courierCostGross', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'fuelCostNet', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'bonusCostGross', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'polyline', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimationRatio', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimationRatioDate', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'timeFrameStartAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'timeFrameEndAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'vehicle', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'serviceTime', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'orderTaskIssues', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'eta'];
    }

    return ['__isInitialized__', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'id', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'order', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'status', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'createdAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'promisedAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'completeAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'assignAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'activeAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'destinationAddress', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'courier', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'type', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'distance', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'isActive', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'endLatitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'endLongitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'startLatitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'startLongitude', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'endLocationAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'primaryEstimatedDistance', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'primaryEstimatedDuration', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimatedDuration', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimatedDistance', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'lastEstimationAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'ratio', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'courierCostGross', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'fuelCostNet', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'bonusCostGross', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'polyline', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimationRatio', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'estimationRatioDate', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'timeFrameStartAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'timeFrameEndAt', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'vehicle', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'serviceTime', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'orderTaskIssues', '' . "\0" . 'App\\Entity\\OrderTask' . "\0" . 'eta'];
}

@beberlei
Copy link
Member

@dunglas looping you in as this seems to be an interaction between APIPlatfoorm, Symfony Serializer and Doctrine entities.

@EmanuelOster
Copy link

I can confirm this. API Platform injects an object_to_populate into the $context variable during normalization for PATCH and PUT requests. This object is a doctrine entity and might have some doctrine proxy classes for relations.

A possible solution would be to change AbstractObjectNormalizers getCacheKey function and make it catch all Throwables instead of just Exceptions.

However it is still unclear to me which property is supposedly uninitialized in that proxy class. I couldn't find any upon inspection with xdebug

@ZhukV
Copy link

ZhukV commented Feb 20, 2020

Same problem (non initialized object) exist in UnitOfWork.

If the entity contain typed property with nullable flag, after persist and flush we receive error:

Error: Typed property Plutus\Context\Payment\Order\Model\Order::$completedAt must not be accessed before initialization

/code/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:634
/code/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:493
/code/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:807
/code/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:342
/code/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php:368
.....

It's can be fixed with next solution:

Index: vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php	(date 1582193964616)
+++ vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php	(date 1582193964616)
@@ -631,7 +631,12 @@
         $actualData = [];
 
         foreach ($class->reflFields as $name => $refProp) {
-            $value = $refProp->getValue($entity);
+            if (method_exists($refProp, 'isInitialized')) {
+                // Use PHP 7.4
+                $value = $refProp->isInitialized($entity) ? $refProp->getValue($entity) : null;
+            } else {
+                $value = $refProp->getValue($entity);
+            }
 
             if ($class->isCollectionValuedAssociation($name) && $value !== null) {
                 if ($value instanceof PersistentCollection) {

@ZhukV
Copy link

ZhukV commented Feb 20, 2020

In embedded properties (\Doctrine\ORM\Mapping\ReflectionEmbeddedProperty) we've same errors.
If property not initialized and we try to load entity we receive next error:

Error: Typed property Plutus\Context\Payment\Order\Model\Order::$money must not be accessed before initialization

/code/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php:89
/code/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:2685
/code/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php:146
/code/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php:68
/code/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php:153
/code/vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:726
/code/vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:736
/code/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php:469
....

The property $money is embedded.

But in this case we've a critical bug, because this property can't contain null value by default (value set from constructor) and we can't assign null by default.

@bizley
Copy link
Author

bizley commented Feb 20, 2020

Ok, now I understand why it's happening in PHP 7.4.2 - https://bugs.php.net/bug.php?id=79002

Turns out serialize with __sleep was behaving incorrectly before and allowing objects with uninitialized properties to be serialized (unserialize() was throwing TypeError then) and it was fixed in 7.4.2 (https://www.php.net/ChangeLog-7.php).

Right now I see 3 possible solutions:

  1. To catch Error in AbstractObjectNormalizer::getCacheKey (like @EmanuelOster suggested) and return false there since object with uninitialized property can not be serialized anyway).
  2. To tell everyone to initialize immediately every entity's typed property.
  3. To change doctrine proxy to work around that (I don't know how, maybe add checks like ReflectionProperty::isInitialized).

@beberlei
Copy link
Member

@ZhukV are you using the latest versions of doctrine/persistence (1.3.6) and doctrine/reflection (1.1)? Your bug should have been fixed with them.

@beberlei beberlei added this to the 2.7.2 milestone Feb 20, 2020
@greedyivan
Copy link

Do not use private properties in entities: Projects/ORM/Documentation/Architecture#Serializing entities. Doctrine does not have full support for that.

If you intend to serialize (and unserialize) entity instances that still hold references to proxy objects you may run into problems with private properties because of technical limitations. Proxy objects implement __sleep and it is not possible for __sleep to return names of private properties in parent classes.

@Ocramius
Copy link
Member

Ocramius commented Feb 22, 2020 via email

@ZhukV
Copy link

ZhukV commented Feb 22, 2020

@beberlei , thank! After update to latest versions all correct work.

  - Updating doctrine/reflection (v1.0.0 => v1.1.0): Loading from cache
  - Updating doctrine/persistence (1.3.3 => 1.3.6): Loading from cache

@beberlei
Copy link
Member

beberlei commented Mar 1, 2020

@bizley i would think option 2 is the correct one from my PoV, the other two are not really good and your initial bug report seems to be not true that it happens because of the constructor, but specificially from sleep.

@beberlei beberlei closed this as completed Mar 1, 2020
@bizley
Copy link
Author

bizley commented Mar 1, 2020

@beberlei thank you for looking at this case but I believe that you missed something here completely.

First of all, if you are referring to my list of possible solutions - number 2 is quite a joke that I hoped is obvious (apparently it is not). You can not initialize immediately every property in every possible case, and what is most important you can not possibly believe that you can force users to do that. Number 1 is a Symfony thing and I believe they should take care of it (as in "catch Error and not just Exception"). So I think number 3 is only viable solution (as in "fix sleep in proxies").

Second - I only described symptoms and the way to reproduce the problem in the bug description so yes - it is possible to live with current state of Doctrine and PHP 7.4.2. Still, the problem with sleep is there and should be fixed. I'm not sure what kind of a reporting policy is working here - should I open another ticket basically referring everything in this one?

I'm a bit disappointed how all this is handled right now.

@beberlei
Copy link
Member

beberlei commented Mar 1, 2020

@bizley I am a bit disappointed as well, that you provide a joke as an option and then feel offended when I pick it.

From the description I cannot see how this is a Doctrine bug yet. Maybe if you provide a reproducable test-case using EntityManager#getReference (creates a proxy) and serializing it will show the error. I only provide my free time here, I can't be responsible to invest hours understanding every bug report, especially when it doesn't come with a reproducible test case.

See https://github.com/doctrine/reflection/blob/master/phpunit.xml.dist#L12 how you can create a test in a special directory that only runs on a specific PHP version as this must be 7.4 only.

@bizley
Copy link
Author

bizley commented Mar 1, 2020

My apologies then. I'll try to work on the test case. And I think this issue should be moved to doctrine/common since this is where ProxyGenerator is placed.

@chrBrd
Copy link

chrBrd commented Mar 9, 2020

@bizley

This example given in your first comment:

private ?DateTimeInterface $property = null;

will delay the issue presenting itself, but as described in the comment by @EmanuelOster using PUT and PATCH will make it show up again.

Consequently, even though it was tongue-in-cheek, this won't actually work:

To tell everyone to initialize immediately every entity's typed property.

@bizley
Copy link
Author

bizley commented Mar 9, 2020

@chrBrd you are absolutely right. The problem is with proxy generator creating proxies with uninitialized typed properties in its __sleep and probably object normalizer not catching all errors.

@chrBrd
Copy link

chrBrd commented Mar 9, 2020

@bizley - looks like not entirely. While I agree with you this is an issue with the proxy generator, it turns out I hadn't quite initialized everything. I pasted your example above to save time, but the actual code snippet I'm looking into this with is:

/**
* @ORM\ManyToOne(targetEntity="App\Entity\Question", inversedBy="answers")
 */
private ?Question $question = null;

I noticed I was still getting the error with private $question = null;, so I had a look at the inverse:

/**
* @ORM\OneToMany(targetEntity="App\Entity\Answer", mappedBy="question", orphanRemoval=true)
*/
private Collection $answers;

Changing that to private ?Collection $answers = null; "fixes" it.

Immediately initializing every typed property is definitely more of a workaround than a fix though, especially as it necessitates setting null values on properties that shouldn't be nullable.

@beberlei, I completely understand why you want a reproducible test case for this, but I feel the issue may have been closed prematurely.

@bizley
Copy link
Author

bizley commented Mar 9, 2020

I was wondering why doctrine/common's ProxyLogicTypedPropertiesTest passes when there is at least one uninitialized typed property in tested object and its proxy is serialized in the test.

Turns out that every uninitialized field is actually being initialized in the setup phase of that test which is why this issue I've reported goes undetected. You can easily verify it by adding additional public string $test; in LazyLoadableObjectWithTypedProperties and running Doctrine\Tests\Common\Proxy\ProxyLogicTypedPropertiesTest::testNotInitializedProxyUnserialization. The result is then

Error: Typed property Doctrine\Tests\Common\ProxyProxy_CG_\Doctrine\Tests\Common\Proxy\LazyLoadableObjectWithTypedProperties::$test must not be accessed before initialization (in __sleep)

And please don't get me wrong, I know this test is to verify lazy loading and properties should be initialized but still I think it makes my point.

Will it be enough for reproducible test case @beberlei ? Once again sorry for sounding harsh before. You guys are doing great job, and I really appreciate it.

@someniatko
Copy link
Contributor

someniatko commented Mar 23, 2020

Hi there!

I get

 Error: Typed property MyNamespace\EmbeddableEntityClass::$value must not be accessed before initialization

with Embeddables when using doctrine/reflection 1.2.0. It's fine with 1.1.0.
tried doctrine/orm 2.7.1 and 2.7.2

@dunglas
Copy link
Contributor

dunglas commented Apr 3, 2020

Sorry for the late reply @beberlei. Thanks to @alanpoulain it's gonna be fixed in Symfony: symfony/symfony#36332

fabpot added a commit to symfony/symfony that referenced this issue Apr 4, 2020
…hen serializing context for the cache key (alanpoulain)

This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[Serializer] Fix unitialized properties (from PHP 7.4.2) when serializing context for the cache key

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35574 doctrine/orm#8030
| License       | MIT
| Doc PR        | N/A

This bug only happens on the following conditions:
- A Doctrine entity (`Book`) having a relation with another entity (`Author`) is used;
- The `Author` entity uses typed properties (PHP 7.4) not initialized;
- The `Serializer` is used with the `Book` in the `OBJECT_TO_POPULATE` key in the context.

For instance:
```php
<?php

declare(strict_types=1);

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;

/** @Orm\Entity */
class Book
{
    /**
     * @Orm\ManyToOne(targetEntity="Author")
     */
	public Author $author;

	public ?string $isbn;
}
```

```php
<?php

declare(strict_types=1);

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;

/** @Orm\Entity */
class Author
{
    public ?string $name;
}
```

Or even:

```php
<?php

declare(strict_types=1);

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;

/** @Orm\Entity */
class Author
{
    private string $name;

    public function __construct()
    {
        $this->name = 'Leo';
    }
}
```

If the following is done (it's the case for instance in API Platform when a `PUT` is made):
```php
$serializer->deserialize('{"isbn":"2038717141"}', Book::class, 'json', ['object_to_populate' => $book]);
```

Then there will be the following error:
> Fatal error: Typed property Proxies\__CG__\App\Entity\Author::$ must not be accessed before initialization (in __sleep)

It's because of these lines in the `getCacheKey` method of the `AbstractObjectNormalizer`:
https://github.com/symfony/symfony/blob/5da141b8d0ec7d71fb0ad637245833cd7fa56164/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php#L405-L409

Since the lazy proxyfied relation has a `__sleep` with unitialized properties, the `serialize` method will throw (since https://bugs.php.net/bug.php?id=79002: php/php-src@846b647).

I propose to fix this issue by unsetting the `OBJECT_TO_POPULATE` key in the context because I don't think it's useful for determining the attributes of the object.

For the next versions of Symfony, the fix should probably be elsewhere, in the default context.
For instance in Symfony 4.4, instead of:
https://github.com/symfony/symfony/blob/15edfd39d4b3da4d011a6242238c4b2874eb1ace/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php#L118
It should be:
```php
$this->defaultContext[self::EXCLUDE_FROM_CACHE_KEY] = [self::CIRCULAR_REFERENCE_LIMIT_COUNTERS, self::OBJECT_TO_POPULATE];
```
But I'm not sure how it should be merged (another PR maybe?).

Commits
-------

1fafff7 [Serializer] Fix unitialized properties (from PHP 7.4.2) when serializing context for the cache key
@berkut1
Copy link

berkut1 commented Jul 21, 2020

@dunglas It looks like the problem still here.

Typed property Class::$item must be an instance of Item, Id used

    /**
     *
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="NONE")
     * @ORM\OneToOne(targetEntity=Item::class, cascade={"persist", "remove"})
     * @ORM\JoinColumns({
     *   @ORM\JoinColumn(name="id_item", referencedColumnName="id_item", onDelete="CASCADE")
     * })
     */
    private Item $item;

I temporarily fixed by adding the Id field:

    /** 
     *    
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="NONE")
     * @ORM\Column(name="id_item", type="dcim_item_id", nullable=false)
     */
    private Id $id_item;

    /** 
     *
     * @ORM\OneToOne(targetEntity=Item::class, cascade={"persist", "remove"})
     * @ORM\JoinColumns({
     *   @ORM\JoinColumn(name="id_item", referencedColumnName="id_item", onDelete="CASCADE")
     * })
     */
    private Item $item;

Also the first version works if remove the Item typed property, like it is in the php 7.3

Symfony 4.4.10 with last packages.

@dunglas
Copy link
Contributor

dunglas commented Jul 21, 2020

My PR hasn't been merged in Symfony. Has you can see in the comments, not everybody agree that it's the proper fix.

fabpot added a commit to symfony/symfony that referenced this issue Aug 17, 2020
This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Serializer] Fix configuration of the cache key

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #35574 doctrine/orm#8030 (partially)
| License       | MIT
| Doc PR        | n/a

Currently, a bug prevents to configure the context keys to exclude from the cache key computation. The value is always replaced in the constructor. This PR fixes the problem.

Commits
-------

3b034cb [Serializer] Fix configuration of the cache key
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