]> source.dussan.org Git - sonarqube.git/commitdiff
Fix shutdown of server after DB migration
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Thu, 11 Feb 2016 17:14:52 +0000 (18:14 +0100)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Thu, 11 Feb 2016 17:15:47 +0000 (18:15 +0100)
When a DB migration is required, two containers are up at the same
time ("safemode" and "level 2"). This is not handled by ComponentContainer,
so one of the two children is badly dereferenced and can't be stopped.

server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel.java
server/sonar-server/src/test/java/org/sonar/server/computation/container/ComputeEngineContainerImplTest.java
sonar-core/src/main/java/org/sonar/core/platform/ComponentContainer.java
sonar-core/src/test/java/org/sonar/core/platform/ComponentContainerTest.java

index 75f29777a22e102a362714cb48b01717c62f27f6..d060d8bc2c699ae433cfad3dfc61accf1b5f6830 100644 (file)
@@ -100,7 +100,7 @@ public abstract class PlatformLevel {
    */
   public PlatformLevel destroy() {
     if (parent != null) {
-      parent.container.removeChild();
+      parent.container.removeChild(container);
     }
     return this;
   }
index e37c1856e9c74d091d804c5e9f5fb77bbc60ef75..13abce63cc832f6ab2976791cc162732ee64c64c 100644 (file)
@@ -54,7 +54,7 @@ public class ComputeEngineContainerImplTest {
     ContainerPopulator populator = mock(ContainerPopulator.class);
     ComputeEngineContainerImpl ceContainer = new ComputeEngineContainerImpl(parent, populator);
 
-    assertThat(parent.getChild()).isNull();
+    assertThat(parent.getChildren()).isEmpty();
     verify(populator).populateContainer(ceContainer);
   }
 
index 8db0da8cd5809b32b96e7b82f28f0c464872b93c..3d01d8f482e55fdc2dcf533af7aac9931434f452 100644 (file)
@@ -21,6 +21,7 @@ package org.sonar.core.platform;
 
 import com.google.common.collect.Iterables;
 import java.lang.annotation.Annotation;
+import java.util.ArrayList;
 import java.util.List;
 import javax.annotation.Nullable;
 import org.picocontainer.Characteristics;
@@ -71,12 +72,11 @@ public class ComponentContainer implements ContainerPopulator.Container {
     }
   }
 
-  // no need for multiple children
-  ComponentContainer parent;
-  ComponentContainer child;
-  MutablePicoContainer pico;
-  PropertyDefinitions propertyDefinitions;
-  ComponentKeys componentKeys;
+  private ComponentContainer parent;
+  private final List<ComponentContainer> children = new ArrayList<>();
+  private MutablePicoContainer pico;
+  private PropertyDefinitions propertyDefinitions;
+  private ComponentKeys componentKeys;
 
   /**
    * Create root container
@@ -87,7 +87,6 @@ public class ComponentContainer implements ContainerPopulator.Container {
 
   protected ComponentContainer(MutablePicoContainer picoContainer) {
     this.parent = null;
-    this.child = null;
     this.pico = picoContainer;
     this.componentKeys = new ComponentKeys();
     propertyDefinitions = new PropertyDefinitions();
@@ -101,7 +100,7 @@ public class ComponentContainer implements ContainerPopulator.Container {
   protected ComponentContainer(ComponentContainer parent) {
     this.parent = parent;
     this.pico = parent.pico.makeChildContainer();
-    this.parent.child = this;
+    this.parent.children.add(this);
     this.propertyDefinitions = parent.propertyDefinitions;
     this.componentKeys = new ComponentKeys();
     addSingleton(this);
@@ -168,9 +167,9 @@ public class ComponentContainer implements ContainerPopulator.Container {
         throw PicoUtils.propagate(e);
       }
     } finally {
-      removeChild();
+      removeChildren();
       if (parent != null) {
-        parent.removeChild();
+        parent.removeChild(this);
       }
     }
     return this;
@@ -270,11 +269,22 @@ public class ComponentContainer implements ContainerPopulator.Container {
     return pico.getComponents(tClass);
   }
 
-  public ComponentContainer removeChild() {
-    if (child != null) {
+  public ComponentContainer removeChild(ComponentContainer childToBeRemoved) {
+    for (ComponentContainer child : children) {
+      if (child == childToBeRemoved) {
+        pico.removeChildContainer(child.pico);
+        children.remove(child);
+        break;
+      }
+    }
+    return this;
+  }
+
+  private ComponentContainer removeChildren() {
+    for (ComponentContainer child : children) {
       pico.removeChildContainer(child.pico);
-      child = null;
     }
+    children.clear();
     return this;
   }
 
@@ -299,8 +309,8 @@ public class ComponentContainer implements ContainerPopulator.Container {
     return parent;
   }
 
-  public ComponentContainer getChild() {
-    return child;
+  public List<ComponentContainer> getChildren() {
+    return children;
   }
 
   public MutablePicoContainer getPicoContainer() {
index f3aaeddc1bdb568022f309ea74ce5804b7ce5c70..4de73a5e9c9ef58089ddc205476a2dcc6eb206de 100644 (file)
@@ -116,7 +116,7 @@ public class ComponentContainerTest {
     child.startComponents();
 
     assertThat(child.getParent()).isSameAs(parent);
-    assertThat(parent.getChild()).isSameAs(child);
+    assertThat(parent.getChildren()).containsOnly(child);
     assertThat(child.getComponentByType(ComponentContainer.class)).isSameAs(child);
     assertThat(parent.getComponentByType(ComponentContainer.class)).isSameAs(parent);
     assertThat(child.getComponentByType(StartableComponent.class)).isNotNull();
@@ -131,12 +131,30 @@ public class ComponentContainerTest {
     parent.startComponents();
 
     ComponentContainer child = parent.createChild();
-    assertThat(parent.getChild()).isSameAs(child);
+    assertThat(parent.getChildren()).containsOnly(child);
 
-    parent.removeChild();
-    assertThat(parent.getChild()).isNull();
+    parent.removeChild(child);
+    assertThat(parent.getChildren()).isEmpty();
   }
 
+  @Test
+  public void support_multiple_children() {
+    ComponentContainer parent = new ComponentContainer();
+    parent.startComponents();
+    ComponentContainer child1 = parent.createChild();
+    child1.startComponents();
+    ComponentContainer child2 = parent.createChild();
+    child2.startComponents();
+    assertThat(parent.getChildren()).containsOnly(child1, child2);
+
+    child1.stopComponents();
+    assertThat(parent.getChildren()).containsOnly(child2);
+
+    parent.stopComponents();
+    assertThat(parent.getChildren()).isEmpty();
+  }
+
+
   @Test
   public void shouldForwardStartAndStopToDescendants() {
     ComponentContainer grandParent = new ComponentContainer();