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

Typed property must not be accessed before initialization #7944

Closed
oojacoboo opened this issue Dec 5, 2019 · 21 comments
Closed

Typed property must not be accessed before initialization #7944

oojacoboo opened this issue Dec 5, 2019 · 21 comments

Comments

@oojacoboo
Copy link
Contributor

Bug Report

Q A
BC Break no
Version 2.7.0

Summary

Typed property Foo::$id must not be accessed before initialization

doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:634

Seems to be an issue with reflection attempting to access typed properties.

@Ocramius
Copy link
Member

Ocramius commented Dec 5, 2019

This is relatively normal: the UnitOfWork will almost always try to access properties of entities, so if you declared a property such as private ?int $id;, if you don't want it to be set at __construct, it must have a default value through private ?int $id = null;

@oojacoboo
Copy link
Contributor Author

oojacoboo commented Dec 5, 2019

That totally makes sense. However... what's the recommended route forward here? Obviously you don't want the $id (PK) to be nullable. And, when calling new/instantiating the object, you generally won't know the correct value. I guess you could set it to zero, but that's a hack. Also, is this only going to be an issue with the PK/IDENTITY for Entities with typed properties?

@malarzm
Copy link
Member

malarzm commented Dec 5, 2019

@oojacoboo but technically $id is nullable when using any kind of auto increment identifier - its value is literally null before UnitOfWork gets the value from DB and assigns it back to the entity. I'd say it's perfect time to preach using uuid as an identifier :)

@oojacoboo
Copy link
Contributor Author

Yea, had a good discussion on Slack about this. I suspect that many other people will run into this issue as PHP 7.4 begins to be adopted more and people start typing their properties.

We'll just use protected ?int $id = null. This has a fringe benefit in allowing you to identify if the Entity has been persisted and makes sense for a default value with auto-increment on most RDBMSes.

@pfazzi
Copy link

pfazzi commented Dec 6, 2019

I have the same issue, ATM i'm adopting the private ?int $id = null; strategy.

@kyrrr
Copy link

kyrrr commented Jan 7, 2020

Would be nice to have a different solution than setting nullables everywhere.

@oojacoboo
Copy link
Contributor Author

@kyrrr it's totally fixable. There is an open pull request related to this, but it seems more needs to be done on it and there hasn't been much activity recently.

#7857

@beberlei
Copy link
Member

Updating doctrine/persistence to 1.3.6 will fix this issue.

@leongersen
Copy link

Unfortunately, even with the updates to doctrine/persistence, ids must be nullable to be able to remove the entity, as the UnitOfWork will be setting the id value back to null in \Doctrine\ORM\UnitOfWork::executeDeletions:

// Entity with this $oid after deletion treated as NEW, even if the $oid
// is obtained by a new entity because the old one went out of scope.
//$this->entityStates[$oid] = self::STATE_NEW;
if ( ! $class->isIdentifierNatural()) {
	$class->reflFields[$class->identifier[0]]->setValue($entity, null);
}

I've filed this issue: #7999

@artyom-wcd
Copy link

artyom-wcd commented Mar 6, 2020

using this: "name": "doctrine/persistence", "version": "1.3.6",
but the problem (as described before) is still there when trying to flush

@oojacoboo
Copy link
Contributor Author

Can the property just be unset or, by using reflection, check if nullable and optionally set as null?

Deletion is an interesting scenario. Technically, unless using soft-delete, that object shouldn't really exist anymore, and certainly not the ID. I can see how it'd be useful to use an object after deletion though.

@yapro
Copy link

yapro commented Mar 19, 2020

My error:

"Typed property Proxies\\__CG__\\App\\Entity\\Organization::$ must not be accessed before initialization (in __sleep)"

My solution - add next method to the class:

public function __sleep()
{
    return [];
}

@VaN-dev
Copy link

VaN-dev commented Jun 23, 2020

Updating doctrine/persistence to 1.3.6 will fix this issue.

it doesn't

@Amunak
Copy link

Amunak commented Aug 18, 2020

What about properties where you can't set the default value, like in associations that use collection? Object cannot be a default value, and nullable makes no sense here.

This example:

class Entity {
    	/**
	 * @ORM\OneToMany(targetEntity="Other")
	 * @var Collection|Other[]
	 */
	protected $friends;
	public function __construct() {
		$this->friends = new ArrayCollection();
	}
}

used to be the proper way to initialize collections, but now if you add PHP7.4 property type hints (protected Collection $friends;) it no longer works, as Doctrine accesses the property through reflection but doesn't run the constructor and then you get the Fatal Error.

