bug #25203 [DI] Fix infinite loop in InlineServiceDefinitionsPass (nicolas-grekas)

nicolas-grekas · nicolas-grekas · commit 85223d373b4e · 2017-11-29T10:21:33.000+01:00
This PR was merged into the 3.3 branch. Discussion ---------- [DI] Fix infinite loop in InlineServiceDefinitionsPass | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - When a non-shared is involved in a setter-circular loop, the pass enters an infinite loop right now. Commits ------- b988aa7 [DI] Fix infinite loop in InlineServiceDefinitionsPass
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php
@@ -61,10 +61,7 @@ protected function processValue($value, $isRoot = false)
                 $this->container->log($this, sprintf('Inlined service "%s" to "%s".', $id, $this->currentId));
                 $this->inlinedServiceIds[$id][] = $this->currentId;
 
-                if ($definition->isShared()) {
-                    return $definition;
-                }
-                $value = clone $definition;
+                return $definition->isShared() ? $definition : clone $definition;
             }
         }
 
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/InlineServiceDefinitionsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/InlineServiceDefinitionsPassTest.php
@@ -92,6 +92,25 @@ public function testProcessDoesInlineNonSharedService()
         $this->assertNotSame($container->getDefinition('bar'), $arguments[2]);
     }
 
+    public function testProcessInlinesMixedServicesLoop()
+    {
+        $container = new ContainerBuilder();
+        $container
+            ->register('foo')
+            ->addArgument(new Reference('bar'))
+            ->setShared(false)
+        ;
+        $container
+            ->register('bar')
+            ->setPublic(false)
+            ->addMethodCall('setFoo', array(new Reference('foo')))
+        ;
+
+        $this->process($container);
+
+        $this->assertEquals($container->getDefinition('foo')->getArgument(0), $container->getDefinition('bar'));
+    }
+
     public function testProcessInlinesIfMultipleReferencesButAllFromTheSameDefinition()
     {
         $container = new ContainerBuilder();

Original file line number	Diff line number	Diff line change
`@@ -61,10 +61,7 @@ protected function processValue($value, $isRoot = false)`
`61`	`61`	`$this->container->log($this, sprintf('Inlined service "%s" to "%s".', $id, $this->currentId));`
`62`	`62`	`$this->inlinedServiceIds[$id][] = $this->currentId;`
`63`	`63`
`64`		`- if ($definition->isShared()) {`
`65`		`- return $definition;`
`66`		`- }`
`67`		`- $value = clone $definition;`
	`64`	`+ return $definition->isShared() ? $definition : clone $definition;`
`68`	`65`	`}`
`69`	`66`	`}`
`70`	`67`