Automatic merge from branch-6.7

* origin/branch-6.7:
  SONAR-9973 IT for report processing resilient to failure during start
  SONAR-9973 TaskContainer now support errors during start of components
  SONAR-9973 add IT ensuring resilience of Ce Workers to exceptions
  SONAR-9973 ComponentContainer#stopComponent now never fail

6 files changed, 56 insertions, 34 deletions

diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/engine/MigrationContainerImpl.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/engine/MigrationContainerImpl.java
index 96c2fe978cf..bce29650b9a 100644
--- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/engine/MigrationContainerImpl.java
+++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/engine/MigrationContainerImpl.java
@@ -28,6 +28,7 @@ import org.picocontainer.lifecycle.ReflectionLifecycleStrategy;
 import org.picocontainer.monitors.NullComponentMonitor;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.core.platform.ComponentContainer;
+import org.sonar.core.platform.StopSafeReflectionLifecycleStrategy;
 
 import static java.util.Objects.requireNonNull;
 
@@ -49,7 +50,7 @@ public class MigrationContainerImpl extends ComponentContainer implements Migrat
    */
   private static MutablePicoContainer createContainer(ComponentContainer parent) {
     ComponentMonitor componentMonitor = new NullComponentMonitor();
-    ReflectionLifecycleStrategy lifecycleStrategy = new ReflectionLifecycleStrategy(componentMonitor, "start", "stop", "close") {
+    ReflectionLifecycleStrategy lifecycleStrategy = new StopSafeReflectionLifecycleStrategy(componentMonitor) {
       @Override
       public boolean isLazy(ComponentAdapter<?> adapter) {
         return true;
@@ -60,7 +61,7 @@ public class MigrationContainerImpl extends ComponentContainer implements Migrat
 
   @Override
   public void cleanup() {
-    stopComponents(true);
+    stopComponents();
   }
 
   @Override
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainer.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainer.java
index e830163292d..53d1f8d6ca1 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainer.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainer.java
@@ -25,16 +25,22 @@ import org.sonar.core.platform.ComponentContainer;
 import org.sonar.core.platform.ContainerPopulator;
 
 /**
- * The Compute Engine container. Created for a specific parent {@link ComponentContainer} and a specific {@link CeTask}.
+ * The Compute Engine task container. Created for a specific parent {@link ComponentContainer} and a specific {@link CeTask}.
  */
-public interface TaskContainer extends ContainerPopulator.Container {
+public interface TaskContainer extends ContainerPopulator.Container, AutoCloseable {
 
   ComponentContainer getParent();
 
   /**
+   * Starts task container, starting any startable component in it.
+   */
+  void bootup();
+
+  /**
    * Cleans up resources after process has been called and has returned.
    */
-  void cleanup();
+  @Override
+  void close();
 
   /**
    * Access to the underlying pico container.
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainerImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainerImpl.java
index cd65f1c886d..853187cd562 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainerImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainerImpl.java
@@ -32,6 +32,7 @@ import org.sonar.api.utils.log.Loggers;
 import org.sonar.core.platform.ComponentContainer;
 import org.sonar.core.platform.ContainerPopulator;
 import org.sonar.core.platform.Module;
+import org.sonar.core.platform.StopSafeReflectionLifecycleStrategy;
 
 import static java.util.Objects.requireNonNull;
 
@@ -41,7 +42,6 @@ public class TaskContainerImpl extends ComponentContainer implements TaskContain
     super(createContainer(requireNonNull(parent)), parent.getComponentByType(PropertyDefinitions.class));
 
     populateContainer(requireNonNull(populator));
-    startComponents();
   }
 
   private void populateContainer(ContainerPopulator<TaskContainer> populator) {
@@ -62,7 +62,7 @@ public class TaskContainerImpl extends ComponentContainer implements TaskContain
    */
   private static MutablePicoContainer createContainer(ComponentContainer parent) {
     ComponentMonitor componentMonitor = new NullComponentMonitor();
-    ReflectionLifecycleStrategy lifecycleStrategy = new ReflectionLifecycleStrategy(componentMonitor, "start", "stop", "close") {
+    ReflectionLifecycleStrategy lifecycleStrategy = new StopSafeReflectionLifecycleStrategy(componentMonitor) {
       @Override
       public boolean isLazy(ComponentAdapter<?> adapter) {
         return adapter.getComponentImplementation().getAnnotation(EagerStart.class) == null;
@@ -73,16 +73,21 @@ public class TaskContainerImpl extends ComponentContainer implements TaskContain
   }
 
   @Override
-  public void cleanup() {
-    try {
-      stopComponents();
-    } catch (Throwable t) {
-      Loggers.get(TaskContainerImpl.class).error("Cleanup of container failed", t);
-    }
+  public void bootup() {
+    startComponents();
   }
 
   @Override
   public String toString() {
     return "TaskContainerImpl";
   }
+
+  @Override
+  public void close() {
+    try {
+      stopComponents();
+    } catch (Throwable t) {
+      Loggers.get(TaskContainerImpl.class).error("Cleanup of container failed", t);
+    }
+  }
 }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/taskprocessor/ReportTaskProcessor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/taskprocessor/ReportTaskProcessor.java
index b31c53afdb9..200b9e4c28f 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/taskprocessor/ReportTaskProcessor.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/taskprocessor/ReportTaskProcessor.java
@@ -24,7 +24,6 @@ import java.util.Set;
 import javax.annotation.CheckForNull;
 import org.sonar.ce.queue.CeTask;
 import org.sonar.ce.queue.CeTaskResult;
-import org.sonar.ce.settings.SettingsLoader;
 import org.sonar.ce.taskprocessor.CeTaskProcessor;
 import org.sonar.core.platform.ComponentContainer;
 import org.sonar.db.ce.CeTaskTypes;
@@ -33,7 +32,6 @@ import org.sonar.server.computation.task.container.TaskContainer;
 import org.sonar.server.computation.task.projectanalysis.container.ContainerFactory;
 import org.sonar.server.computation.task.step.ComputationStepExecutor;
 import org.sonar.server.computation.taskprocessor.TaskResultHolder;
-import org.sonar.server.setting.ThreadLocalSettings;
 
 public class ReportTaskProcessor implements CeTaskProcessor {
 
@@ -69,20 +67,11 @@ public class ReportTaskProcessor implements CeTaskProcessor {
 
   @Override
   public CeTaskResult process(CeTask task) {
-    TaskContainer ceContainer = containerFactory.create(serverContainer, task, componentProviders);
+    try (TaskContainer ceContainer = containerFactory.create(serverContainer, task, componentProviders)) {
+      ceContainer.bootup();
 
-    try {
       ceContainer.getComponentByType(ComputationStepExecutor.class).execute();
       return ceContainer.getComponentByType(TaskResultHolder.class).getResult();
-    } finally {
-      ensureThreadLocalIsClean(ceContainer);
-
-      ceContainer.cleanup();
     }
   }
-
-  /** safety call to clear ThreadLocal even if Pico container fails to call {@link SettingsLoader#stop()}) */
-  private static void ensureThreadLocalIsClean(TaskContainer ceContainer) {
-    ceContainer.getComponentByType(ThreadLocalSettings.class).unload();
-  }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/container/TaskContainerImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/container/TaskContainerImplTest.java
index aa0d746b649..bd0c28462d4 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/container/TaskContainerImplTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/container/TaskContainerImplTest.java
@@ -58,16 +58,14 @@ public class TaskContainerImplTest {
   }
 
   @Test
-  public void components_are_started_lazily_unless_they_are_annotated_with_EagerStart() {
+  public void bootup_starts_components_lazily_unless_they_are_annotated_with_EagerStart() {
     final DefaultStartable defaultStartable = new DefaultStartable();
     final EagerStartable eagerStartable = new EagerStartable();
-    TaskContainerImpl ceContainer = new TaskContainerImpl(parent, new ContainerPopulator<TaskContainer>() {
-      @Override
-      public void populateContainer(TaskContainer container) {
-        container.add(defaultStartable);
-        container.add(eagerStartable);
-      }
+    TaskContainerImpl ceContainer = new TaskContainerImpl(parent, container -> {
+      container.add(defaultStartable);
+      container.add(eagerStartable);
     });
+    ceContainer.bootup();
 
     assertThat(defaultStartable.startCalls).isEqualTo(0);
     assertThat(defaultStartable.stopCalls).isEqualTo(0);
@@ -75,6 +73,24 @@ public class TaskContainerImplTest {
     assertThat(eagerStartable.stopCalls).isEqualTo(0);
   }
 
+  @Test
+  public void close_stops_started_components() {
+    final DefaultStartable defaultStartable = new DefaultStartable();
+    final EagerStartable eagerStartable = new EagerStartable();
+    TaskContainerImpl ceContainer = new TaskContainerImpl(parent, container -> {
+      container.add(defaultStartable);
+      container.add(eagerStartable);
+    });
+    ceContainer.bootup();
+
+    ceContainer.close();
+
+    assertThat(defaultStartable.startCalls).isEqualTo(0);
+    assertThat(defaultStartable.stopCalls).isEqualTo(0);
+    assertThat(eagerStartable.startCalls).isEqualTo(1);
+    assertThat(eagerStartable.stopCalls).isEqualTo(1);
+  }
+
   public static class DefaultStartable implements Startable {
     protected int startCalls = 0;
     protected int stopCalls = 0;
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java
index 4fbe85c0476..5231a360e3e 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java
@@ -129,12 +129,17 @@ public class ProjectAnalysisTaskContainerPopulatorTest {
     private List<Object> added = new ArrayList<>();
 
     @Override
+    public void bootup() {
+      // no effect
+    }
+
+    @Override
     public ComponentContainer getParent() {
       throw new UnsupportedOperationException("getParent is not implemented");
     }
 
     @Override
-    public void cleanup() {
+    public void close() {
       throw new UnsupportedOperationException("cleanup is not implemented");
     }