From 17d4e1538fcc06e6192216d5365a1815fb0f80bb Mon Sep 17 00:00:00 2001
From: Fabien Potencier <fabien.potencier@gmail.com>
Date: Mon, 1 Mar 2010 18:37:22 +0100
Subject: [PATCH] [DependencyInjection] store references to shared services as
 soon as possible to avoid circular references on legal code

---
 .../DependencyInjection/Builder.php           | 17 +++++----
 .../DependencyInjection/Dumper/PhpDumper.php  | 36 ++++++++-----------
 .../DependencyInjection/php/services9.php     | 12 ++++---
 3 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/src/Symfony/Components/DependencyInjection/Builder.php b/src/Symfony/Components/DependencyInjection/Builder.php
index 2082e184c3..65ee52894f 100644
--- a/src/Symfony/Components/DependencyInjection/Builder.php
+++ b/src/Symfony/Components/DependencyInjection/Builder.php
@@ -97,14 +97,7 @@ class Builder extends Container implements AnnotatedContainerInterface
 
       $this->loading[$id] = true;
 
-      if ($definition->isShared())
-      {
-        $service = $this->services[$id] = $this->createService($definition);
-      }
-      else
-      {
-        $service = $this->createService($definition);
-      }
+      $service = $this->createService($definition, $id);
 
       unset($this->loading[$id]);
 
@@ -339,10 +332,11 @@ class Builder extends Container implements AnnotatedContainerInterface
    * Creates a service for a service definition.
    *
    * @param  Definition $definition A service definition instance
+   * @param  string     $id         The service identifier
    *
    * @return object              The service described by the service definition
    */
-  protected function createService(Definition $definition)
+  protected function createService(Definition $definition, $id)
   {
     if (null !== $definition->getFile())
     {
@@ -362,6 +356,11 @@ class Builder extends Container implements AnnotatedContainerInterface
       $service = null === $r->getConstructor() ? $r->newInstance() : $r->newInstanceArgs($arguments);
     }
 
+    if ($definition->isShared())
+    {
+      $this->services[$id] = $service;
+    }
+
     foreach ($definition->getMethodCalls() as $call)
     {
       $services = self::getServiceConditionals($call[1]);
diff --git a/src/Symfony/Components/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Components/DependencyInjection/Dumper/PhpDumper.php
index 149cb42e6a..5202c5f261 100644
--- a/src/Symfony/Components/DependencyInjection/Dumper/PhpDumper.php
+++ b/src/Symfony/Components/DependencyInjection/Dumper/PhpDumper.php
@@ -76,24 +76,12 @@ EOF;
 
   protected function addServiceReturn($id, $definition)
   {
-    if ($definition->isShared())
-    {
-      return <<<EOF
-
-    return \$this->shared['$id'] = \$instance;
-  }
-
-EOF;
-    }
-    else
-    {
-      return <<<EOF
+    return <<<EOF
 
     return \$instance;
   }
 
 EOF;
-    }
   }
 
   protected function addServiceInstance($id, $definition)
@@ -108,19 +96,23 @@ EOF;
 
     if (null !== $definition->getConstructor())
     {
-      return sprintf("    \$instance = call_user_func(array(%s, '%s')%s);\n", $class, $definition->getConstructor(), $arguments ? ', '.implode(', ', $arguments) : '');
+      $code = sprintf("    \$instance = call_user_func(array(%s, '%s')%s);\n", $class, $definition->getConstructor(), $arguments ? ', '.implode(', ', $arguments) : '');
+    }
+    elseif ($class != "'".str_replace('\\', '\\\\', $definition->getClass())."'")
+    {
+      $code = sprintf("    \$class = %s;\n    \$instance = new \$class(%s);\n", $class, implode(', ', $arguments));
     }
     else
     {
-      if ($class != "'".str_replace('\\', '\\\\', $definition->getClass())."'")
-      {
-        return sprintf("    \$class = %s;\n    \$instance = new \$class(%s);\n", $class, implode(', ', $arguments));
-      }
-      else
-      {
-        return sprintf("    \$instance = new %s(%s);\n", $definition->getClass(), implode(', ', $arguments));
-      }
+      $code = sprintf("    \$instance = new %s(%s);\n", $definition->getClass(), implode(', ', $arguments));
     }
+
+    if ($definition->isShared())
+    {
+      $code .= sprintf("    \$this->shared['$id'] = \$instance;\n");
+    }
+
+    return $code;
   }
 
   protected function addServiceMethodCalls($id, $definition)
diff --git a/tests/fixtures/Symfony/Components/DependencyInjection/php/services9.php b/tests/fixtures/Symfony/Components/DependencyInjection/php/services9.php
index 2307c8b857..8d902f21a7 100644
--- a/tests/fixtures/Symfony/Components/DependencyInjection/php/services9.php
+++ b/tests/fixtures/Symfony/Components/DependencyInjection/php/services9.php
@@ -61,9 +61,10 @@ class ProjectServiceContainer extends Container
     if (isset($this->shared['bar'])) return $this->shared['bar'];
 
     $instance = new FooClass('foo', $this->getFoo_BazService(), $this->getParameter('foo_bar'));
+    $this->shared['bar'] = $instance;
     $this->getFoo_BazService()->configure($instance);
 
-    return $this->shared['bar'] = $instance;
+    return $instance;
   }
 
   /**
@@ -79,9 +80,10 @@ class ProjectServiceContainer extends Container
     if (isset($this->shared['foo.baz'])) return $this->shared['foo.baz'];
 
     $instance = call_user_func(array($this->getParameter('baz_class'), 'getInstance'));
+    $this->shared['foo.baz'] = $instance;
     call_user_func(array($this->getParameter('baz_class'), 'configureStatic1'), $instance);
 
-    return $this->shared['foo.baz'] = $instance;
+    return $instance;
   }
 
   /**
@@ -98,8 +100,9 @@ class ProjectServiceContainer extends Container
 
     $class = $this->getParameter('foo_class');
     $instance = new $class();
+    $this->shared['foo_bar'] = $instance;
 
-    return $this->shared['foo_bar'] = $instance;
+    return $instance;
   }
 
   /**
@@ -115,6 +118,7 @@ class ProjectServiceContainer extends Container
     if (isset($this->shared['method_call1'])) return $this->shared['method_call1'];
 
     $instance = new FooClass();
+    $this->shared['method_call1'] = $instance;
     $instance->setBar($this->getFooService());
     $instance->setBar($this->getService('foo', Container::NULL_ON_INVALID_REFERENCE));
     if ($this->hasService('foo'))
@@ -126,7 +130,7 @@ class ProjectServiceContainer extends Container
       $instance->setBar($this->getService('foobaz', Container::NULL_ON_INVALID_REFERENCE));
     }
 
-    return $this->shared['method_call1'] = $instance;
+    return $instance;
   }
 
   /**