merged branch kriswallsmith/doctrine/proxy-loader-fix (PR #3419)

Commits
-------

6e75fd1 Resolves issue with spl_autoload_register creating new copies of the container and passing that into the closure.

Discussion
----------

[DoctrineBundle] fixed proxy loader memory leak

[![Build Status](https://secure.travis-ci.org/kriswallsmith/symfony.png?branch=doctrine/proxy-loader-fix)](http://travis-ci.org/kriswallsmith/symfony)

The hack for loading Doctrine proxy classes has an obscure memory leak, fixed here by @jjbohn.

## The Proof

Run this test case before and after this patch:

```php
<?php

namespace Kris\JunkBundle\Tests\Controller;

use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;

class DefaultControllerTest extends WebTestCase
{
    /**
     * @dataProvider asdf
     */
    public function testIndex()
    {
        $client = static::createClient();

        $crawler = $client->request('GET', '/hello/Fabien');

        $this->assertTrue($crawler->filter('html:contains("Hello Fabien")')->count() > 0);
    }

    public function asdf()
    {
        return array_fill(0, 500, array());
    }
}
```

### Before

```
~/Sites/symfony/standard (2.0) $ phpunit -c app/
PHPUnit 3.6.10 by Sebastian Bergmann.

Configuration read from /Users/kriswallsmith/Sites/symfony/standard/app/phpunit.xml.dist

...............................................................  63 / 500 ( 12%)
............................................................... 126 / 500 ( 25%)
............................................................... 189 / 500 ( 37%)
............................................................... 252 / 500 ( 50%)
............................................................... 315 / 500 ( 63%)
............................................................... 378 / 500 ( 75%)
............................................................... 441 / 500 ( 88%)
...........................................................

Time: 31 seconds, Memory: 289.50Mb

OK (500 tests, 500 assertions)
```

### After

```
~/Sites/symfony/standard (2.0) $ phpunit -c app/
PHPUnit 3.6.10 by Sebastian Bergmann.

Configuration read from /Users/kriswallsmith/Sites/symfony/standard/app/phpunit.xml.dist

...............................................................  63 / 500 ( 12%)
............................................................... 126 / 500 ( 25%)
............................................................... 189 / 500 ( 37%)
............................................................... 252 / 500 ( 50%)
............................................................... 315 / 500 ( 63%)
............................................................... 378 / 500 ( 75%)
............................................................... 441 / 500 ( 88%)
...........................................................

Time: 40 seconds, Memory: 51.25Mb

OK (500 tests, 500 assertions)
```

## tl;dr

Your test suite will use much less memory — 82% in this case.

```
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
```

---------------------------------------------------------------------------

by mvrhov at 2012-02-23T06:25:57Z

IMHO this change warrants a comment inside a source code as somebody might actually try to remove the first by reference assign like stof said.

---------------------------------------------------------------------------

by lsmith77 at 2012-02-23T07:55:48Z

this autoloader sounds like something we also need in the ODM's?

---------------------------------------------------------------------------

by stof at 2012-02-23T08:23:17Z

@lsmith77 if you want to allow unserializing proxies without forcing to generate them before (which would be an issue in debug mode), yeah. But take care that each Doctrine bundle should use a different proxy namespace to allow doing the check (there was some issues for people using both the ORM and the mongo ODM because of this)

---------------------------------------------------------------------------

by lsmith77 at 2012-02-23T08:24:33Z

then maybe this could should be a static method inside the bridge?

---------------------------------------------------------------------------

by beberlei at 2012-02-23T11:50:08Z

I think another side of this problem is that ->boot() ALWAYS adds this method on the autoloading stack. So with N tests you have N more autoloaders on the stack.

---------------------------------------------------------------------------

by pminnieur at 2012-02-23T12:07:00Z

This could be an issue if you use Symfony with Leach as an application server, too. After a while, memory is exhausted in face of `gc_collect_cycles` and `$kernel->boot()` and `$kernel->shutdown()` calls in between each request - which ultimately leads to a segfault after some time. I tried to track down what causes increasing memory usage and I think this could be the error.

---------------------------------------------------------------------------

by beberlei at 2012-02-23T12:28:06Z

its definately the problem, we need to remove the autoloader in shutdown, or move it elsewhere.

---------------------------------------------------------------------------

by lsmith77 at 2012-02-23T14:58:37Z

why isnt this just a setup task for the autoloader just like the annotation registry?

---------------------------------------------------------------------------

by stof at 2012-02-23T16:52:42Z

@lsmith77 because the proxy namespace and the proxy dir are not known in the autoload.php file. They are configured in the config files

---------------------------------------------------------------------------

by fabpot at 2012-02-23T18:05:51Z

The `shutdown()` method is where the autoloader should be removed. Can we include this in this PR as well so that we fix everything once and for all?

---------------------------------------------------------------------------

by kriswallsmith at 2012-02-23T19:12:05Z

The once and for all solution is for the Doctrine O*M projects to provide a ProxyLoader class with register and unregister methods that we call in boot and shutdown. We're not solving anything specific to Symfony here.
This commit is contained in:
Fabien Potencier 2012-02-24 11:29:42 +01:00
commit 6af6531bfb

View File

@ -42,9 +42,9 @@ class DoctrineBundle extends Bundle
if ($this->container->hasParameter('doctrine.orm.proxy_namespace')) {
$namespace = $this->container->getParameter('doctrine.orm.proxy_namespace');
$dir = $this->container->getParameter('doctrine.orm.proxy_dir');
$container = $this->container;
$container =& $this->container;
spl_autoload_register(function($class) use ($namespace, $dir, $container) {
spl_autoload_register(function($class) use ($namespace, $dir, &$container) {
if (0 === strpos($class, $namespace)) {
$className = substr($class, strlen($namespace) +1);
$file = $dir.DIRECTORY_SEPARATOR.$className.'.php';