mezzio / mezzio-swoole

Swoole support for Mezzio
https://docs.mezzio.dev/mezzio-swoole/
BSD 3-Clause "New" or "Revised" License
89 stars 28 forks source link

TaskEventDispatchListener DI loop #133

Open kirkmadera opened 4 months ago

kirkmadera commented 4 months ago

Bug Report

I am following the recommended way to run a task event listener, using a TaskEventDispatchListener, but have run into a DI loop.

Mezzio\Swoole\Task\TaskEventDispatchListener > Mezzio\Swoole\Event\EventDispatcherInterface > Mezzio\Swoole\Event\SwooleListenerProvider > Mezzio\Swoole\Task\TaskEventDispatchListener.

Q A
Version(s) 4.9.0

Summary

The following config results in memory exhaustion due to a circular dependency in DI.

<?php

return [
    'mezzio-swoole' => [
        'swoole-http-server' => [
            'listeners' => [
                Event\TaskEvent::class => [
                    TaskEventDispatchListener::class,
                ],
            ],
        ],
    ],
];

Current behavior

Memory exhaustion due to circular dependency.

  1. \Mezzio\Swoole\Event\EventDispatcherInterface requested from container
  2. \Mezzio\Swoole\Event\EventDispatcherFactory runs
  3. Gets \Mezzio\Swoole\Event\SwooleListenerProvider from the container
  4. \Mezzio\Swoole\Event\SwooleListenerProviderFactory runs
  5. Gets configured listeners
  6. One of the listeners is \Mezzio\Swoole\Task\TaskEventDispatchListener
  7. \Mezzio\Swoole\Task\TaskEventDispatchListenerFactory runs
  8. Gets Mezzio\Swoole\Event\EventDispatcherInterface from container

How to reproduce

Add this configuration, then attempt to get \Mezzio\Swoole\Event\EventDispatcherInterface from the container.

<?php

return [
    'mezzio-swoole' => [
        'swoole-http-server' => [
            'listeners' => [
                Event\TaskEvent::class => [
                    TaskEventDispatchListener::class,
                ],
            ],
        ],
    ],
];

<?php

/** @var Laminas\ServiceManager\ServiceManager $container */
$eventDispatcher = $container->get(\Mezzio\Swoole\Event\EventDispatcherInterface::class);

#### Expected behavior

$eventDispatcher returned
kirkmadera commented 4 months ago

A Swoole event listener that requires the Swoole Event Dispatcher probably needs to implement EventDispatcherAwareInterface and have it set in \Mezzio\Swoole\Event\EventDispatcher::__construct looping through the provided listeners. Or lazier, in the \Mezzio\Swoole\Event\EventDispatcher::dispatch once the listener is identified for use.

kirkmadera commented 4 months ago

This may be an untested path in Mezzio and be functional in the Phly codebase only. Can you confirm if this is tested to be working and if I am doing something wrong? It seems like an unavoidable circular dependency that makes this idea impossible to implement in its current state.

kirkmadera commented 4 months ago

I realized that it makes more sense to have a separate TaskEventDispatcher rather than reusing the Mezzio\Swoole\Event\EventDispatcherInterface. I also found this in Mezzio\Swoole\Task\TaskEventDispatchListenerFactory.

$dispatcherService = $config['mezzio-swoole']['task-dispatcher-service'] ?? EventDispatcherInterface::class;

If this is meant to be a separate event dispatcher, then my issue was caused by the fact that the docs don't state to configure a separate Task event dispatcher and the default behavior is to use Mezzio\Swoole\Event\EventDispatcher.