analysis: Fix leaks in call stack tests and analysis waitForCompletion

In unit tests that extend CallStackTestBase, the CallGraphAnalysis gets
a CompositeHostModel that is never disposed.

The models can be disposed in the @after method of the test, however for
this to work, the analysis must be completed before the test ends.

In CallStackTestBase, the @before method schedules the
CallStackAnalysisStub(InstrumentedCallStackAnalysis) and waits for its
completion before executing the test.

However, when the InstrumentedCallStackAnalysis completes its execution,
it schedules a CallGraphAnalysis and does not wait for its completion.
This can cause the model to be created after the test has completed and
attempted to dispose the models, and causes a leak.

The InstrumentedCallStackAnalysis waitForCompletion methods should
additionally wait for completion of the CallGraphAnalysis.

However, the CallGraphAnalysis is setup to have the
InstrumentedCallStackAnalysis as a dependent analysis, so this now
causes a deadlock as each analysis is waiting for the other to complete.

Normally an analysis that has dependent analyses will schedule these and
wait for completion before executing itself. But in this case it is the
InstrumentedCallStackAnalysis that executes to completion before
scheduling the CallGraphAnalysis, so the latter does not need to have
the former as a dependent analysis.

In other tests that do not extend CallStackTestBase, dispose the models
in the @after method.

Change-Id: I154d9d1eda7ca43740191f7fda34786c53f69b05
Signed-off-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-on: https://git.eclipse.org/r/c/tracecompass/org.eclipse.tracecompass/+/202591
Tested-by: Marco Miller <marco.miller@ericsson.com>
Tested-by: Trace Compass Bot <tracecompass-bot@eclipse.org>
Reviewed-by: Marco Miller <marco.miller@ericsson.com>

analysis/org.eclipse.tracecompass.analysis.callstack.core.tests/src/org/eclipse/tracecompass/analysis/callstack/core/tests/CallStackTestBase.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 École Polytechnique de Montréal
+ * Copyright (c) 2016, 2023 École Polytechnique de Montréal and others
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License 2.0 which
@@ -22,6 +22,7 @@
 import org.eclipse.tracecompass.internal.analysis.callstack.core.base.ICallStackSymbol;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.AggregatedCallSite;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.instrumented.InstrumentedCallStackAnalysis;
+import org.eclipse.tracecompass.internal.analysis.callstack.core.model.ModelManager;
 import org.eclipse.tracecompass.tmf.core.event.TmfEvent;
 import org.eclipse.tracecompass.tmf.core.exceptions.TmfTraceException;
 import org.eclipse.tracecompass.tmf.core.signal.TmfTraceOpenedSignal;
@@ -83,6 +84,7 @@ public void tearDown() {
         if (module != null) {
             module.dispose();
         }
+        ModelManager.disposeModels();
     }
 
     /**

analysis/org.eclipse.tracecompass.analysis.callstack.core.tests/src/org/eclipse/tracecompass/analysis/callstack/core/tests/callgraph/AggregatedCalledFunctionStatisticsTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Ericsson
+ * Copyright (c) 2016, 2023 Ericsson
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License 2.0 which
@@ -27,6 +27,7 @@
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.AggregatedCalledFunction;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.AggregatedCalledFunctionStatistics;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.ICallGraphProvider;
+import org.eclipse.tracecompass.internal.analysis.callstack.core.model.ModelManager;
 import org.eclipse.tracecompass.statesystem.core.ITmfStateSystemBuilder;
 import org.eclipse.tracecompass.statesystem.core.StateSystemFactory;
 import org.eclipse.tracecompass.statesystem.core.backend.IStateHistoryBackend;
@@ -107,6 +108,7 @@ public void disposeCga() {
         if (cga != null) {
             cga.dispose();
         }
+        ModelManager.disposeModels();
     }
 
     /**

analysis/org.eclipse.tracecompass.analysis.callstack.core.tests/src/org/eclipse/tracecompass/analysis/callstack/core/tests/callgraph/AggregationTreeTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Ericsson
+ * Copyright (c) 2016, 2023 Ericsson
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License 2.0 which
@@ -32,6 +32,7 @@
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.AggregatedCalledFunction;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.CallGraph;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.ICallGraphProvider;
+import org.eclipse.tracecompass.internal.analysis.callstack.core.model.ModelManager;
 import org.eclipse.tracecompass.statesystem.core.ITmfStateSystemBuilder;
 import org.eclipse.tracecompass.statesystem.core.StateSystemFactory;
 import org.eclipse.tracecompass.statesystem.core.backend.IStateHistoryBackend;
@@ -138,6 +139,14 @@ public void before() {
         TmfTraceManager.getInstance().traceOpened(signal);
     }
 
+    /**
+     * Cleanup models
+     */
+    @After
+    public void after() {
+        ModelManager.disposeModels();
+    }
+
     /**
      * Dispose the callgraph analysis that has been set
      */

