Skip to content

Commit

Permalink
analysis: Fix resource leak in CompositeHostModel
Browse files Browse the repository at this point in the history
The original design had some flaws:

- The CompositeHostModel is instantiated in many cases, for example when
a kernel analysis module is created. But it is only disposed from
ModelManager when an InstrumentedCallStackAnalysis is disposed, which
only happens if a LTTng UST trace is opened in the experiment. Therefore
the CompositeHostModel leaks when opening a single LTTng kernel trace.

- When the InstrumentedCallStackAnalysis is disposed, all models are
disposed, regardless of whether the models are still in use by another
opened trace.

- The CompositeHostModel is disposed, deregistering itself from the
signal manager, before it has had time to handle the TmfTraceClosed
signal, which means this signal handler's cleanup tasks are not done.

The implemented fix implements these changes:

- Remove the responsibility from InstrumentedCallStackAnalysis to
dispose the models.

- Add the responsibility to CompositeHostModel itself, in the
TmfTraceClosed signal handler, to dispose all models, but only if there
are no remaining opened traces. This means the models remain in memory
when other traces are still opened, whether they are associated with
these traces or not.

Change-Id: I2fb8c836c5498266b757b115466be13d8e02b05d
Signed-off-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-on: https://git.eclipse.org/r/c/tracecompass/org.eclipse.tracecompass/+/202404
Tested-by: Trace Compass Bot <tracecompass-bot@eclipse.org>
Tested-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
(cherry picked from commit c92ebcc)
Reviewed-on: https://git.eclipse.org/r/c/tracecompass/org.eclipse.tracecompass/+/201994
  • Loading branch information
PatrickTasse committed Jun 13, 2023
1 parent a839e16 commit fc4226b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
Expand Up @@ -28,10 +28,9 @@
import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.CallGraphAnalysis;
import org.eclipse.tracecompass.internal.analysis.callstack.core.callgraph.ICallGraphProvider;
import org.eclipse.tracecompass.internal.analysis.callstack.core.callstack.CallStackHostUtils;
import org.eclipse.tracecompass.internal.analysis.callstack.core.callstack.CallStackSeries;
import org.eclipse.tracecompass.internal.analysis.callstack.core.callstack.CallStackHostUtils.TraceHostIdResolver;
import org.eclipse.tracecompass.internal.analysis.callstack.core.callstack.CallStackSeries;
import org.eclipse.tracecompass.internal.analysis.callstack.core.callstack.CallStackSeries.IThreadIdResolver;
import org.eclipse.tracecompass.internal.analysis.callstack.core.model.ModelManager;
import org.eclipse.tracecompass.internal.analysis.callstack.core.tree.IWeightedTreeGroupDescriptor;
import org.eclipse.tracecompass.segmentstore.core.ISegment;
import org.eclipse.tracecompass.segmentstore.core.ISegmentStore;
Expand Down Expand Up @@ -251,7 +250,6 @@ public String getTitle() {
public void dispose() {
super.dispose();
fCallGraph.dispose();
ModelManager.disposeModels();
}

@Override
Expand Down
Expand Up @@ -318,6 +318,11 @@ public void traceClosed(final TmfTraceClosedSignal signal) {
}
}
});

/* Dispose all models when the last trace is closed. */
if (TmfTraceManager.getInstance().getOpenedTraces().isEmpty()) {
ModelManager.disposeModels();
}
}

@Override
Expand Down

0 comments on commit fc4226b

Please sign in to comment.