[FrameworkBundle] Memory leak in 4.4.41

[pmishev]

Symfony version(s) affected

4.4.41

Description

Upgrading from 4.4.40 to 4.4.41 introduces a memory leak.

How to reproduce

Have a Command with:

<?php

namespace App\Command;

use App\Entity\Product;
use Doctrine\ORM\AbstractQuery;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Query;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class TestCommand extends Command
{
    protected static $defaultName = 'app:test';

    private EntityManagerInterface $em;

    public function __construct(EntityManagerInterface $em)
    {
        parent::__construct();

        $this->em = $em;
    }

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        $repo = $this->em->getRepository(Product::class);
        $qb = $repo->createQueryBuilder('t')->orderBy('t.id', 'ASC');

        $batchSize = 5000;
        $offset = 0;
        $qb->setMaxResults($batchSize);
        do {
            $qb->setFirstResult($offset);
            $records = $qb->getQuery()->getResult(AbstractQuery::HYDRATE_ARRAY);
            foreach ($records as $record) {
                $qb1 = $repo->createQueryBuilder('t')->where('t.id = :id')->setParameter('id', $record['id']);
                $qb1->getQuery()->getSingleResult(Query::HYDRATE_ARRAY);
            }
            dump(sprintf('Memory used: %u MB', memory_get_usage(true) / 1048576));
            $offset += $batchSize;
        } while (!empty($records));

        return 0;
    }
}

Run it:

bin/console app:test --no-debug

Output with "symfony/framework-bundle": "4.4.40":

^ "Memory used: 40 MB"
^ "Memory used: 56 MB"
^ "Memory used: 56 MB"
^ "Memory used: 56 MB"
^ "Memory used: 56 MB"
^ "Memory used: 56 MB"
^ "Memory used: 56 MB"
^ "Memory used: 58 MB"
^ "Memory used: 56 MB"
^ "Memory used: 58 MB"
^ "Memory used: 56 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"
^ "Memory used: 60 MB"
^ "Memory used: 58 MB"
^ "Memory used: 58 MB"

Output with "symfony/framework-bundle": "4.4.41":

^ "Memory used: 42 MB"
^ "Memory used: 56 MB"
^ "Memory used: 60 MB"
^ "Memory used: 62 MB"
^ "Memory used: 66 MB"
^ "Memory used: 70 MB"
^ "Memory used: 74 MB"
^ "Memory used: 76 MB"
^ "Memory used: 78 MB"
^ "Memory used: 82 MB"
^ "Memory used: 86 MB"
^ "Memory used: 88 MB"
^ "Memory used: 92 MB"
^ "Memory used: 96 MB"
^ "Memory used: 100 MB"
^ "Memory used: 102 MB"
^ "Memory used: 106 MB"
^ "Memory used: 108 MB"
^ "Memory used: 112 MB"
^ "Memory used: 114 MB"
^ "Memory used: 118 MB"
^ "Memory used: 120 MB"
^ "Memory used: 124 MB"
^ "Memory used: 126 MB"
^ "Memory used: 128 MB"
^ "Memory used: 128 MB"

Possible Solution

No response

Additional Context

No response

[xabbuh]

Can you create a small example application that allows to reproduce your issue?

[GromNaN]

This leaks looks related to Doctrine. Blackfire may help you to track what leaks.
The details of the release is here: #46192

[derrabus]

This leaks looks related to Doctrine.

Indeed. Are you sure, FrameworkBundle is the only package that you've updated?

[pmishev]

I am sure. This is the only package updated that makes the difference. I've been trying to trace the problem for more than 2 days after my last composer update, package by package and I had to rm -rf var/cache/dev/* every time, because even after cache:clear something was remaining and I was getting inconsistent results.

I edited my issue report with the full Command code if that helps. This is as minimal as I could get it to clearly show the problem. Just put it in any Sf4.4 project and change the entity to something with lots of DB records (I've run it against a table of ~100,000 records). I'm not sure what more to do to make it more easily reproducible.

[pmishev]

This leaks looks related to Doctrine. Blackfire may help you to track what leaks. The details of the release is here: #46192

This is actually the release: symfony/framework-bundle@v4.4.40...v4.4.41

[derrabus]

I'm not sure what more to do to make it more easily reproducible.

see https://symfony.com/doc/current/contributing/code/reproducer.html#reproducing-complex-bugs

[pmishev]

I went through the changes in this release and I can further confirm that the problem comes from this: https://github.com/symfony/symfony/pull/46125/files

[derrabus]

cc @fancyweb

[fancyweb]

Your command generates thousands and thousands of calls to the cache (through Doctrine result and query cache) and all those calls are now traced by TraceableAdapter which creates an object for every one of them and appends them to an array that fills the memory. This collector was an exception before my patch since it was also tied to the debug mode being on. That's why it was working.

The profiler/data collectors will always have a memory problem in those situations where a large number of things are traced. They can't collect an enormous amount of data and at the same time not consume memory.

IMHO, in your situation, the profiler should be disabled for batch processing. You could just create another env for that to test the command in "dev" mode (eg: "dev-no-profiler").

I understand it creates frustration since the change was merged on 4.4. However, I'm still in favor of keeping my patch on 4.4 since it realigns the behavior of all collector passes and IMHO, the real problem here is using the profiler/data collectors with code that overloads them while it's a known limit. If other core team members and users think it's a legitimate regression, let's revert and reapply the patch on 6.2? 😬

[lyrixx]

We have the same issue with the event dispatcher #32422

IMHO, we should do something in symfony, like adding a hardlimit. We already do that in the Dumper to avoid dumping to much element (cut stub)

[derrabus]

let's revert and reapply the patch on 6.2? 😬

I would be in favor of that approach since 4.4 is nearing EOL and should be kept super-stable.

[chalasr]

I agree. #46442

[pmishev]

I welcome your decision to revert this, as it is a nasty breaking change, albeit just in dev.

As for adding it in general, I think there should be an easy way to be able to execute long-running scripts in dev without the profiler.
Adding a new env like "dev-no-profiler" as suggested doesn't sound easy enough to me.
UX-wise, turning off the profiler with --no-debug makes sense to me, as the profiler is meant for debug, isn't it?

If not, you should at least be able to turn it off based on an env variable, which I'm not sure this is possible at the moment:

This doesn't seem to have any effect (memory is still being consumed until it's over):

framework:
    profiler:
        collect: false

And this gives me You have requested a non-existent service "profiler".:

framework:
    profiler:
        enabled: false

[stof]

If not, you should at least be able to turn it off based on an env variable, which I'm not sure this is possible at the moment

This is not possible, because what needs to be done to reduce memory usage is to remove the Traceable decorators from the container setup, which is a build-time change, not a runtime change.