From 6d28f3e811a00bfb4bda9282fbb095040da9674d Mon Sep 17 00:00:00 2001 From: Marc-Andre Laperle Date: Tue, 12 Apr 2016 15:38:02 -0400 Subject: [PATCH] lttng: Make ZoomThread more thread-safe (also helps ResourcesViewTest) In AbstractTimeGraphView and extenders, when a ZoomThread applies its results (links, markers), there is no guarantee of the order in the presence of multiple ZoomThreads. For example, in the case where a new ZoomThread is started and an old one is still executing, the old one might be canceled so late that it will still apply its results and do so *after* the new ZoomThread completes. This change introduces some synchronization to make sure the results are applied only in the case of the last ZoomThread spawned. To reproduce this issue, go to AbstractTimeGraphView.getTraceMarkerList and add a Thread.sleep(5000); before and after the isCanceled() check. Because the test takes much longer, you need to increase the timeout in ResourcesViewTest.timeGraphIsReadyCondition. Then you can run ResourcesViewTest in a loop using SWTBotStressTests and after a few iterations, it should fail. Change-Id: If41f7b264676c04538cdc39c5c9bb40507b7e521 Signed-off-by: Marc-Andre Laperle Reviewed-on: https://git.eclipse.org/r/70510 Reviewed-by: Hudson CI Reviewed-by: Matthew Khouzam --- .../AbstractStateSystemTimeGraphView.java | 17 +++++-- .../timegraph/AbstractTimeGraphView.java | 48 +++++++++++++++---- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractStateSystemTimeGraphView.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractStateSystemTimeGraphView.java index 10b2b619e0..b2ec511046 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractStateSystemTimeGraphView.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractStateSystemTimeGraphView.java @@ -114,10 +114,12 @@ public abstract class AbstractStateSystemTimeGraphView extends AbstractTimeGraph } } if (!getMonitor().isCanceled()) { - getTimeGraphViewer().setLinks(links); /* Refresh the trace-specific markers when zooming */ markers.addAll(getTraceMarkerList(getZoomStartTime(), getZoomEndTime(), getResolution(), getMonitor())); - getTimeGraphViewer().setMarkers(markers); + applyResults(() -> { + getTimeGraphViewer().setLinks(links); + getTimeGraphViewer().setMarkers(markers); + }); } } @@ -160,9 +162,14 @@ public abstract class AbstractStateSystemTimeGraphView extends AbstractTimeGraph private void zoom(@NonNull TimeGraphEntry entry, ITmfStateSystem ss, @NonNull List> fullStates, @Nullable List prevFullState, @NonNull IProgressMonitor monitor) { List eventList = getEventList(entry, ss, fullStates, prevFullState, monitor); if (eventList != null) { - for (ITimeEvent event : eventList) { - entry.addZoomedEvent(event); - } + applyResults(() -> { + for (ITimeEvent event : eventList) { + if (monitor.isCanceled()) { + return; + } + entry.addZoomedEvent(event); + } + }); } for (ITimeGraphEntry child : entry.getChildren()) { if (monitor.isCanceled()) { diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java index 8dc4573f3a..1a31a46dd6 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/views/timegraph/AbstractTimeGraphView.java @@ -157,6 +157,8 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA private AtomicInteger fDirty = new AtomicInteger(); + private final Object fZoomThreadResultLock = new Object(); + /** The selected trace */ private ITmfTrace fTrace; @@ -774,6 +776,24 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA fDirty.decrementAndGet(); } + /** + * Applies the results of the ZoomThread calculations. + * + * Note: This method makes sure that only the results of the last + * created ZoomThread are applied. + * + * @param runnable + * the code to run in order to apply the results + * @since 2.0 + */ + protected void applyResults(Runnable runnable) { + synchronized (fZoomThreadResultLock) { + if (this == fZoomThread) { + runnable.run(); + } + } + } + /** * Run the zoom operation. * @since 2.0 @@ -802,25 +822,30 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA } /* Refresh the arrows when zooming */ List events = getLinkList(getZoomStartTime(), getZoomEndTime(), getResolution(), getMonitor()); - if (events != null) { - fTimeGraphWrapper.getTimeGraphViewer().setLinks(events); - redraw(); - } /* Refresh the view-specific markers when zooming */ List markers = new ArrayList<>(getViewMarkerList(getZoomStartTime(), getZoomEndTime(), getResolution(), getMonitor())); /* Refresh the trace-specific markers when zooming */ markers.addAll(getTraceMarkerList(getZoomStartTime(), getZoomEndTime(), getResolution(), getMonitor())); - fTimeGraphWrapper.getTimeGraphViewer().setMarkers(markers); - redraw(); + applyResults(() -> { + if (events != null) { + fTimeGraphWrapper.getTimeGraphViewer().setLinks(events); + } + fTimeGraphWrapper.getTimeGraphViewer().setMarkers(markers); + redraw(); + }); } private void zoom(@NonNull TimeGraphEntry entry, @NonNull IProgressMonitor monitor) { if (getZoomStartTime() <= fStartTime && getZoomEndTime() >= fEndTime) { - entry.setZoomedEventList(null); + applyResults(() -> { + entry.setZoomedEventList(null); + }); } else { List zoomedEventList = getEventList(entry, getZoomStartTime(), getZoomEndTime(), getResolution(), monitor); if (zoomedEventList != null) { - entry.setZoomedEventList(zoomedEventList); + applyResults(() -> { + entry.setZoomedEventList(zoomedEventList); + }); } } redraw(); @@ -1919,7 +1944,12 @@ public abstract class AbstractTimeGraphView extends TmfView implements ITmfTimeA long resolution = Math.max(1, (clampedEndTime - clampedStartTime) / fDisplayWidth); fZoomThread = createZoomThread(clampedStartTime, clampedEndTime, resolution, restart); if (fZoomThread != null) { - fZoomThread.start(); + // Don't start a new thread right away if results are being applied + // from an old ZoomThread. Otherwise, the old results might + // overwrite the new results if it finishes after. + synchronized (fZoomThreadResultLock) { + fZoomThread.start(); + } } else { fDirty.decrementAndGet(); } -- 2.34.1