tmf: Bug 509691: Changes to mutable trace context can be lost
authorPatrick Tasse <patrick.tasse@gmail.com>
Fri, 23 Dec 2016 16:04:13 +0000 (11:04 -0500)
committerPatrick Tasse <patrick.tasse@gmail.com>
Sun, 15 Jan 2017 17:21:29 +0000 (12:21 -0500)
The method TmfTraceManager.updateTraceContext() is now used to apply all
changes to a trace context under synchronization of the trace manager.

Change-Id: Id8cecd92c7f7c4c7203d965ce7fb36a6fed74626
Signed-off-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-on: https://git.eclipse.org/r/87711
Reviewed-by: Hudson CI
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Tested-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/trace/LinuxTraceContext.java
analysis/org.eclipse.tracecompass.analysis.os.linux.ui/src/org/eclipse/tracecompass/internal/analysis/os/linux/ui/views/cpuusage/CpuUsageView.java
analysis/org.eclipse.tracecompass.analysis.os.linux.ui/src/org/eclipse/tracecompass/internal/analysis/os/linux/ui/views/kernelmemoryusage/KernelMemoryUsageView.java
analysis/org.eclipse.tracecompass.analysis.os.linux.ui/src/org/eclipse/tracecompass/internal/analysis/os/linux/ui/views/resources/ResourcesView.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/TmfTraceContext.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/TmfTraceManager.java

index da8019cea44219ee253ce7f6e8b9fa6831e31029..47fc544c65ccec87bfe2cdb8171b9d49f07da22b 100644 (file)
@@ -19,6 +19,7 @@ import org.eclipse.tracecompass.tmf.core.signal.TmfTraceModelSignal;
 import org.eclipse.tracecompass.tmf.core.timestamp.TmfTimeRange;
 import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
 import org.eclipse.tracecompass.tmf.core.trace.TmfTraceContext;
+import org.eclipse.tracecompass.tmf.core.trace.TmfTraceManager;
 
 /**
  * A Linux trace context is a context that stores OS related actions as well as
@@ -35,8 +36,8 @@ public class LinuxTraceContext extends TmfTraceContext {
     /** An invalid thread id */
     public static final int INVALID_THREAD_ID = -1;
 
-    private int fCpu = INVALID_CPU;
-    private int fTid = INVALID_THREAD_ID;
+    private final int fCpu;
+    private final int fTid;
     private final ITmfTrace fTrace;
 
     /**
@@ -56,6 +57,8 @@ public class LinuxTraceContext extends TmfTraceContext {
      */
     public LinuxTraceContext(TmfTimeRange selection, TmfTimeRange windowRange, @Nullable IFile editorFile, @Nullable ITmfFilter filter, ITmfTrace trace) {
         super(selection, windowRange, editorFile, filter);
+        fCpu = INVALID_CPU;
+        fTid = INVALID_THREAD_ID;
         fTrace = trace;
     }
 
@@ -76,11 +79,16 @@ public class LinuxTraceContext extends TmfTraceContext {
     @Override
     public void receive(@NonNull TmfTraceModelSignal signal) {
         if (signal.getHostId().equals(fTrace.getHostId())) {
-            if (signal instanceof TmfThreadSelectedSignal) {
-                fTid = ((TmfThreadSelectedSignal) signal).getThreadId();
-            } else if (signal instanceof TmfCpuSelectedSignal) {
-                fCpu = ((TmfCpuSelectedSignal) signal).getCore();
-            }
+            TmfTraceManager.getInstance().updateTraceContext(fTrace, builder -> {
+                if (builder instanceof LinuxBuilder) {
+                    if (signal instanceof TmfThreadSelectedSignal) {
+                        ((LinuxBuilder) builder).setTid(((TmfThreadSelectedSignal) signal).getThreadId());
+                    } else if (signal instanceof TmfCpuSelectedSignal) {
+                        ((LinuxBuilder) builder).setCpu(((TmfCpuSelectedSignal) signal).getCore());
+                    }
+                }
+                return builder;
+            });
         }
     }
 
@@ -139,5 +147,29 @@ public class LinuxTraceContext extends TmfTraceContext {
         public TmfTraceContext build() {
             return new LinuxTraceContext(this);
         }
+
+        /**
+         * Sets the current CPU.
+         *
+         * @param cpu
+         *            the current CPU
+         * @return this {@code Builder} object
+         */
+        public Builder setCpu(int cpu) {
+            this.cpu = cpu;
+            return this;
+        }
+
+        /**
+         * Sets the current TID.
+         *
+         * @param tid
+         *            the current TID
+         * @return this {@code Builder} object
+         */
+        public Builder setTid(int tid) {
+            this.tid = tid;
+            return this;
+        }
     }
 }
