tmf: annotate TmfContext#location as nullable

This fixes a potential race condition if the context is changed
after a null check but before a dereference. In the current
implementation, it is extremely unlikely to happen, but this
may save later investigation time.

Change-Id: I9083d16fd8e3728fe829583f4de78384109a91e6
Signed-off-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/82325
Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
Tested-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-by: Hudson CI

diff --git a/btf/org.eclipse.tracecompass.btf.core/src/org/eclipse/tracecompass/btf/core/trace/BtfTrace.java b/btf/org.eclipse.tracecompass.btf.core/src/org/eclipse/tracecompass/btf/core/trace/BtfTrace.java
index b881b9bb216b2a2ab47aa75ed51ad0d2f438e2aa..8d938a083c6c696fb8cb5f467a607da8ad6c1ba2 100644 (file)
--- a/btf/org.eclipse.tracecompass.btf.core/src/org/eclipse/tracecompass/btf/core/trace/BtfTrace.java
+++ b/btf/org.eclipse.tracecompass.btf.core/src/org/eclipse/tracecompass/btf/core/trace/BtfTrace.java
@@ -144,7 +144,8 @@ public class BtfTrace extends TmfTrace implements ITmfPersistentlyIndexable, ITm
                 fProperties.put(CREATIONDATE, fCreationDate);
 
                 try {
-                    // DateFormats are inherently unsafe for multithreaded use so we can't make this a field. Just in case.
+                    // DateFormats are inherently unsafe for multithreaded use
+                    // so we can't make this a field. Just in case.
                     final SimpleDateFormat ISO8601DATEFORMAT = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssX"); //$NON-NLS-1$
                     Date dateTime = ISO8601DATEFORMAT.parse(fCreationDate);
                     fTsOffset = dateTime.getTime() * MICROSECONDS_IN_A_SECOND;
@@ -363,9 +364,10 @@ public class BtfTrace extends TmfTrace implements ITmfPersistentlyIndexable, ITm
         }
 
         final TmfContext context = (TmfContext) tmfContext;
