From 7255573c13c43ecd96e204c52928db14c9bfe28c Mon Sep 17 00:00:00 2001 From: Patrick Tasse Date: Mon, 30 May 2016 17:20:10 -0400 Subject: [PATCH] tmf: Bug 494952: Remove deadlock in Time Chart view The handling of signal TmfTraceUpdated is now done in a ProcessTraceThread. There is only one such thread per trace active at a time. If an update is requested while a thread is active, the thread is marked to be restarted after its current iteration completion. The starting or restarting of a ProcessTraceThread is consolidated in a method and synchronized. The ProcessTraceThread now always processes the full time range of the trace, even when the view is opened with a trace already opened. A missing synchronization on DecorateThread concurrent access is added. Method ITimeDataProvider.resetStartFinishTime(boolean) is added to allow the TimeChartView to reset its window range without notifying listeners. Change-Id: I0d49d2712af4e5c645bd61bfde46ba37fc42779c Signed-off-by: Patrick Tasse Reviewed-on: https://git.eclipse.org/r/73996 Reviewed-by: Hudson CI Reviewed-by: Matthew Khouzam Reviewed-by: Bernd Hufmann Tested-by: Matthew Khouzam --- .../tmf/ui/views/timechart/TimeChartView.java | 55 ++++++++++++++----- .../ui/widgets/timegraph/TimeGraphViewer.java | 13 +++++ .../timegraph/widgets/ITimeDataProvider.java | 17 +++++- .../TimeDataProviderCyclesConverter.java | 10 +++- 4 files changed, 80 insertions(+), 15 deletions(-) diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timechart/TimeChartView.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timechart/TimeChartView.java index ff31a99cb4..28a198ed71 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timechart/TimeChartView.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timechart/TimeChartView.java @@ -82,6 +82,7 @@ public class TimeChartView extends TmfView implements ITimeGraphRangeListener, I private final List fTimeAnalysisEntries = new ArrayList<>(); private final Map fDecorationProviders = new HashMap<>(); private final List fDecorateThreads = new ArrayList<>(); + private final Map fProcessTraceThreads = new HashMap<>(); private long fStartTime = 0; private long fStopTime = Long.MAX_VALUE; private boolean fRefreshBusy = false; @@ -120,8 +121,7 @@ public class TimeChartView extends TmfView implements ITimeGraphRangeListener, I TimeChartAnalysisEntry timeAnalysisEntry = new TimeChartAnalysisEntry(trace, fDisplayWidth * 2); fTimeAnalysisEntries.add(timeAnalysisEntry); fDecorationProviders.put(trace, new TimeChartDecorationProvider(bookmarksFile)); - Thread thread = new ProcessTraceThread(timeAnalysisEntry); - thread.start(); + startProcessTraceThread(timeAnalysisEntry); } fViewer.setInput(fTimeAnalysisEntries.toArray(new TimeChartAnalysisEntry[0])); @@ -132,8 +132,10 @@ public class TimeChartView extends TmfView implements ITimeGraphRangeListener, I @Override public void dispose() { ResourcesPlugin.getWorkspace().removeResourceChangeListener(this); - for (DecorateThread thread : fDecorateThreads) { - thread.cancel(); + synchronized (fDecorateThreads) { + for (DecorateThread thread : fDecorateThreads) { + thread.cancel(); + } } ColorSettingsManager.removeColorSettingsListener(this); super.dispose(); @@ -144,22 +146,50 @@ public class TimeChartView extends TmfView implements ITimeGraphRangeListener, I fViewer.setFocus(); } + private void startProcessTraceThread(TimeChartAnalysisEntry entry) { + synchronized (fProcessTraceThreads) { + ProcessTraceThread thread = fProcessTraceThreads.get(entry.getTrace()); + if (thread != null) { + thread.restart(); + } else { + thread = new ProcessTraceThread(entry); + fProcessTraceThreads.put(entry.getTrace(), thread); + thread.start(); + } + } + } + private class ProcessTraceThread extends Thread { private final TimeChartAnalysisEntry fTimeAnalysisEntry; + private boolean fRestart; public ProcessTraceThread(TimeChartAnalysisEntry timeAnalysisEntry) { super("ProcessTraceJob:" + timeAnalysisEntry.getName()); //$NON-NLS-1$ fTimeAnalysisEntry = timeAnalysisEntry; } + public void restart() { + synchronized (fProcessTraceThreads) { + fRestart = true; + } + } + @Override public void run() { - TmfTimeRange range = TmfTraceManager.getInstance().getCurrentTraceContext().getWindowRange(); - - updateTraceEntry(fTimeAnalysisEntry, Long.MAX_VALUE, - range.getStartTime().toNanos(), - range.getEndTime().toNanos()); + while (true) { + updateTraceEntry(fTimeAnalysisEntry, Long.MAX_VALUE, + TmfTimestamp.BIG_BANG.toNanos(), + TmfTimestamp.BIG_CRUNCH.toNanos()); + synchronized (fProcessTraceThreads) { + if (fRestart) { + fRestart = false; + } else { + fProcessTraceThreads.remove(fTimeAnalysisEntry.getTrace()); + return; + } + } + } } } @@ -240,7 +270,7 @@ public class TimeChartView extends TmfView implements ITimeGraphRangeListener, I return; } fViewer.setInput(fTimeAnalysisEntries.toArray(new TimeChartAnalysisEntry[0])); - fViewer.resetStartFinishTime(); + fViewer.resetStartFinishTime(false); synchronized (fSyncObj) { fRefreshBusy = false; if (fRefreshPending) { @@ -598,8 +628,7 @@ public class TimeChartView extends TmfView implements ITimeGraphRangeListener, I timeAnalysisEntry = new TimeChartAnalysisEntry(trace, fDisplayWidth * 2); fTimeAnalysisEntries.add(timeAnalysisEntry); fDecorationProviders.put(trace, new TimeChartDecorationProvider(bookmarksFile)); - Thread thread = new ProcessTraceThread(timeAnalysisEntry); - thread.start(); + startProcessTraceThread(timeAnalysisEntry); } refreshViewer(); } @@ -667,7 +696,7 @@ public class TimeChartView extends TmfView implements ITimeGraphRangeListener, I for (int i = 0; i < fTimeAnalysisEntries.size(); i++) { TimeChartAnalysisEntry timeAnalysisEntry = fTimeAnalysisEntries.get(i); if (timeAnalysisEntry.getTrace().equals(trace)) { - updateTraceEntry(timeAnalysisEntry, Long.MAX_VALUE, 0, Long.MAX_VALUE); + startProcessTraceThread(timeAnalysisEntry); break; } } diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/TimeGraphViewer.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/TimeGraphViewer.java index f4060c2992..976eafec7b 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/TimeGraphViewer.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/TimeGraphViewer.java @@ -979,6 +979,19 @@ public class TimeGraphViewer implements ITimeDataProvider, IMarkerAxisListener, fTimeRangeFixed = false; } + /** + * @since 2.0 + */ + @Override + public void resetStartFinishTime(boolean notify) { + if (notify) { + setStartFinishTimeNotify(fTime0Bound, fTime1Bound); + } else { + setStartFinishTime(fTime0Bound, fTime1Bound); + } + fTimeRangeFixed = false; + } + @Override public void setSelectedTimeNotify(long time, boolean ensureVisible) { setSelectedTimeInt(time, ensureVisible, true); diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/ITimeDataProvider.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/ITimeDataProvider.java index e83509f2ea..b60f22719e 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/ITimeDataProvider.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/ITimeDataProvider.java @@ -1,5 +1,5 @@ /***************************************************************************** - * Copyright (c) 2007, 2015 Intel Corporation, Ericsson + * Copyright (c) 2007, 2016 Intel Corporation, Ericsson * * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -176,6 +176,21 @@ public interface ITimeDataProvider { */ void resetStartFinishTime(); + /** + * Reset the start and end times. + * + * @param notify + * if true, notify the registered listeners + * @since 2.0 + */ + default void resetStartFinishTime(boolean notify) { + if (notify) { + setStartFinishTimeNotify(getMinTime(), getMaxTime()); + } else { + setStartFinishTime(getMinTime(), getMaxTime()); + } + } + /** * @return The names' width */ diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/TimeDataProviderCyclesConverter.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/TimeDataProviderCyclesConverter.java index bb69d7ccd3..7393de4ec2 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/TimeDataProviderCyclesConverter.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/widgets/timegraph/widgets/TimeDataProviderCyclesConverter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2014, 2015 Ericsson + * Copyright (c) 2014, 2016 Ericsson * * All rights reserved. This program and the accompanying materials are * made available under the terms of the Eclipse Public License v1.0 which @@ -159,6 +159,14 @@ public class TimeDataProviderCyclesConverter implements ITimeDataProviderConvert fProvider.resetStartFinishTime(); } + /** + * @since 2.0 + */ + @Override + public void resetStartFinishTime(boolean notify) { + fProvider.resetStartFinishTime(notify); + } + @Override public int getNameSpace() { return fProvider.getNameSpace(); -- 2.34.1