analysis/org.eclipse.tracecompass.analysis.callstack.core.tests/src/org/eclipse/tracecompass/analysis/callstack/core/tests/callgraph/CalledFunctionTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Ericsson
+ * Copyright (c) 2016, 2023 Ericsson
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License 2.0 which
@@ -25,6 +25,7 @@
 import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.ICalledFunction;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.model.IHostModel;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.model.ModelManager;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -54,6 +55,14 @@ public void setup() {
         fHiFixture = hiFixture;
     }
 
+    /**
+     * Cleanup models
+     */
+    @After
+    public void after() {
+        ModelManager.disposeModels();
+    }
+
     /**
      * This is more to make sure that the arguments are OK except for the state
      * value

analysis/org.eclipse.tracecompass.analysis.callstack.core/src/org/eclipse/tracecompass/internal/analysis/callstack/core/callgraph/CallGraphAnalysis.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Ericsson
+ * Copyright (c) 2016, 2023 Ericsson
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License 2.0 which
@@ -37,7 +37,6 @@
 import org.eclipse.tracecompass.internal.analysis.callstack.core.model.ModelManager;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.model.ProcessStatusInterval;
 import org.eclipse.tracecompass.internal.analysis.callstack.core.tree.IWeightedTreeGroupDescriptor;
-import org.eclipse.tracecompass.tmf.core.analysis.IAnalysisModule;
 import org.eclipse.tracecompass.tmf.core.analysis.TmfAbstractAnalysisModule;
 import org.eclipse.tracecompass.tmf.core.symbols.ISymbolProvider;
 import org.eclipse.tracecompass.tmf.core.symbols.SymbolProviderManager;
@@ -149,11 +148,6 @@ public boolean canExecute(ITmfTrace trace) {
         return true;
     }
 
-    @Override
-    protected Iterable<IAnalysisModule> getDependentAnalyses() {
-        return Collections.singleton(fCsProvider);
-    }
-
     @Override
     protected boolean executeAnalysis(@Nullable IProgressMonitor monitor) {
         return executeForRange(fCallGraph, TmfTimeRange.ETERNITY, monitor);
@@ -164,30 +158,18 @@ private boolean executeForRange(CallGraph callgraph, TmfTimeRange range, @Nullab
         if (monitor == null || trace == null) {
             return false;
         }
-        Iterable<IAnalysisModule> dependentAnalyses = getDependentAnalyses();
-        for (IAnalysisModule module : dependentAnalyses) {
-            if (!(module instanceof IFlameChartProvider)) {
+        IFlameChartProvider callstackModule = fCsProvider;
+        IHostModel model = ModelManager.getModelFor(callstackModule.getHostId());
+
+        CallStackSeries callstack = callstackModule.getCallStackSeries();
+        if (callstack != null) {
+            long time0 = range.getStartTime().toNanos();
+            long time1 = range.getEndTime().toNanos();
+            long start = Math.min(time0, time1);
+            long end = Math.max(time0, time1);
+            if (!iterateOverCallstackSerie(callstack, model, callgraph, start, end, monitor)) {
                 return false;
             }
-            module.schedule();
-        }
-        // TODO: Look at updates while the state system's being built
-        dependentAnalyses.forEach(t -> t.waitForCompletion(monitor));
-
-        for (IAnalysisModule module : dependentAnalyses) {
-            IFlameChartProvider callstackModule = (IFlameChartProvider) module;
-            IHostModel model = ModelManager.getModelFor(callstackModule.getHostId());
-
-            CallStackSeries callstack = callstackModule.getCallStackSeries();
-            if (callstack != null) {
-                long time0 = range.getStartTime().toNanos();
-                long time1 = range.getEndTime().toNanos();
-                long start = Math.min(time0, time1);
-                long end = Math.max(time0, time1);
-                if (!iterateOverCallstackSerie(callstack, model, callgraph, start, end, monitor)) {
-                    return false;
-                }
-            }
         }
         monitor.worked(1);
         monitor.done();
@@ -298,15 +280,7 @@ private void iterateOverCallstack(ICallStackElement element, CallStack callstack
      * @return The collection of callstack series
      */
     public @Nullable CallStackSeries getSeries() {
-        CallStackSeries series = null;
-        for (IAnalysisModule dependent : getDependentAnalyses()) {
-            if (!(dependent instanceof IFlameChartProvider)) {
-                continue;
-            }
-            IFlameChartProvider csProvider = (IFlameChartProvider) dependent;
-            series = csProvider.getCallStackSeries();
-        }
-        return series;
+        return fCsProvider.getCallStackSeries();
     }
 
     @Override
