Skip to content

Commit

Permalink
bug #32421 [EventDispatcher] Add tag kernel.rest on 'debug.event_disp…
Browse files Browse the repository at this point in the history
…atcher' service (lyrixx)

This PR was submitted for the 4.4 branch but it was merged into the 3.4 branch instead (closes #32421).

Discussion
----------

[EventDispatcher] Add tag kernel.rest on 'debug.event_dispatcher' service

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? |
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

In CLI, Symfony leaks because it uses the TraceableEventDispatcher.
Let's make this service resetable.

This PR is related to #32418

---

Side note: Forcing user to use the `ServicesResetter` is IMHO bad for
the DX. I prefer to have something that does not leak by default.
More over, the TraceableEventDispatcher stores some informations for the
profiler. But in CLI it does not make sens, since the tooling does not
exist. I have already fixed such issue for monolog in #30339. I think we
should do the same for the TraceableEventDispatcher.

---

Note: When using #32418 + this PR and with the following code, it does not leak **at all**

<details>
<summary>code</summary>

```php
<?php

namespace App\Command;

use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\EventDispatcher\Event;
use Symfony\Component\HttpKernel\DependencyInjection\ServicesResetter;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;

class LeakCommand extends Command
{
    protected static $defaultName = 'leak';

    private $dispatcher;
    private $resetter;
    private $logger;

    public function __construct(EventDispatcherInterface $dispatcher, ServicesResetter $resetter, LoggerInterface $logger)
    {
        $this->dispatcher = $dispatcher;
        $this->resetter = $resetter;
        $this->logger = $logger;

        parent::__construct();
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $this->dispatcher->addListener(MyEvent::class, function ($e) {

        });
        for ($i=0; $i < INF; $i++) {
            $output->writeln(memory_get_usage());

            $this->dispatcher->dispatch(new MyEvent());
            $this->logger->debug('some log');

            // if ($i > 2000) {
            //     meminfo_dump(fopen('/tmp/my_dump_file.json', 'w'));
            //     die;
            // }

            usleep(100);
            $this->resetter->reset();
        }
    }
}

class MyEvent {}

```
</detail>

Commits
-------

5249eaf [EventDispatcher] Add tag kernel.rest on 'debug.event_dispatcher' service
  • Loading branch information
fabpot committed Jul 8, 2019
2 parents f2d7170 + 5249eaf commit d92c9b8
Showing 1 changed file with 1 addition and 0 deletions.
Expand Up @@ -9,6 +9,7 @@

<service id="debug.event_dispatcher" class="Symfony\Component\HttpKernel\Debug\TraceableEventDispatcher" decorates="event_dispatcher">
<tag name="monolog.logger" channel="event" />
<tag name="kernel.reset" method="reset" />
<argument type="service" id="debug.event_dispatcher.inner" />
<argument type="service" id="debug.stopwatch" />
<argument type="service" id="logger" on-invalid="null" />
Expand Down

0 comments on commit d92c9b8

Please sign in to comment.