Fix memory leak: install module logger before the application is built

src/Codeception/Lib/Connector/Yii2.php

@@ -135,6 +135,79 @@ final class Yii2 extends Client
      */
     private array $emails = [];
 
+    /**
+     * Whether an application is currently being started or already started and not yet reset.
+     * While this is true, `yii\log` shutdown registrations are stored (see
+     * `registerShutdown()`) instead of being registered for real: a real shutdown closure
+     * bound to a logger keeps the logger — and with it its dispatcher, log targets and the
+     * application they hold — alive until process exit, even though the application is
+     * discarded after every test.
+     *
+     * @internal
+     */
+    private static bool $appStarting = false;
+
+    /**
+     * Shutdown registrations captured in the `yii\log` namespace while an application is
+     * being started. `yii\log\Logger::init()` registers two of them per logger (a closure
+     * bound to the logger and `[$this, 'flush']` with `true`); they are run by
+     * `resetApplication()` instead of surviving to process exit.
+     *
+     * @internal
+     *
+     * @var list<array{0: callable, 1: list<mixed>}>
+     */
+    private static array $deferredShutdowns = [];
+
+    /**
+     * @internal
+     *
+     * @return bool whether an application is currently being started or not yet reset
+     */
+    public static function appStarting(): bool
+    {
+        return self::$appStarting;
+    }
+
+    /**
+     * Stores a shutdown registration (with its arguments) instead of registering it,
+     * to be run later by `resetApplication()`.
+     *
+     * The arguments are kept because `yii\log\Logger::init()` registers
+     * `[$this, 'flush']` with `true` — the final-flush flag that gates target export —
+     * and it must survive the store/invoke round-trip.
+     *
+     * @internal
+     */
+    public static function registerShutdown(callable $callable, mixed ...$args): void
+    {
+        // `array_values()` normalizes the splatted args to a `list`: a variadic array is always
+        // int-keyed at runtime, so this only makes the stored shape match the property.
+        self::$deferredShutdowns[] = [$callable, array_values($args)];
+    }
+
+    /**
+     * Runs the stored shutdown registrations.
+     *
+     * The queue is drained FIFO. `yii\log\Logger::init()` — the only production producer —
+     * contributes exactly two invocations per logger (the stored outer closure first, then
+     * the nested final `flush(true)` it re-registers when invoked), so a cap of four times
+     * the queue's own size fully covers every finite logger queue. It is there to bound
+     * pathological re-registration in the `yii\log` namespace: when a callable stores more
+     * entries than the headroom allows, the drain stops and the leftover entries stay in
+     * the queue until the next drain — never in PHP's real shutdown registry.
+     *
+     * @internal
+     */
+    private static function runDeferredShutdowns(): void
+    {
+        $cap = 4 * count(self::$deferredShutdowns);
+        while (self::$deferredShutdowns !== [] && $cap-- > 0) {
+            [$callable, $args] = array_shift(self::$deferredShutdowns);
+            \call_user_func($callable, ...$args);
+        }
+    }
+
     /**
      * @internal
      */
