Correctly stop all hierarchy of pico containers on error

2 files changed, 54 insertions, 8 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java
index 88d253f005d..4a9cd88de30 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java
@@ -117,18 +117,18 @@ public class ComponentContainer implements BatchComponent, ServerComponent {
 
   public ComponentContainer stopComponents(boolean swallowException) {
     try {
-      if (!pico.getLifecycleState().isStopped()) {
-        pico.stop();
-      }
-      removeChild();
-      if (parent!=null) {
-        parent.removeChild();
-      }
+      pico.stop();
+
 
     } catch (RuntimeException e) {
       if (!swallowException) {
         throw PicoUtils.propagate(e);
       }
+    } finally {
+      removeChild();
+      if (parent != null) {
+        parent.removeChild();
+      }
     }
     return this;
   }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentContainerTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentContainerTest.java
index 22d386326e4..d2db710a91f 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentContainerTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentContainerTest.java
@@ -47,7 +47,7 @@ public class ComponentContainerTest {
   }
 
   @Test
-  public void testStartAndStop() {
+  public void should_start_and_stop() {
     ComponentContainer container = spy(new ComponentContainer());
     container.addSingleton(StartableComponent.class);
     container.startComponents();
@@ -62,6 +62,52 @@ public class ComponentContainerTest {
   }
 
   @Test
+  public void should_start_and_stop_hierarchy_of_containers() {
+    StartableComponent parentComponent = new StartableComponent();
+    final StartableComponent childComponent = new StartableComponent();
+    ComponentContainer parentContainer = new ComponentContainer() {
+      @Override
+      public void doAfterStart() {
+        ComponentContainer childContainer = new ComponentContainer(this);
+        childContainer.add(childComponent);
+        childContainer.execute();
+      }
+    };
+    parentContainer.add(parentComponent);
+    parentContainer.execute();
+    assertThat(parentComponent.started).isTrue();
+    assertThat(parentComponent.stopped).isTrue();
+    assertThat(childComponent.started).isTrue();
+    assertThat(childComponent.stopped).isTrue();
+  }
+
+  @Test
+  public void should_stop_hierarchy_of_containers_on_failure() {
+    StartableComponent parentComponent = new StartableComponent();
+    final StartableComponent childComponent1 = new StartableComponent();
+    final UnstartableComponent childComponent2 = new UnstartableComponent();
+    ComponentContainer parentContainer = new ComponentContainer() {
+      @Override
+      public void doAfterStart() {
+        ComponentContainer childContainer = new ComponentContainer(this);
+        childContainer.add(childComponent1);
+        childContainer.add(childComponent2);
+        childContainer.execute();
+      }
+    };
+    parentContainer.add(parentComponent);
+    try {
+      parentContainer.execute();
+      fail();
+    } catch (Exception e) {
+      assertThat(parentComponent.started).isTrue();
+      assertThat(parentComponent.stopped).isTrue();
+      assertThat(childComponent1.started).isTrue();
+      assertThat(childComponent1.stopped).isTrue();
+    }
+  }
+
+  @Test
   public void testChild() {
     ComponentContainer parent = new ComponentContainer();
     parent.startComponents();