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 <marc-andre.laperle@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/70510
Reviewed-by: Hudson CI
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>

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 10b2b619e0eb020a54833d7c9be3abb7bd86e3e2..b2ec511046df5251cde08f02df56572fadd3a07a 100644 (file)
--- 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<List<ITmfStateInterval>> fullStates, @Nullable List<ITmfStateInterval> prevFullState, @NonNull IProgressMonitor monitor) {
             List<ITimeEvent> 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 8dc4573f3aa4a0e8fa19cfb529055e8201438984..1a31a46dd66aed8938326a753871354095671d70 100644 (file)
--- 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<ILinkEvent> events = getLinkList(getZoomStartTime(), getZoomEndTime(), getResolution(), getMonitor());
-            if (events != null) {
-                fTimeGraphWrapper.getTimeGraphViewer().setLinks(events);
-                redraw();
-            }
             /* Refresh the view-specific markers when zooming */
             List<IMarkerEvent> 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<ITimeEvent> 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();
         }