index 85202c459650e09db858a8d71a8cae10b89cf7d9..ac5cf75a435be550716e5cca43691fceaf67a5dd 100644 (file)
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2014, 2016 École Polytechnique de Montréal and others
+ * Copyright (c) 2014, 2017 É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 v1.0 which
@@ -12,8 +12,6 @@
 
 package org.eclipse.tracecompass.internal.analysis.os.linux.ui.views.cpuusage;
 
-import static org.eclipse.tracecompass.common.core.NonNullUtils.checkNotNull;
-
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -133,9 +131,13 @@ public class CpuUsageView extends TmfChartView {
     /**
      * Save a data in the data map of {@link TmfTraceContext}
      */
-    private static void saveData(@NonNull String key, Object data) {
-        TmfTraceContext ctx = TmfTraceManager.getInstance().getCurrentTraceContext();
-        ctx.setData(key, checkNotNull(data));
+    private static void saveData(@NonNull String key, @NonNull Object data) {
+        ITmfTrace trace = TmfTraceManager.getInstance().getActiveTrace();
+        if (trace == null) {
+            return;
+        }
+        TmfTraceManager.getInstance().updateTraceContext(trace,
+                builder -> builder.setData(key, data));
     }
 
     private static Object getData(@NonNull String key) {
index e144f65f0baae1ba9ff93a16da0f7f4a8b3e385e..525206839ebcd10fbcc512c6ed2580cb92a96909 100644 (file)
@@ -1,5 +1,5 @@
 /**********************************************************************
- * Copyright (c) 2016 École Polytechnique de Montréal
+ * Copyright (c) 2016, 2017 É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 v1.0 which
@@ -69,8 +69,12 @@ public class KernelMemoryUsageView extends TmfChartView {
                     KernelMemoryUsageEntry entry = (KernelMemoryUsageEntry) structSelection;
                     fTreeViewerReference.setSelectedThread(entry.getTid());
                     ((KernelMemoryUsageViewer) getChartViewer()).setSelectedThread(entry.getTid());
-                    TmfTraceContext ctx = TmfTraceManager.getInstance().getCurrentTraceContext();
-                    ctx.setData(KERNEL_MEMORY, checkNotNull(entry.getTid()));
+                    ITmfTrace trace = TmfTraceManager.getInstance().getActiveTrace();
+                    if (trace == null) {
+                        return;
+                    }
+                    TmfTraceManager.getInstance().updateTraceContext(trace,
+                            builder -> builder.setData(KERNEL_MEMORY, checkNotNull(entry.getTid())));
                 }
             }
         }
index a51d637b8e380e045bea454a56dd8d2af23e1223..5a329b4f54924ae8c891a5ac31c16ef125b350e5 100644 (file)
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2016 Ericsson, École Polytechnique de Montréal
+ * Copyright (c) 2012, 2017 Ericsson, École Polytechnique de Montréal
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License v1.0 which
@@ -436,8 +436,12 @@ public class ResourcesView extends AbstractStateSystemTimeGraphView {
     @TmfSignalHandler
     public void listenToCpu(TmfCpuSelectedSignal signal) {
         int data = signal.getCore() >= 0 ? signal.getCore() : -1;
-        TmfTraceContext ctx = TmfTraceManager.getInstance().getCurrentTraceContext();
-        ctx.setData(RESOURCES_FOLLOW_CPU, data);
+        ITmfTrace trace = getTrace();
+        if (trace == null) {
+            return;
+        }
+        TmfTraceManager.getInstance().updateTraceContext(trace,
+                builder -> builder.setData(RESOURCES_FOLLOW_CPU, data));
     }
 
 }
index 139cff30e47099b2ff42f25d5c7fd4f4feee0ea8..dae6723f5c06e19515deb93e0c81a8933e8e28e2 100644 (file)
@@ -130,7 +130,11 @@ public class TmfTraceContext implements ITraceContextSignalHandler {
      * @param value
      *            The value of the data
      * @since 2.1
+     * @deprecated Use
+     *             {@link TmfTraceManager#updateTraceContext(ITmfTrace, java.util.function.UnaryOperator)}
+     *             and apply {@link Builder#setData(String, Object)} instead.
      */
+    @Deprecated
     public synchronized void setData(String key, Object value) {
         fData.put(key, value);
     }
@@ -141,7 +145,11 @@ public class TmfTraceContext implements ITraceContextSignalHandler {
      * @param data
      *            The map of data to copy
      * @since 2.1
+     * @deprecated Use
+     *             {@link TmfTraceManager#updateTraceContext(ITmfTrace, java.util.function.UnaryOperator)}
+     *             and apply {@link Builder#setData(Map)} instead.
      */
+    @Deprecated
     public synchronized void setData(Map<String, Object> data) {
         fData.putAll(data);
     }
@@ -249,6 +257,32 @@ public class TmfTraceContext implements ITraceContextSignalHandler {
             this.filter = filter;
             return this;
         }
+
+        /**
+         * Sets a data mapping.
+         *
+         * @param key
+         *            The key of the data
+         * @param value
+         *            The value of the data
+         * @return this {@code Builder} object
+         */
+        public Builder setData(String key, Object value) {
+            this.data.put(key, value);
+            return this;
+        }
+
+        /**
+         * Sets data mappings.
+         *
+         * @param data
+         *            The map of data
+         * @return this {@code Builder} object
+         */
+        public Builder setData(Map<String, Object> data) {
+            this.data.putAll(data);
+            return this;
+        }
     }
 
     @Override
index 049c7ad83cb192412adda8e1937b83f7548cda8a..b178a7d5edd7732c017b204e5763b66ccd3084d1 100644 (file)
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013, 2016 Ericsson
+ * Copyright (c) 2013, 2017 Ericsson and others
  *
  * All rights reserved. This program and the accompanying materials are
  * made available under the terms of the Eclipse Public License v1.0 which
@@ -26,6 +26,7 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.UnaryOperator;
 
 import org.apache.commons.io.FileUtils;
 import org.eclipse.core.resources.IFile;
@@ -183,6 +184,23 @@ public final class TmfTraceManager {
         return curCtx;
     }
 
+    /**
+     * Get the {@link TmfTraceContext} of the given trace.
+     *
+     * @param trace
+     *            The trace or experiment.
+     * @return The trace's context.
+     * @since 2.3
+     */
+    public synchronized TmfTraceContext getTraceContext(ITmfTrace trace) {
+        TmfTraceContext curCtx = fTraces.get(trace);
+        if (curCtx == null) {
+            /* The trace is not opened. */
+            return TmfTraceContext.NULL_CONTEXT;
+        }
+        return curCtx;
+    }
+
     // ------------------------------------------------------------------------
     // Public utility methods
     // ------------------------------------------------------------------------
@@ -303,6 +321,22 @@ public final class TmfTraceManager {
         refreshSupplementaryFiles(trace);
     }
 
+    /**
+     * Update the trace context of a given trace.
+     *
+     * @param trace
+     *            The trace
+     * @param updater
+     *            the function to apply to the trace context's builder
+     * @since 2.3
+     */
+    public synchronized void updateTraceContext(ITmfTrace trace, UnaryOperator<TmfTraceContext.Builder> updater) {
+        TmfTraceContext ctx = getTraceContext(trace);
+        if (!ctx.equals(TmfTraceContext.NULL_CONTEXT)) {
+            fTraces.put(trace, checkNotNull(updater.apply(ctx.builder())).build());
+        }
+    }
+
     // ------------------------------------------------------------------------
     // Signal handlers
     // ------------------------------------------------------------------------
@@ -368,15 +402,12 @@ public final class TmfTraceManager {
      */
     @TmfSignalHandler
     public synchronized void filterApplied(TmfEventFilterAppliedSignal signal) {
-        final ITmfTrace trace = signal.getTrace();
-        TmfTraceContext context = fTraces.get(trace);
-        if (context == null) {
-            throw new RuntimeException();
+        ITmfTrace trace = signal.getTrace();
+        if (trace == null) {
+            return;
         }
-        final TmfTraceContext newContext = context.builder()
-                .setFilter(signal.getEventFilter())
-                .build();
-        fTraces.put(trace, newContext);
+        updateTraceContext(trace, builder ->
+                builder.setFilter(signal.getEventFilter()));
     }
 
     /**
@@ -412,20 +443,10 @@ public final class TmfTraceManager {
         final ITmfTimestamp beginTs = signal.getBeginTime();
         final ITmfTimestamp endTs = signal.getEndTime();
 
-        for (Map.Entry<ITmfTrace, TmfTraceContext> entry : fTraces.entrySet()) {
-            final ITmfTrace trace = entry.getKey();
+        for (ITmfTrace trace : fTraces.keySet()) {
             if (beginTs.intersects(getValidTimeRange(trace)) || endTs.intersects(getValidTimeRange(trace))) {
-                TmfTraceContext prevCtx = checkNotNull(entry.getValue());
-
-                /*
-                 * We want to update the selection range, but keep everything
-                 * else the same as the previous trace context.
-                 */
-                TmfTimeRange newSelectionRange = new TmfTimeRange(beginTs, endTs);
-                TmfTraceContext newCtx = prevCtx.builder()
-                        .setSelection(newSelectionRange)
-                        .build();
-                entry.setValue(newCtx);
+                updateTraceContext(trace, builder ->
+                        builder.setSelection(new TmfTimeRange(beginTs, endTs)));
             }
         }
     }
@@ -442,10 +463,7 @@ public final class TmfTraceManager {
      */
     @TmfSignalHandler
     public synchronized void windowRangeUpdated(final TmfWindowRangeUpdatedSignal signal) {
-        for (Map.Entry<ITmfTrace, TmfTraceContext> entry : fTraces.entrySet()) {
-            final ITmfTrace trace = entry.getKey();
-            final TmfTraceContext prevCtx = checkNotNull(entry.getValue());
-
+        for (ITmfTrace trace : fTraces.keySet()) {
             final TmfTimeRange validTr = getValidTimeRange(trace);
             if (validTr == null) {
                 return;
@@ -453,13 +471,10 @@ public final class TmfTraceManager {
 
             /* Determine the new time range */
             TmfTimeRange targetTr = signal.getCurrentRange().getIntersection(validTr);
-            TmfTimeRange newWindowTr = (targetTr == null ? prevCtx.getWindowRange() : targetTr);
-
-            /* Keep the values from the old context, except for the window range */
-            TmfTraceContext newCtx = prevCtx.builder()
-                    .setWindowRange(newWindowTr)
-                    .build();
-            entry.setValue(newCtx);
+            if (targetTr != null) {
+                updateTraceContext(trace, builder ->
+                        builder.setWindowRange(targetTr));
+            }
         }
     }
 
This page took 0.02993 seconds and 5 git commands to generate.