@@ -327,13 +301,9 @@ public CallGraph getCallGraph() {
     @Override
     public Collection<IWeightedTreeGroupDescriptor> getGroupDescriptors() {
         List<IWeightedTreeGroupDescriptor> descriptors = new ArrayList<>();
-        for (IAnalysisModule module : getDependentAnalyses()) {
-            if (module instanceof IFlameChartProvider) {
-                CallStackSeries serie = ((IFlameChartProvider) module).getCallStackSeries();
-                if (serie != null) {
-                    descriptors.add(serie.getRootGroup());
-                }
-            }
+        CallStackSeries serie = fCsProvider.getCallStackSeries();
+        if (serie != null) {
+            descriptors.add(serie.getRootGroup());
         }
         return descriptors;
     }

analysis/org.eclipse.tracecompass.analysis.callstack.core/src/org/eclipse/tracecompass/internal/analysis/callstack/core/instrumented/InstrumentedCallStackAnalysis.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013, 2015 Ericsson
+ * Copyright (c) 2013, 2023 Ericsson
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License 2.0 which
@@ -159,6 +159,24 @@ protected boolean executeAnalysis(@Nullable IProgressMonitor monitor) {
         return true;
     }
 
+    @Override
+    public boolean waitForCompletion() {
+        boolean waitForCompletion = super.waitForCompletion();
+        if (fAutomaticCallgraph) {
+            waitForCompletion &= fCallGraph.waitForCompletion();
+        }
+        return waitForCompletion;
+    }
+
+    @Override
+    public boolean waitForCompletion(IProgressMonitor monitor) {
+        boolean waitForCompletion = super.waitForCompletion(monitor);
+        if (fAutomaticCallgraph) {
+            waitForCompletion &= fCallGraph.waitForCompletion(monitor);
+        }
+        return waitForCompletion;
+    }
+
     /**
      * Get the patterns for the process, threads and callstack levels in the
      * state system

analysis/org.eclipse.tracecompass.analysis.callstack.ui.swtbot.tests/src/org/eclipse/tracecompass/analysis/callstack/ui/swtbot/tests/FlameGraphTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Ericsson
+ * Copyright (c) 2016, 2023 Ericsson
  *
  * All rights reserved. This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0 which
@@ -119,8 +119,10 @@ public void before() {
      * Clean up after a test, reset the views and reset the states of the
      * timegraph by pressing reset on all the resets of the legend
      */
+    @Override
     @After
     public void after() {
+        super.after();
         // Calling maximize again will minimize
         FlameGraphView fg = fFg;
         if (fg != null) {