SONAR-9973 TaskContainer now support errors during start of components

TaskContainer must now be started by a specific method call #bootup() (rather than it being started in constructor)
TaskContainer is now AutoCloseable (method cleanup() is therefor replaced by method close())
This change will break at compile time and runtime plugins relying on TaskContainer and TaskContainerImpl

5 files changed, 51 insertions, 31 deletions

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 27888a6f9be..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
@@ -42,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) {
@@ -74,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");
     }