bug #25858 [DI] Fix initialization of legacy containers by delaying include_once (nicolas-grekas)

This PR was merged into the 3.4 branch. Discussion ---------- [DI] Fix initialization of legacy containers by delaying include_once | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Best reviewed ignoring whitespaces: https://github.com/symfony/symfony/pull/25858/files?w=1 Noticed while removing a package: silencing the failing `include_once` as introduced in #25255 is not working for the `$oldContainer` in `Kernel`, and fails with a fatal error when an include succeeds but the class inside misses a parent. Delaying the calls to `include_once` to the moment where the fresh container is actually used first, when setting the "kernel" service, works around the situation. Commits ------- 5e750ec4b5 [DI] Fix initialization of legacy containers by delaying include_once
2018-01-23 07:50:41 +01:00 · 2018-01-23 07:50:41 +01:00 · f004895e46
commit f004895e46
parent 8d4e3c52c0 5e750ec4b5
4 changed files with 88 additions and 68 deletions
--- a/src/Symfony/Component/DependencyInjection/Container.php
+++ b/src/Symfony/Component/DependencyInjection/Container.php
@ -167,6 +167,13 @@ class Container implements ResettableContainerInterface
     */
    public function set($id, $service)
    {
+        // Runs the internal initializer; used by the dumped container to include always-needed files
+        if (isset($this->privates['service_container']) && $this->privates['service_container'] instanceof \Closure) {
+            $initialize = $this->privates['service_container'];
+            unset($this->privates['service_container']);
+            $initialize();
+        }
+
        $id = $this->normalizeId($id);

        if ('service_container' === $id) {
--- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@ -1243,16 +1243,16 @@ EOF;
            }
        }

-        $code = "\n";
+        $code = '';

        foreach ($lineage as $file) {
            if (!isset($this->inlinedRequires[$file])) {
                $this->inlinedRequires[$file] = true;
-                $code .= sprintf("        include_once %s;\n", $file);
+                $code .= sprintf("\n            include_once %s;", $file);
            }
        }

-        return "\n" === $code ? '' : $code;
+        return $code ? sprintf("\n        \$this->privates['service_container'] = function () {%s\n        };\n", $code) : '';
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php
@ -46,10 +46,12 @@ class ProjectServiceContainer extends Container

        $this->aliases = array();

-        include_once $this->targetDirs[1].'/includes/HotPath/I1.php';
-        include_once $this->targetDirs[1].'/includes/HotPath/P1.php';
-        include_once $this->targetDirs[1].'/includes/HotPath/T1.php';
-        include_once $this->targetDirs[1].'/includes/HotPath/C1.php';
+        $this->privates['service_container'] = function () {
+            include_once $this->targetDirs[1].'/includes/HotPath/I1.php';
+            include_once $this->targetDirs[1].'/includes/HotPath/P1.php';
+            include_once $this->targetDirs[1].'/includes/HotPath/T1.php';
+            include_once $this->targetDirs[1].'/includes/HotPath/C1.php';
+        };
    }

    public function getRemovedIds()
--- a/src/Symfony/Component/HttpKernel/Kernel.php
+++ b/src/Symfony/Component/HttpKernel/Kernel.php
@ -581,80 +581,91 @@ abstract class Kernel implements KernelInterface, RebootableInterface, Terminabl
        $class = $this->getContainerClass();
        $cacheDir = $this->warmupDir ?: $this->getCacheDir();
        $cache = new ConfigCache($cacheDir.'/'.$class.'.php', $this->debug);
+        $oldContainer = null;
        if ($fresh = $cache->isFresh()) {
            // Silence E_WARNING to ignore "include" failures - don't use "@" to prevent silencing fatal errors
            $errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
+            $fresh = $oldContainer = false;
            try {
-                $this->container = include $cache->getPath();
+                if (\is_object($this->container = include $cache->getPath())) {
+                    $this->container->set('kernel', $this);
+                    $oldContainer = $this->container;
+                    $fresh = true;
+                }
+            } catch (\Throwable $e) {
+            } catch (\Exception $e) {
            } finally {
                error_reporting($errorLevel);
            }
-            $fresh = \is_object($this->container);
        }
