8000 bug #25858 [DI] Fix initialization of legacy containers by delaying i… · symfony/symfony@f004895 · GitHub
[go: up one dir, main page]
Skip to content

Commit f004895

Browse files
fabpotfabpot
committed
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 ------- 5e750ec [DI] Fix initialization of legacy containers by delaying include_once
2 parents 8d4e3c5 + 5e750ec commit f004895

File tree

4 files changed

+79
-59
lines changed

4 files changed

+79
-59
lines changed

‎src/Symfony/Component/DependencyInjection/Container.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,13 @@ public function setParameter($name, $value)
167167
*/
168168
public function set($id, $service)
169169
{
170+
// Runs the internal initializer; used by the dumped container to include always-needed files
171+
if (isset($this->privates['service_container']) && $this->privates['service_container'] instanceof \Closure) {
172+
$initialize = $this->privates['service_container'];
173+
unset($this->privates['service_container']);
174+
$initialize();
175+
}
176+
170177
$id = $this->normalizeId($id);
171178

172179
if ('service_container' === $id) {

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,16 +1243,16 @@ private function addInlineRequires()
12431243
}
12441244
}
12451245

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

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

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

12581258
/**

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,12 @@ public function __construct()
4646

4747
$this->aliases = array();
4848

49-
include_once $this->targetDirs[1].'/includes/HotPath/I1.php';
50-
include_once $this->targetDirs[1].'/includes/HotPath/P1.php';
51-
include_once $this->targetDirs[1].'/includes/HotPath/T1.php';
52-
include_once $this->targetDirs[1].'/includes/HotPath/C1.php';
49+
$this->privates['service_container'] = function () {
50+
include_once $this->targetDirs[1].'/includes/HotPath/I1.php';
51+
include_once $this->targetDirs[1].'/includes/HotPath/P1.php';
52+
include_once $this->targetDirs[1].'/includes/HotPath/T1.php';
53+
include_once $this->targetDirs[1].'/includes/HotPath/C1.php';
54+
};
5355
}
5456

5557
public function getRemovedIds()

‎src/Symfony/Component/HttpKernel/Kernel.php‎

Lines changed: 63 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -581,80 +581,91 @@ protected function initializeContainer()
581581
$class = $this->getContainerClass();
582582
$cacheDir = $this->warmupDir ?: $this->getCacheDir();
583583
$cache = new ConfigCache($cacheDir.'/'.$class.'.php', $this->debug);
584+
$oldContainer = null;
584585
if ($fresh = $cache->isFresh()) {
585586
// Silence E_WARNING to ignore "include" failures - don't use "@" to prevent silencing fatal errors
586587
$errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
588+
$fresh = $oldContainer = false;
587589
try {
588-
$this->container = include $cache->getPath();
590+
if (\is_object($this->container = include $cache->getPath())) {
591+
$this->container->set('kernel', $this);
592+
$oldContainer = $this->container;
593+
$fresh = true;
594+
}
595+
} catch (\Throwable $e) {
596+
} catch (\Exception $e) {
589597
} finally {
590598
error_reporting($errorLevel);
591599
}
592-
$fresh = \is_object($this->container);
593600
}
594-
if (!$fresh) {
595-
if ($this->debug) {
596-
$collectedLogs = array();
597-
$previousHandler = defined('PHPUNIT_COMPOSER_INSTALL');
598-
$previousHandler = $previousHandler ?: set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
599-
if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
600-
return $previousHandler ? $previousHandler($type & ~E_WARNING, $message, $file, $line) : E_WARNING === $type;
601-
}
602601

603-
if (isset($collectedLogs[$message])) {
604-
++$collectedLogs[$message]['count'];
602+
if ($fresh) {
603+
return;
604+
}
605605

606-
return;
607-
}
606+
if ($this->debug) {
607+
$collectedLogs = array();
608+
$previousHandler = defined('PHPUNIT_COMPOSER_INSTALL');
609+
$previousHandler = $previousHandler ?: set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
610+
if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
611+
return $previousHandler ? $previousHandler($type, $message, $file, $line) : false;
612+
}
613+
614+
if (isset($collectedLogs[$message])) {
615+
++$collectedLogs[$message]['count'];
616+
617+
return;
618+
}
608619

609-
$backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3);
610-
// Clean the trace by removing first frames added by the error handler itself.
611-
for ($i = 0; isset($backtrace[$i]); ++$i) {
612-
if (isset($backtrace[$i]['file'], $backtrace[$i]['line']) && $backtrace[$i]['line'] === $line && $backtrace[$i]['file'] === $file) {
613-
$backtrace = array_slice($backtrace, 1 + $i);
614-
break;
615-
}
620+
$backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3);
621+
// Clean the trace by removing first frames added by the error handler itself.
622+
for ($i = 0; isset($backtrace[$i]); ++$i) {
623+
if (isset($backtrace[$i]['file'], $backtrace[$i]['line']) && $backtrace[$i]['line'] === $line && $backtrace[$i]['file'] === $file) {
624+
$backtrace = array_slice($backtrace, 1 + $i);
625+
break;
616626
}
627+
}
617628

618-
$collectedLogs[$message] = array(
619-
'type' => $type,
620-
'message' => $message,
621-
'file' => $file,
622-
'line' => $line,
623-
'trace' => $backtrace,
624-
'count' => 1,
625-
);
626-
});
627-
} else {
628-
$errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
629+
$collectedLogs[$message] = array(
630+
'type' => $type,
631+
'message' => $message,
632+
'file' => $file,
633+
'line' => $line,
634+
'trace' => $backtrace,
635+
'count' => 1,
636+
);
637+
});
638+
}
639+
640+
try {
641+
$container = null;
642+
$container = $this->buildContainer();
643+
$container->compile();
644+
} finally {
645+
if ($this->debug && true !== $previousHandler) {
646+
restore_error_handler();
647+
648+
file_put_contents($cacheDir.'/'.$class.'Deprecations.log', serialize(array_values($collectedLogs)));
649+
file_put_contents($cacheDir.'/'.$class.'Compiler.log', null !== $container ? implode("\n", $container->getCompiler()->getLog()) : '');
629650
}
651+
}
630652

653+
if (null === $oldContainer) {
654+
$errorLevel = error_reporting(\E_ALL ^ \E_WARNING);
631655
try {
632-
$container = null;
633-
$container = $this->buildContainer();
634-
$container->compile();
635-
636-
$oldContainer = file_exists($cache->getPath()) && is_object($oldContainer = include $cache->getPath()) ? new \ReflectionClass($oldContainer) : false;
656+
$oldContainer = include $cache->getPath();
657+
} catch (\Throwable $e) {
658+
} catch (\Exception $e) {
637659
} finally {
638-
if (!$this->debug) {
639-
error_reporting($errorLevel);
640-
} elseif (true !== $previousHandler) {
641-
restore_error_handler();
642-
643-
file_put_contents($cacheDir.'/'.$class.'Deprecations.log', serialize(array_values($collectedLogs)));
644-
file_put_contents($cacheDir.'/'.$class.'Compiler.log', null !== $container ? implode("\n", $container->getCompiler()->getLog()) : '');
645-
}
660+
error_reporting($errorLevel);
646661
}
647-
648-
$this->dumpContainer($cache, $container, $class, $this->getContainerBaseClass());
649-
$this->container = require $cache->getPath();
650662
}
663+
$oldContainer = is_object($oldContainer) ? new \ReflectionClass($oldContainer) : false;
651664

665+
$this->dumpContainer($cache, $container, $class, $this->getContainerBaseClass());
666+
$this->container = require $cache->getPath();
652667
$this->container->set('kernel', $this);
653668

654-
if ($fresh) {
655-
return;
656-
}
657-
658669
if ($oldContainer && get_class($this->container) !== $oldContainer->name) {
659670
// Because concurrent requests might still be using them,
660671
// old container files are not removed immediately,

0 commit comments

Comments
 (0)
0