DDC-1597: ClassMetadata::newInstance() potentially breaks when entities implement __clone() #2004

New Issue

admin · 2026-01-22T13:37:16+01:00

admin commented

2026-01-22 13:37:16 +01:00

Originally created by @doctrinebot on GitHub (Jan 11, 2012).

Originally assigned to: @beberlei on GitHub.

Jira issue originally created by user benjamin:

The ClassMetadata::newInstance() method caches a prototype of the entity to hydrate, to speedup instantiation. The problem is that when an entity implements **clone(), it might break some valid code (that has not been written with Doctrine internals in mind):

class Order {
    protected $orderLines;

    public function **construct() {
        $this->orderLines = new ArrayCollection();
    }

    public function **clone() {
        $clonedOrderLines = new ArrayCollection();
        foreach ($this->orderLines as $orderLine) {
            $clonedOrderLines[] = clone $orderLine;
        }
        $this->orderLines = $clonedOrderLines;
    }
}

This code is perfectly valid... as long as we're not trying to retrieve this persisted entity with Doctrine.
If we do, then ClassMetadata::newInstance() will call **clone(), which will return an empty Order object with $orderLines = null, causing an "Invalid argument supplied for foreach()".

Of course there is a workaround for this: always check if a property is not null in **clone(), but that would mean adapting some legitimate PHP OOP code to make it work with Doctrine, which I believe is a bad thing.

Suggested fix: don't cache a prototype of the entity:

public function newInstance()
{
    return unserialize(sprintf('O:%d:"%s":0:{}', strlen($this->name), $this->name));
}

According to my benchmarks, this is 3x slower than the original version (which is running at 1 million instantiations / second on my 3-years old development machine, do we need this speed?). We can even lower this speed impact by caching only the sprintf() result in the constructor, which then runs only 2x slower than the original code:

public function **construct(...) {
    ...
    $this->serializedPrototype = sprintf('O:%d:"%s":0:{}', strlen($this->name), $this->name);
}
public function newInstance()
{
    return unserialize($this->serializedPrototype);
}

Originally created by @doctrinebot on GitHub (Jan 11, 2012). Originally assigned to: @beberlei on GitHub. Jira issue originally created by user benjamin: The ClassMetadata::newInstance() method caches a prototype of the entity to hydrate, to speedup instantiation. The problem is that when an entity implements **clone(), it might break some valid code (that has not been written with Doctrine internals in mind): ``` class Order { protected $orderLines; public function **construct() { $this->orderLines = new ArrayCollection(); } public function **clone() { $clonedOrderLines = new ArrayCollection(); foreach ($this->orderLines as $orderLine) { $clonedOrderLines[] = clone $orderLine; } $this->orderLines = $clonedOrderLines; } } ``` This code is perfectly valid... as long as we're not trying to retrieve this persisted entity with Doctrine. If we do, then ClassMetadata::newInstance() will call **clone(), which will return an empty Order object with $orderLines = null, causing an "Invalid argument supplied for foreach()". Of course there is a workaround for this: always check if a property is not null in **clone(), but that would mean adapting some legitimate PHP OOP code to make it work with Doctrine, which I believe is a bad thing. Suggested fix: don't cache a prototype of the entity: ``` public function newInstance() { return unserialize(sprintf('O:%d:"%s":0:{}', strlen($this->name), $this->name)); } ``` According to my benchmarks, this is 3x slower than the original version (which is running at 1 million instantiations / second on my 3-years old development machine, do we need this speed?). We can even lower this speed impact by caching only the sprintf() result in the constructor, which then runs only 2x slower than the original code: ``` public function **construct(...) { ... $this->serializedPrototype = sprintf('O:%d:"%s":0:{}', strlen($this->name), $this->name); } public function newInstance() { return unserialize($this->serializedPrototype); } ```

admin added the Bug label 2026-01-22 13:37:16 +01:00

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#2004