And that's not the only example, what about custom Doctrine types that use some object to hold the value?

@Ocramius
Copy link
Member

Ocramius commented Aug 18, 2020

but now if you add PHP7.4 property type hints (protected Collection $friends;) it no longer works, as Doctrine accesses the property through reflection but doesn't run the constructor and then you get the Fatal Error.

Proxy initialization does set collections?

What doesn't work (and also didn't work in the past) in the 2.x series is "friend objects", but otherwise proxy initialization does populate their associations.

This is the broken behavior (also pre-php-7.4):

class Thing
{
    private $id;
    private $lazyField;
    function equals(self $other) {
        return $other->lazyField->equals($this->lazyField); // call to `equals` on `null` happening here
    }
}

@oojacoboo
Copy link
Contributor Author

oojacoboo commented Aug 18, 2020

@Ocramius That's partially right. But, you don't need the call to equals there. You only need the call on lazyField, which is the issue. It's of particular concern for auto incremented IDs. You make it sound like this has always been an issue and it's not an important one.

@Amunak
Copy link

Amunak commented Aug 18, 2020

Proxy initialization does set collections?

Interesting, for some reason I have this issue only in our Jenkins test build but can't replicate it locally. I'll investigate further.

@Ocramius
Copy link
Member

@Ocramius That's partially right. But, you don't need the call to equals there. You only need the call on lazyField, which is the issue. It's of particular concern for auto incremented IDs. You make it sound like this has always been an issue and it's not an important one.

@oojacoboo yes, I made the example to show that the method call will lead to a crash: much harder to notice if I used === there :)

@Amunak
Copy link

Amunak commented Aug 19, 2020

Alright, investigation concluded; when clearing cache on Jenkins no APP_ENV was defined so it cleared the default, but we needed other envs cleared. This lead to old metadata being used from cache and in those Doctrine didn't even know about some of the fields, skipping the initialization.

Sorry, my mistake.

@Bguignard
Copy link

Bguignard commented Jan 29, 2021

I had exactly the same problem, only happening on the web server and not in local environment, and the cause was :

  • I had objects in the Symfony session
  • I copied (cloned) one object from the session in a controller to fill it in order to send it to the template
  • Since cloning in PHP is simply a shallow and not a deep copy, all attribute which are also objects are NOT copied but only passed by reference to the new clone.
  • Since Doctrine attribute of One to Many collections are ArrayCollection and not array, they are passed by reference when the owning object is cloned.
  • Since it uses the __sleep() method when its taken from the session, when I tried to set this collection in the clonned object, this ArrayCollection was not initialized which caused the error.

The worst thing was, after triggering the error, I had to delete the session manually in the server in the /var/lib/php/sessions folder because Symfony couldn't access to the session anymore which completely stuck the website for the current browser.

I first tried to "override" the __sleep() method as answered before, but even if the error was not triggered, my attribute was empty and that's not what I wanted.

My solution was to clone, then empty the collection with a ->clear() method after cloning the object, then settting it with the add() method.

I think the heart of the problem is this __sleep() method when objects with ArrayCollection attributes are cloned from the session.

I still don't understand why the bug didn't happen with php 7.4.9 and 7.4.1 in a local environment but happened with 7.4.3 on the server...

The error is the same as described here but they closed it : symfony/symfony#35574

tomudding added a commit to GEWIS/gewisweb that referenced this issue Jul 28, 2021
tomudding added a commit to GEWIS/gewisweb that referenced this issue Aug 20, 2021
Koen1999 pushed a commit to Koen1999/gewisweb that referenced this issue Aug 31, 2021
tomudding added a commit to GEWIS/gewisweb that referenced this issue Sep 17, 2021
@cbichis
Copy link

cbichis commented Dec 15, 2021

I am getting the same issue with quite new libraries...

doctrine/annotations 1.13.2
doctrine/cache 1.12.1
doctrine/collections 1.6.8
doctrine/common 3.2.0
doctrine/dbal 2.13.6
doctrine/deprecations v0.5.3
doctrine/doctrine-laminas-hydrator 2.2.1
doctrine/doctrine-module 4.4.0
doctrine/doctrine-orm-module 4.2.x-dev 26f2270
doctrine/event-manager 1.1.1
doctrine/inflector 2.0.4
doctrine/instantiator 1.4.0
doctrine/lexer 1.2.1
doctrine/migrations 3.3.2
doctrine/orm 2.10.3
doctrine/persistence 2.2.3

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