From e2b4c8d3c9cc5d2b1b152f99b2a8b8904948ae35 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 27 Nov 2018 11:58:55 +0100 Subject: [PATCH 1/2] [Debug] workaround opcache bug mutating "$this" !?! --- phpunit | 2 +- src/Symfony/Component/Debug/DebugClassLoader.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/phpunit b/phpunit index f4b80ed064..9975195309 100755 --- a/phpunit +++ b/phpunit @@ -1,7 +1,7 @@ #!/usr/bin/env php isFinder && !isset($this->loaded[$class])) { $this->loaded[$class] = true; - if ($file = $this->classLoader[0]->findFile($class) ?: false) { - $wasCached = \function_exists('opcache_is_script_cached') && @opcache_is_script_cached($file); - + if (!$file = $this->classLoader[0]->findFile($class) ?: false) { + // no-op + } elseif (\function_exists('opcache_is_script_cached') && @opcache_is_script_cached($file)) { require $file; - if ($wasCached) { - return; - } + return; + } else { + require $file; } } else { \call_user_func($this->classLoader, $class); From 0d0be12e07b1e04e4d87286dbeb1c64302dccbfa Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 27 Nov 2018 11:55:51 +0100 Subject: [PATCH 2/2] [DI] fix combinatorial explosion when analyzing the service graph --- .../Compiler/InlineServiceDefinitionsPass.php | 12 +++- .../DependencyInjection/Dumper/PhpDumper.php | 68 +++++++++++++------ 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php index 05eb72d974..65da0b9ac7 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php @@ -127,13 +127,19 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe } $ids = array(); + $isReferencedByConstructor = false; foreach ($graph->getNode($id)->getInEdges() as $edge) { + $isReferencedByConstructor = $isReferencedByConstructor || $edge->isReferencedByConstructor(); if ($edge->isWeak()) { return false; } $ids[] = $edge->getSourceNode()->getId(); } + if (!$ids) { + return true; + } + if (\count(array_unique($ids)) > 1) { return false; } @@ -142,6 +148,10 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe return false; } - return !$ids || $this->container->getDefinition($ids[0])->isShared(); + if ($isReferencedByConstructor && $this->container->getDefinition($ids[0])->isLazy() && ($definition->getProperties() || $definition->getMethodCalls() || $definition->getConfigurator())) { + return false; + } + + return $this->container->getDefinition($ids[0])->isShared(); } } diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index fd7eec0575..95a98c6aaa 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -155,17 +155,18 @@ class PhpDumper extends Dumper } (new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container); + $checkedNodes = array(); $this->circularReferences = array(); - foreach (array(true, false) as $byConstructor) { - foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) { - if (!$node->getValue() instanceof Definition) { - continue; - } - $currentPath = array($id => true); - $this->analyzeCircularReferences($node->getOutEdges(), $currentPath, $id, $byConstructor); + foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) { + if (!$node->getValue() instanceof Definition) { + continue; + } + if (!isset($checkedNodes[$id])) { + $this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes); } } $this->container->getCompiler()->getServiceReferenceGraph()->clear(); + $checkedNodes = array(); $this->docStar = $options['debug'] ? '*' : ''; @@ -301,12 +302,12 @@ EOF; return $this->proxyDumper; } - private function analyzeCircularReferences(array $edges, &$currentPath, $sourceId, $byConstructor) + private function analyzeCircularReferences($sourceId, array $edges, &$checkedNodes, &$currentPath = array()) { + $checkedNodes[$sourceId] = true; + $currentPath[$sourceId] = $sourceId; + foreach ($edges as $edge) { - if ($byConstructor && !$edge->isReferencedByConstructor()) { - continue; - } $node = $edge->getDestNode(); $id = $node->getId(); @@ -315,20 +316,42 @@ EOF; } elseif (isset($currentPath[$id])) { $currentId = $id; foreach (array_reverse($currentPath) as $parentId) { - if (!isset($this->circularReferences[$parentId][$currentId])) { - $this->circularReferences[$parentId][$currentId] = $byConstructor; - } + $this->circularReferences[$parentId][$currentId] = $currentId; if ($parentId === $id) { break; } $currentId = $parentId; } - } else { - $currentPath[$id] = $id; - $this->analyzeCircularReferences($node->getOutEdges(), $currentPath, $id, $byConstructor); - unset($currentPath[$id]); + } elseif (!isset($checkedNodes[$id])) { + $this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath); + } elseif (isset($this->circularReferences[$id])) { + $this->connectCircularReferences($id, $currentPath); } } + unset($currentPath[$sourceId]); + } + + private function connectCircularReferences($sourceId, &$currentPath, &$subPath = array()) + { + $subPath[$sourceId] = $sourceId; + $currentPath[$sourceId] = $sourceId; + + foreach ($this->circularReferences[$sourceId] as $id) { + if (isset($currentPath[$id])) { + $currentId = $id; + foreach (array_reverse($currentPath) as $parentId) { + $this->circularReferences[$parentId][$currentId] = $currentId; + if ($parentId === $id) { + break; + } + $currentId = $parentId; + } + } elseif (!isset($subPath[$id]) && isset($this->circularReferences[$id])) { + $this->connectCircularReferences($id, $currentPath, $subPath); + } + } + unset($currentPath[$sourceId]); + unset($subPath[$sourceId]); } private function collectLineage($class, array &$lineage) @@ -569,8 +592,11 @@ EOF; if (\is_array($callable)) { if ($callable[0] instanceof Reference - || ($callable[0] instanceof Definition && $this->definitionVariables->contains($callable[0]))) { - return sprintf(" %s->%s(\$%s);\n", $this->dumpValue($callable[0]), $callable[1], $variableName); + || ($callable[0] instanceof Definition && $this->definitionVariables->contains($callable[0])) + ) { + $callable[0] = $this->dumpValue($callable[0]); + + return sprintf(' '.('$' === $callable[0][0] ? '%s' : '(%s)')."->%s(\$%s);\n", $callable[0], $callable[1], $variableName); } $class = $this->dumpValue($callable[0]); @@ -724,7 +750,7 @@ EOF; $hasSelfRef = isset($this->circularReferences[$id][$targetId]); $forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]); - $code = $hasSelfRef && $this->circularReferences[$id][$targetId] && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : ''; + $code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : ''; if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) { return $code;