@@ -158,9 +231,25 @@ private function getWebRequest(): YiiRequest
     public function resetApplication(bool $closeSession = true): void
     {
         codecept_debug('Destroying application');
+
+        // Close the session while the application is still reachable. If `Yii::$app` was
+        // already gone, `closeSession()` starts an application to close it; doing that
+        // before the drain keeps `$appStarting` on for that build, so its logger
+        // registrations are captured and drained below, and the flag is cleared right
+        // after — instead of being left pending with interception still enabled.
         if ($closeSession) {
             $this->closeSession();
         }
+
+        // Run the shutdown registrations stored during the previous `startApp()` before the
+        // application is torn down: invoking them here (instead of letting them run at
+        // process exit) is what stops the loggers — and the dispatchers, targets and
+        // application they hold — from being retained for the whole test process. This is
+        // idempotent: when called before the next application is started, the queue is
+        // already empty.
+        self::runDeferredShutdowns();
+        self::$appStarting = false;
+
         Yii::$app = null;
         \yii\web\UploadedFile::reset();
         Event::offAll();
@@ -284,6 +373,36 @@ public function getCsrfParamName(): string
     public function startApp(?\yii\log\Logger $logger = null): void
     {
         codecept_debug('Starting application');
+
+        // Intercept shutdown registrations made in the `yii\log` namespace for the rest of
+        // this process. Yii does not import `register_shutdown_function`, so an unqualified
+        // call in `yii\log\Logger::init()` resolves to a namespace-level function when one
+        // is defined there; while `$appStarting` is on, those calls are stored instead of
+        // being registered. Without this, every plain logger built during the application
+        // bootstrap (e.g. one the application itself sets via a `bootstrap` callback) keeps
+        // itself alive — with its dispatcher, log targets and the application — until
+        // process exit, because PHP's shutdown closures outlive the discarded application.
+        // PHP resolves this call at runtime (namespace fallback per call), so defining the
+        // function here also intercepts call sites compiled before it; and `$appStarting`
+        // is on before the config file is included, so a plain logger constructed there —
+        // or in a file it requires — is captured from the start.
+        if (! function_exists('\yii\log\register_shutdown_function')) {
+            eval(<<<'PHP'
+                namespace yii\log {
+                    function register_shutdown_function(callable $callable, mixed ...$args) {
+                        if (\Codeception\Lib\Connector\Yii2::appStarting()) {
+                            \Codeception\Lib\Connector\Yii2::registerShutdown($callable, ...$args);
+                            return null;
+                        }
+                        return \register_shutdown_function($callable, ...$args);
+                    }
+                }
+            PHP);
+        }
+        // Keep it on through the whole test so loggers created after the build are
+        // captured too; `resetApplication()` runs the stored registrations and turns it off.
+        self::$appStarting = true;
+
         $config = include $this->configFile;
         if (! isset($config['class'])) {
             $config['class'] = $this->applicationClass ?? \yii\web\Application::class;
@@ -305,18 +424,30 @@ public function startApp(?\yii\log\Logger $logger = null): void
             self::MAIL_IGNORE => null// Do nothing
         };
 
+        // Install the module logger before the application is built: while `createObject()`
+        // runs, the application already calls `Yii::getLogger()` (its log dispatcher and
+        // debug bootstrap). If no logger is installed yet, a plain `yii\log\Logger` is
+        // created, and its `init()` registers a shutdown function that keeps it alive —
+        // along with its dispatcher, log targets and the application — after every test.
+        // The module logger overrides `init()` to avoid that, so it must be in place first.
+        $moduleLogger = $logger instanceof \yii\log\Logger
+            ? $logger
+            : new Logger();
+        Yii::setLogger($moduleLogger);
+
         // @phpstan-ignore argument.templateType
         $app = Yii::createObject($config);
         if (! $app instanceof \yii\base\Application) {
             throw new ModuleConfigException($this, "Failed to initialize Yii2 app");
         }
         \Yii::$app = $app;
 
-        if ($logger instanceof \yii\log\Logger) {
-            Yii::setLogger($logger);
-        } else {
-            Yii::setLogger(new Logger());
-        }
+        // The application's bootstrap may replace the global logger while it is being
+        // built (a `bootstrap` callback calling `Yii::setLogger()`, or a `logger` config
+        // entry). Restore the module logger afterwards so that messages emitted during the
+        // test reach it and `Module\Yii2::_failed()` can report them; otherwise such a
+        // bootstrap replacement would swallow the module's failure diagnostics.
+        Yii::setLogger($moduleLogger);
     }
 
     /**

tests/cases/bootstrap-logger/app/App.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace app\bootstrapLogger;
+
+use yii\web\Application as BaseApplication;
+
+final class App extends BaseApplication
+{
+}

tests/cases/bootstrap-logger/app/CustomLogger.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace app\bootstrapLogger;
+
+use yii\log\Logger as YiiLogger;
+
+/**
+ * A standalone global logger installed by the application's bootstrap callback.
+ * It overrides `init()` (as the module logger does) only to avoid the unrelated
+ * shutdown-registration retention root; the point is that the app replaces the
+ * global logger while it is being built.
+ */
+final class CustomLogger extends YiiLogger
+{
+    public function init(): void
+    {
+    }
+}

tests/cases/bootstrap-logger/codeception.yml

@@ -0,0 +1,12 @@
+paths:
+    tests: .
+    output: ../../_output
+    data: _data
+    support: ../../_support
+namespace: tests\
+modules:
+    config:
+        Yii2:
+            configFile: config.php
+            transaction: false
+            cleanup: false

tests/cases/bootstrap-logger/config.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+require_once __DIR__ . '/app/App.php';
+require_once __DIR__ . '/app/CustomLogger.php';
+
+return [
+    'class' => app\bootstrapLogger\App::class,
+    'id' => 'bootstrap-logger',
+    'basePath' => __DIR__,
+    'bootstrap' => [
+        static function (): void {
+            \Yii::setLogger(new \app\bootstrapLogger\CustomLogger());
+        },
+    ],
+    'components' => [
+        'request' => [
+            'cookieValidationKey' => 'bootstrap-logger',
+            'enableCsrfValidation' => false,
+        ],
+    ],
+];

tests/cases/bootstrap-logger/functional.suite.yml

@@ -0,0 +1,7 @@
+actor: FunctionalTester
+modules:
+    enabled:
+        - Yii2
+        - Asserts
+steps:
+    - Codeception\Step\ConditionalAssertion

tests/cases/bootstrap-logger/functional/BootstrapLoggerCest.php

@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+
+namespace tests;
+
+use Codeception\Lib\Connector\Yii2\Logger;
+use Codeception\Lib\Di;
+use Codeception\Lib\ModuleContainer;
+use Codeception\Module\Yii2 as Yii2Module;
+use Codeception\Test\Unit;
+
+/**
+ * Regression test for https://github.com/Codeception/module-yii2/issues/78.
+ *
+ * When an application's `bootstrap` callback replaces the global logger while the
+ * module builds the application, the module must keep reporting the messages
+ * emitted during the test. The connector installs the module logger before the
+ * application is built (so no plain `yii\log\Logger` is created mid-build, which
+ * is the retention root of issue78) and, after the build, restores it, because the
+ * application may have replaced the global logger during bootstrap (a `bootstrap`
+ * callback calling `Yii::setLogger()`, or a `logger` config entry). Without that
+ * restore the replacement stays global, the messages of the test go to the
+ * replacement, and `Module\Yii2::_failed()` — which reads its separately held
+ * logger — loses the `yii-log` failure report.
+ *
+ * This test drives the real module through its public `_before`/`_failed`/`_after`
+ * lifecycle with an app whose bootstrap installs a custom global logger: a
+ * `Yii::warning()` is emitted during the test and the `yii-log` report attached
+ * on failure must contain it. The global logger must be the module logger both
+ * while the application builds and afterwards.
+ */
+final class BootstrapLoggerCest
+{
+    public function testFailedReportKeepsLogsEmittedAfterBootstrapLoggerReplacement(FunctionalTester $I): void
+    {
+        $modules = new ModuleContainer(new Di(), []);
+        $module = new Yii2Module($modules, [
+            'configFile' => __DIR__ . '/../config.php',
+            'cleanup' => false,
+            'transaction' => false,
+        ]);
+
+        $test = new Unit('testBootstrapLogger505');
+
+        $module->_initialize();
+        $module->_before($test);
+
+        // The global logger must be the module logger both while the application
+        // is being built and once it is done — i.e. the bootstrap did not leave its
+        // custom global logger in place.
+        $I->assertInstanceOf(Logger::class, \Yii::getLogger());
+
+        \Yii::warning('bootstrap-logger-505', 'bootstrap-logger');
+        $module->_failed($test, new \RuntimeException('intentional test failure'));
+
+        $report = $test->getMetadata()->getReports()['yii-log'] ?? '';
+        $I->assertStringContainsString('bootstrap-logger-505', $report);
+
+        $module->_after($test);
+    }
+}

tests/cases/leak/app/App.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace app\leak;
+
+use ReflectionProperty;
+use yii\web\Application as BaseApplication;
+
+/**
+ * Test double that records the class of the logger installed while the application
+ * is being constructed. Used to verify that the module logger is in place before the
+ * application builds itself, so no plain `yii\log\Logger` (whose `init()` registers a
+ * shutdown function that keeps it alive after each test) is created during boot.
+ */
+final class App extends BaseApplication
+{
+    public ?string $loggerAtCtorStart = null;
+
+    public function __construct(array $config = [])
+    {
+        $this->loggerAtCtorStart = $this->loggerClass();
+        parent::__construct($config);
+    }
+
+    private function loggerClass(): ?string
+    {
+        static $property = null;
+        $property ??= new ReflectionProperty(\yii\BaseYii::class, '_logger');
+        $logger = $property->getValue();
+        return $logger === null ? null : $logger::class;
+    }
+}

tests/cases/leak/codeception.yml

@@ -0,0 +1,13 @@
+paths:
+    tests: .
+    output: ../../_output
+    data: _data
+    support: ../../_support
+namespace: tests\
+extensions:
+    enabled:
+        - Codeception\Extension\RunFailed
+modules:
+    config:
+        Yii2:
+            configFile: config.php

tests/cases/leak/config.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+require_once __DIR__ . '/app/App.php';
+
+return [
+    'class' => app\leak\App::class,
+    'id' => 'leak',
+    'basePath' => __DIR__,
+];

tests/cases/leak/functional.suite.yml

@@ -0,0 +1,8 @@
+# Codeception Test Suite Configuration
+#
+# Suite for functional tests
+actor: FunctionalTester
+modules:
+    enabled:
+        - Yii2
+        - Asserts