-        if (!$fresh) {
-            if ($this->debug) {
-                $collectedLogs = array();
-                $previousHandler = defined('PHPUNIT_COMPOSER_INSTALL');
-                $previousHandler = $previousHandler ?: set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
-                    if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
-                        return $previousHandler ? $previousHandler($type & ~E_WARNING, $message, $file, $line) : E_WARNING === $type;
-                    }
-
-                    if (isset($collectedLogs[$message])) {
-                        ++$collectedLogs[$message]['count'];
-
-                        return;
-                    }
-
-                    $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3);
-                    // Clean the trace by removing first frames added by the error handler itself.
-                    for ($i = 0; isset($backtrace[$i]); ++$i) {
-                        if (isset($backtrace[$i]['file'], $backtrace[$i]['line']) && $backtrace[$i]['line'] === $line && $backtrace[$i]['file'] === $file) {
-                            $backtrace = array_slice($backtrace, 1 + $i);
-                            break;
-                        }
-                    }
-
-                    $collectedLogs[$message] = array(
-                        'type' => $type,
-                        'message' => $message,
-                        'file' => $file,
-                        'line' => $line,
-                        'trace' => $backtrace,
-                        'count' => 1,
-                    );
-                });
-            } else {
-                $errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
-            }
-
-            try {
-                $container = null;
-                $container = $this->buildContainer();
-                $container->compile();
-
-                $oldContainer = file_exists($cache->getPath()) && is_object($oldContainer = include $cache->getPath()) ? new \ReflectionClass($oldContainer) : false;
-            } finally {
-                if (!$this->debug) {
-                    error_reporting($errorLevel);
-                } elseif (true !== $previousHandler) {
-                    restore_error_handler();
-
-                    file_put_contents($cacheDir.'/'.$class.'Deprecations.log', serialize(array_values($collectedLogs)));
-                    file_put_contents($cacheDir.'/'.$class.'Compiler.log', null !== $container ? implode("\n", $container->getCompiler()->getLog()) : '');
-                }
-            }
-
-            $this->dumpContainer($cache, $container, $class, $this->getContainerBaseClass());
-            $this->container = require $cache->getPath();
-        }
-
-        $this->container->set('kernel', $this);

        if ($fresh) {
            return;
        }

+        if ($this->debug) {
+            $collectedLogs = array();
+            $previousHandler = defined('PHPUNIT_COMPOSER_INSTALL');
+            $previousHandler = $previousHandler ?: set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
+                if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
+                    return $previousHandler ? $previousHandler($type, $message, $file, $line) : false;
+                }
+
+                if (isset($collectedLogs[$message])) {
+                    ++$collectedLogs[$message]['count'];
+
+                    return;
+                }
+
+                $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3);
+                // Clean the trace by removing first frames added by the error handler itself.
+                for ($i = 0; isset($backtrace[$i]); ++$i) {
+                    if (isset($backtrace[$i]['file'], $backtrace[$i]['line']) && $backtrace[$i]['line'] === $line && $backtrace[$i]['file'] === $file) {
+                        $backtrace = array_slice($backtrace, 1 + $i);
+                        break;
+                    }
+                }
+
+                $collectedLogs[$message] = array(
+                    'type' => $type,
+                    'message' => $message,
+                    'file' => $file,
+                    'line' => $line,
+                    'trace' => $backtrace,
+                    'count' => 1,
+                );
+            });
+        }
+
+        try {
+            $container = null;
+            $container = $this->buildContainer();
+            $container->compile();
+        } finally {
+            if ($this->debug && true !== $previousHandler) {
+                restore_error_handler();
+
+                file_put_contents($cacheDir.'/'.$class.'Deprecations.log', serialize(array_values($collectedLogs)));
+                file_put_contents($cacheDir.'/'.$class.'Compiler.log', null !== $container ? implode("\n", $container->getCompiler()->getLog()) : '');
+            }
+        }
+
+        if (null === $oldContainer) {
+            $errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
+            try {
+                $oldContainer = include $cache->getPath();
+            } catch (\Throwable $e) {
+            } catch (\Exception $e) {
+            } finally {
+                error_reporting($errorLevel);
+            }
+        }
+        $oldContainer = is_object($oldContainer) ? new \ReflectionClass($oldContainer) : false;
+
+        $this->dumpContainer($cache, $container, $class, $this->getContainerBaseClass());
+        $this->container = require $cache->getPath();
+        $this->container->set('kernel', $this);
+
        if ($oldContainer && get_class($this->container) !== $oldContainer->name) {
            // Because concurrent requests might still be using them,
            // old container files are not removed immediately,