-        if (context.getLocation() == null
-                || !(context.getLocation().getLocationInfo() instanceof Long)
-                || NULL_LOCATION.equals(context.getLocation())) {
+        ITmfLocation location = context.getLocation();
+        if (location == null
+                || !(location.getLocationInfo() instanceof Long)
+                || NULL_LOCATION.equals(location)) {
             return null;
         }
 
@@ -381,21 +383,24 @@ public class BtfTrace extends TmfTrace implements ITmfPersistentlyIndexable, ITm
      * @return the event from a given line
      */
     private ITmfEvent parseLine(TmfContext context) {
-        try {
-            if (!context.getLocation().getLocationInfo().equals(fFileInput.getFilePointer())) {
-                seekEvent(context.getLocation());
+        ITmfLocation location = context.getLocation();
+        if (location != null) {
+            try {
+                if (!location.getLocationInfo().equals(fFileInput.getFilePointer())) {
+                    seekEvent(location);
+                }
+            } catch (IOException e1) {
+                seekEvent(location);
             }
-        } catch (IOException e1) {
-            seekEvent(context.getLocation());
-        }
-        String line;
-        try {
-            line = fFileInput.readLine();
-            return parseLine(context.getRank(), line);
+            String line;
+            try {
+                line = fFileInput.readLine();
+                return parseLine(context.getRank(), line);
 
-        } catch (IOException e) {
+            } catch (IOException e) {
+                Activator.logError(e.getMessage(), e);
+            }
         }
-
         return null;
     }
 
@@ -502,9 +507,11 @@ public class BtfTrace extends TmfTrace implements ITmfPersistentlyIndexable, ITm
         if (signal.getTrace() == this) {
             try {
                 synchronized (this) {
-                    // Reset the file handle in case it has reached the end of the
-                    // file already. Otherwise, it will not be able to read new data
-                    // pass the previous end.
+                    /*
+                     * Reset the file handle in case it has reached the end of
+                     * the file already. Otherwise, it will not be able to read
+                     * new data pass the previous end.
+                     */
                     initFile();
                 }
             } catch (TmfTraceException e) {

diff --git a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomTxtTrace.java b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomTxtTrace.java
index 318a56660b1d3676040b694c22ad0b45b37130b2..4327bdfc454ada032f43ae338bc77819321eb245 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomTxtTrace.java
+++ b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomTxtTrace.java
@@ -264,7 +264,8 @@ public class CustomTxtTrace extends TmfTrace implements ITmfPersistentlyIndexabl
         }
 
         final CustomTxtTraceContext context = (CustomTxtTraceContext) tmfContext;
-        if (context.getLocation() == null || !(context.getLocation().getLocationInfo() instanceof Long) || NULL_LOCATION.equals(context.getLocation())) {
+        ITmfLocation location = context.getLocation();
+        if (location == null || !(location.getLocationInfo() instanceof Long) || NULL_LOCATION.equals(location)) {
             return null;
         }
 

diff --git a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomXmlTrace.java b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomXmlTrace.java
index e562fd880e6f232d9669e073e17fc10654222ded..50298e6624a4511c18e6ea9470a5dd1f771fd13c 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomXmlTrace.java
+++ b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/parsers/custom/CustomXmlTrace.java
@@ -266,15 +266,16 @@ public class CustomXmlTrace extends TmfTrace implements ITmfPersistentlyIndexabl
         }
 
         final CustomXmlTraceContext context = (CustomXmlTraceContext) tmfContext;
-        if (context.getLocation() == null || !(context.getLocation().getLocationInfo() instanceof Long) || NULL_LOCATION.equals(context.getLocation())) {
+        ITmfLocation location = context.getLocation();
+        if (location == null || !(location.getLocationInfo() instanceof Long) || NULL_LOCATION.equals(location)) {
             return null;
         }
 
         CustomXmlEvent event = null;
         try {
             // Below +1 for the <
-            if (fFile.getFilePointer() != (Long) context.getLocation().getLocationInfo() + 1) {
-                fFile.seek((Long) context.getLocation().getLocationInfo() + 1);
+            if (fFile.getFilePointer() != (Long) location.getLocationInfo() + 1) {
+                fFile.seek((Long) location.getLocationInfo() + 1);
             }
             final StringBuffer elementBuffer = new StringBuffer("<"); //$NON-NLS-1$
             readElement(elementBuffer, fFile);

diff --git a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/TmfContext.java b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/TmfContext.java
index d62c5ee752dcd9239b2fded7784fef075a5b2db5..8b99e3567a225ca55c216eb108144bd370750a76 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/TmfContext.java
+++ b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/TmfContext.java
@@ -14,6 +14,9 @@
 
 package org.eclipse.tracecompass.tmf.core.trace;
 
+import java.util.Objects;
+
+import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.tracecompass.tmf.core.trace.location.ITmfLocation;
 
 /**
@@ -33,7 +36,7 @@ public class TmfContext implements ITmfContext {
     // ------------------------------------------------------------------------
 
     // The trace location
-    private ITmfLocation fLocation;
+    private @Nullable ITmfLocation fLocation;
 
     // The event rank
     private long fRank;
@@ -87,7 +90,7 @@ public class TmfContext implements ITmfContext {
     // ------------------------------------------------------------------------
 
     @Override
-    public ITmfLocation getLocation() {
+    public @Nullable ITmfLocation getLocation() {
         return fLocation;
     }
 
@@ -128,11 +131,7 @@ public class TmfContext implements ITmfContext {
 
     @Override
     public int hashCode() {
-        final int prime = 31;
-        int result = 1;
-        result = prime * result + ((fLocation == null) ? 0 : fLocation.hashCode());
-        result = prime * result + (int) (fRank ^ (fRank >>> 32));
-        return result;
+        return Objects.hash(fRank, fLocation);
     }
 
     @Override
@@ -147,14 +146,10 @@ public class TmfContext implements ITmfContext {
             return false;
         }
         final TmfContext other = (TmfContext) obj;
-        if (fLocation == null) {
-            if (other.fLocation != null) {
-                return false;
-            }
-        } else if (!fLocation.equals(other.fLocation)) {
+        if (fRank != other.fRank) {
             return false;
         }
-        if (fRank != other.fRank) {
+        if (!Objects.equals(fLocation, other.fLocation)) {
             return false;
         }
         return true;

diff --git a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/experiment/TmfExperiment.java b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/experiment/TmfExperiment.java
index b2e3cea3a9462e4f73e7fcabf79c646716d784bc..a426fd3732d6d4c90c244ffe0a5a588fcda40c07 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/experiment/TmfExperiment.java
+++ b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/experiment/TmfExperiment.java
@@ -299,9 +299,7 @@ public class TmfExperiment extends TmfTrace implements ITmfPersistentlyIndexable
         int length = getNbChildren();
 
         // Initialize the location array if necessary
-        TmfLocationArray locationArray = ((location == null) ?
-                new TmfLocationArray(length) :
-                ((TmfExperimentLocation) location).getLocationInfo());
+        TmfLocationArray locationArray = ((location == null) ? new TmfLocationArray(length) : ((TmfExperimentLocation) location).getLocationInfo());
 
         ITmfLocation[] locations = locationArray.getLocations();
         long[] ranks = locationArray.getRanks();
@@ -424,10 +422,13 @@ public class TmfExperiment extends TmfTrace implements ITmfPersistentlyIndexable
                 }
 
                 // Update the experiment location
-                TmfLocationArray locationArray = new TmfLocationArray(
-                        ((TmfExperimentLocation) expContext.getLocation()).getLocationInfo(),
-                        trace, traceContext.getLocation(), traceContext.getRank());
-                expContext.setLocation(new TmfExperimentLocation(locationArray));
+                ITmfLocation location = expContext.getLocation();
+                if (location instanceof TmfExperimentLocation) {
+                    TmfLocationArray locationArray = new TmfLocationArray(
+                            ((TmfExperimentLocation) location).getLocationInfo(),
+                            trace, traceContext.getLocation(), traceContext.getRank());
+                    expContext.setLocation(new TmfExperimentLocation(locationArray));
+                }
             }
         }
 
@@ -610,8 +611,7 @@ public class TmfExperiment extends TmfTrace implements ITmfPersistentlyIndexable
                         }
                         safeTimestamp = endTimestamp;
                         if (timeRange != null) {
-                            final TmfTraceRangeUpdatedSignal signal =
-                                    new TmfTraceRangeUpdatedSignal(TmfExperiment.this, TmfExperiment.this, timeRange);
+                            final TmfTraceRangeUpdatedSignal signal = new TmfTraceRangeUpdatedSignal(TmfExperiment.this, TmfExperiment.this, timeRange);
                             broadcast(signal);
                         }
                     }

diff --git a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/text/TextTrace.java b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/text/TextTrace.java
index 8c13d91e489ab9247cbd9fcba77ae38bd2b37f0c..48d14770308f6b2553dcce2b2444a53074357b89 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/text/TextTrace.java
+++ b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/text/TextTrace.java
@@ -275,7 +275,8 @@ public abstract class TextTrace<T extends TextTraceEvent> extends TmfTrace imple
             return null;
         }
         TextTraceContext context = tmfContext;
-        if (context.getLocation() == null || !(context.getLocation().getLocationInfo() instanceof Long) || NULL_LOCATION.equals(context.getLocation())) {
+        ITmfLocation location = context.getLocation();
+        if (location == null || !(location.getLocationInfo() instanceof Long) || NULL_LOCATION.equals(location)) {
             return null;
         }