ctf: make CtfStreamInput not hold onto any resources
authorMatthew Khouzam <matthew.khouzam@ericsson.com>
Thu, 28 Aug 2014 19:24:30 +0000 (15:24 -0400)
committerMatthew Khouzam <matthew.khouzam@ericsson.com>
Tue, 2 Sep 2014 21:54:19 +0000 (17:54 -0400)
Change-Id: I61cb09c4c325479115084c54bba03d4746cd6610
Reviewed-on: https://git.eclipse.org/r/32553
Tested-by: Hudson CI
Reviewed-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamInputTest.java
org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamTest.java
org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInput.java
org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputPacketReader.java
org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputReader.java
org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTrace.java
org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTraceReader.java

index 4175b1661108b01feee035b2cf87735e8c7bbf54..efce3c64cd5027dcd87fa1ce3b121f88e39a8d0c 100644 (file)
@@ -21,6 +21,7 @@ import static org.junit.Assume.assumeTrue;
 import java.io.File;
 import java.io.FilenameFilter;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.linuxtools.ctf.core.event.types.IDefinition;
 import org.eclipse.linuxtools.ctf.core.tests.shared.CtfTestTrace;
 import org.eclipse.linuxtools.ctf.core.trace.CTFReaderException;
@@ -55,9 +56,10 @@ public class CTFStreamInputTest {
         fixture.setTimestampEnd(1L);
     }
 
+    @NonNull
     private static File createFile() {
         File path = new File(testTrace.getPath());
-        return path.listFiles(new FilenameFilter() {
+        final File[] listFiles = path.listFiles(new FilenameFilter() {
             @Override
             public boolean accept(File dir, String name) {
                 if (name.contains("hann")) {
@@ -65,7 +67,11 @@ public class CTFStreamInputTest {
                 }
                 return false;
             }
-        })[0];
+        });
+        assertNotNull(listFiles);
+        final File returnFile = listFiles[0];
+        assertNotNull(returnFile);
+        return returnFile;
     }
 
     /**
index 3be9703cd82ab4ffd0de8614aaf77df58f3eec66..62bcf11b47449b02e6eadc475b0182c2b660651a 100644 (file)
@@ -20,6 +20,7 @@ import java.io.FilenameFilter;
 import java.io.IOException;
 import java.util.Set;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.linuxtools.ctf.core.event.types.IDeclaration;
 import org.eclipse.linuxtools.ctf.core.event.types.StructDeclaration;
 import org.eclipse.linuxtools.ctf.core.tests.shared.CtfTestTrace;
@@ -67,13 +68,14 @@ public class CTFStreamTest {
     }
 
     @After
-    public void tearDown() throws IOException{
+    public void tearDown() throws IOException {
         fInput.close();
     }
 
+    @NonNull
     private static File createFile() {
         File path = new File(testTrace.getPath());
-        return path.listFiles(new FilenameFilter() {
+        final File[] listFiles = path.listFiles(new FilenameFilter() {
             @Override
             public boolean accept(File dir, String name) {
                 if (name.contains("hann")) {
@@ -81,7 +83,11 @@ public class CTFStreamTest {
                 }
                 return false;
             }
-        })[0];
+        });
+        assertNotNull(listFiles);
+        final File returnFile = listFiles[0];
+        assertNotNull(returnFile);
+        return returnFile;
     }
 
     /**
index 848ed43358cf9ebb3f99caffaf1399673384866d..157b1049c0857fdb392fa6ef55add8c0ecd21f21 100644 (file)
 package org.eclipse.linuxtools.ctf.core.trace;
 
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.nio.ByteBuffer;
 import java.nio.MappedByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileChannel.MapMode;
+import java.nio.file.StandardOpenOption;
 import java.util.UUID;
 
 import org.eclipse.jdt.annotation.NonNull;
@@ -45,6 +43,7 @@ import org.eclipse.linuxtools.internal.ctf.core.trace.StreamInputPacketIndexEntr
  *
  * @since 3.0
  */
+// TODO: remove AutoCloseable
 public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
 
     // ------------------------------------------------------------------------
@@ -56,14 +55,10 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
      */
     private final CTFStream fStream;
 
-    /**
-     * FileChannel to the trace file
-     */
-    private final FileChannel fFileChannel;
-
     /**
      * Information on the file (used for debugging)
      */
+    @NonNull
     private final File fFile;
 
     /**
@@ -88,11 +83,6 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
      */
     private long fLostSoFar = 0;
 
-    /**
-     * File input stream, the parent input file
-     */
-    private final FileInputStream fFileInputStream;
-
     // ------------------------------------------------------------------------
     // Constructors
     // ------------------------------------------------------------------------
@@ -104,26 +94,15 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
      *            The stream to which this StreamInput belongs to.
      * @param file
      *            Information about the trace file (for debugging purposes).
-     * @throws CTFReaderException
-     *             The file must exist
      */
-    public CTFStreamInput(CTFStream stream, File file) throws CTFReaderException {
+    public CTFStreamInput(CTFStream stream, @NonNull File file) {
         fStream = stream;
         fFile = file;
-        try {
-            fFileInputStream = new FileInputStream(file);
-        } catch (FileNotFoundException e) {
-            throw new CTFReaderException(e);
-        }
-
-        fFileChannel = fFileInputStream.getChannel();
         fIndex = new StreamInputPacketIndex();
     }
 
     @Override
     public void close() throws IOException {
-        fFileChannel.close();
-        fFileInputStream.close();
     }
 
     // ------------------------------------------------------------------------
@@ -241,8 +220,7 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
 
             StreamInputPacketIndexEntry packetIndex = new StreamInputPacketIndexEntry(
                     currentPos);
-            createPacketIndexEntry(fileSize, currentPos, packetIndex,
-                    fTracePacketHeaderDecl, fStreamPacketContextDecl);
+            createPacketIndexEntry(fileSize, currentPos, packetIndex);
             fIndex.addEntry(packetIndex);
             return true;
         }
@@ -253,33 +231,10 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
         return fFile.length();
     }
 
-    private long createPacketIndexEntry(long fileSizeBytes,
-            long packetOffsetBytes, StreamInputPacketIndexEntry packetIndex,
-            StructDeclaration tracePacketHeaderDecl,
-            StructDeclaration streamPacketContextDecl)
+    private long createPacketIndexEntry(long fileSizeBytes, long packetOffsetBytes, StreamInputPacketIndexEntry packetIndex)
             throws CTFReaderException {
 
-        /*
-         * create a packet bit buffer to read the packet header
-         */
-        BitBuffer bitBuffer = new BitBuffer(createPacketBitBuffer(fileSizeBytes, packetOffsetBytes, packetIndex));
-        bitBuffer.setByteOrder(getStream().getTrace().getByteOrder());
-        /*
-         * Read the trace packet header if it exists.
-         */
-        if (tracePacketHeaderDecl != null) {
-            parseTracePacketHeader(tracePacketHeaderDecl, bitBuffer);
-        }
-
-        /*
-         * Read the stream packet context if it exists.
-         */
-        if (streamPacketContextDecl != null) {
-            parsePacketContext(fileSizeBytes, streamPacketContextDecl,
-                    bitBuffer, packetIndex);
-        } else {
-            setPacketContextNull(fileSizeBytes, packetIndex);
-        }
+        long pos = readPacketHeader(fileSizeBytes, packetOffsetBytes, packetIndex);
 
         /* Basic validation */
         if (packetIndex.getContentSizeBits() > packetIndex.getPacketSizeBits()) {
@@ -294,7 +249,7 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
         /*
          * Offset in the file, in bits
          */
-        packetIndex.setDataOffsetBits(bitBuffer.position());
+        packetIndex.setDataOffsetBits(pos);
 
         /*
          * Update the counting packet offset
@@ -312,18 +267,9 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
                 + ((packetIndex.getPacketSizeBits() + 7) / 8);
     }
 
-    @NonNull
-    ByteBuffer getByteBufferAt(long position, long size) throws CTFReaderException, IOException {
-        MappedByteBuffer map = fFileChannel.map(MapMode.READ_ONLY, position, size);
-        if (map == null) {
-            throw new CTFReaderException("Failed to allocate mapped byte buffer"); //$NON-NLS-1$
-        }
-        return map;
-    }
-
-    @NonNull
-    private ByteBuffer createPacketBitBuffer(long fileSizeBytes,
+    private long readPacketHeader(long fileSizeBytes,
             long packetOffsetBytes, StreamInputPacketIndexEntry packetIndex) throws CTFReaderException {
+        long position = -1;
         /*
          * Initial size, it should map at least the packet header + context
          * size.
@@ -342,11 +288,38 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
         /*
          * Map the packet.
          */
-        try {
-            return getByteBufferAt(packetOffsetBytes, mapSize);
+        try (FileChannel fc = FileChannel.open(fFile.toPath(), StandardOpenOption.READ)) {
+            MappedByteBuffer map = fc.map(MapMode.READ_ONLY, packetOffsetBytes, mapSize);
+            if (map == null) {
+                throw new CTFReaderException("Failed to allocate mapped byte buffer"); //$NON-NLS-1$
+            }
+            /*
+             * create a packet bit buffer to read the packet header
+             */
+            BitBuffer bitBuffer = new BitBuffer(map);
+            bitBuffer.setByteOrder(getStream().getTrace().getByteOrder());
+            /*
+             * Read the trace packet header if it exists.
+             */
+            if (fTracePacketHeaderDecl != null) {
+                parseTracePacketHeader(fTracePacketHeaderDecl, bitBuffer);
+            }
+
+            /*
+             * Read the stream packet context if it exists.
+             */
+            if (fStreamPacketContextDecl != null) {
+                parsePacketContext(fileSizeBytes, fStreamPacketContextDecl,
+                        bitBuffer, packetIndex);
+            } else {
+                setPacketContextNull(fileSizeBytes, packetIndex);
+            }
+
+            position = bitBuffer.position();
         } catch (IOException e) {
             throw new CTFReaderException(e);
         }
+        return position;
     }
 
     private void parseTracePacketHeader(StructDeclaration tracePacketHeaderDecl,
@@ -393,6 +366,16 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
         }
     }
 
+    /**
+     * Gets the wrapped file
+     *
+     * @return the file
+     */
+    @NonNull
+    File getFile() {
+        return fFile;
+    }
+
     private static void setPacketContextNull(long fileSizeBytes,
             StreamInputPacketIndexEntry packetIndex) {
         /*
@@ -485,7 +468,7 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
     public int hashCode() {
         final int prime = 31;
         int result = 1;
-        result = (prime * result) + ((fFile == null) ? 0 : fFile.hashCode());
+        result = (prime * result) + fFile.hashCode();
         return result;
     }
 
@@ -501,11 +484,7 @@ public class CTFStreamInput implements IDefinitionScope, AutoCloseable {
             return false;
         }
         CTFStreamInput other = (CTFStreamInput) obj;
-        if (fFile == null) {
-            if (other.fFile != null) {
-                return false;
-            }
-        } else if (!fFile.equals(other.fFile)) {
+        if (!fFile.equals(other.fFile)) {
             return false;
         }
         return true;
index 3004025ffd58aafe66cef429a4e5545da521626f..9509a9929d374594239daf48efd4fac1371e5dfd 100644 (file)
@@ -13,6 +13,8 @@ package org.eclipse.linuxtools.ctf.core.trace;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.MappedByteBuffer;
+import java.nio.channels.FileChannel.MapMode;
 
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
@@ -217,6 +219,14 @@ public class CTFStreamInputPacketReader implements IDefinitionScope, AutoCloseab
     // Operations
     // ------------------------------------------------------------------------
 
+    @NonNull
+    private ByteBuffer getByteBufferAt(long position, long size) throws CTFReaderException, IOException {
+        MappedByteBuffer map = fStreamInputReader.getFc().map(MapMode.READ_ONLY, position, size);
+        if (map == null) {
+            throw new CTFReaderException("Failed to allocate mapped byte buffer"); //$NON-NLS-1$
+        }
+        return map;
+    }
     /**
      * Changes the current packet to the given one.
      *
@@ -235,7 +245,7 @@ public class CTFStreamInputPacketReader implements IDefinitionScope, AutoCloseab
              */
             ByteBuffer bb = null;
             try {
-                bb = fStreamInputReader.getStreamInput().getByteBufferAt(
+                bb = getByteBufferAt(
                         fCurrentPacket.getOffsetBytes(),
                         (fCurrentPacket.getPacketSizeBits() + 7) / 8);
             } catch (IOException e) {
index ebbe1d78038c5bb9090db75c8c25cd87ae220cba..c7965b398893f8ed05e9ddae1ef588af4b9440dc 100644 (file)
 
 package org.eclipse.linuxtools.ctf.core.trace;
 
+import java.io.File;
+import java.io.IOException;
 import java.nio.ByteOrder;
+import java.nio.channels.FileChannel;
+import java.nio.file.StandardOpenOption;
 
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.linuxtools.ctf.core.event.EventDefinition;
 import org.eclipse.linuxtools.ctf.core.event.IEventDeclaration;
 import org.eclipse.linuxtools.ctf.core.event.types.StructDeclaration;
+import org.eclipse.linuxtools.internal.ctf.core.Activator;
 import org.eclipse.linuxtools.internal.ctf.core.trace.StreamInputPacketIndexEntry;
 
 import com.google.common.collect.ImmutableList;
@@ -38,8 +43,12 @@ public class CTFStreamInputReader implements AutoCloseable {
     /**
      * The StreamInput we are reading.
      */
+    private final @NonNull File fFile;
+
     private final @NonNull CTFStreamInput fStreamInput;
 
+    private final FileChannel fFileChannel;
+
     /**
      * The packet reader used to read packets from this trace file.
      */
@@ -68,20 +77,25 @@ public class CTFStreamInputReader implements AutoCloseable {
     // ------------------------------------------------------------------------
     // Constructors
     // ------------------------------------------------------------------------
-
     /**
      * Constructs a StreamInputReader that reads a StreamInput.
      *
      * @param streamInput
      *            The StreamInput to read.
      * @throws CTFReaderException
-     *             if an error occurs
+     *             If the file cannot be opened
      */
     public CTFStreamInputReader(CTFStreamInput streamInput) throws CTFReaderException {
         if (streamInput == null) {
-            throw new IllegalArgumentException("streamInput cannot be null"); //$NON-NLS-1$
+            throw new IllegalArgumentException("stream cannot be null"); //$NON-NLS-1$
         }
         fStreamInput = streamInput;
+        fFile = fStreamInput.getFile();
+        try {
+            fFileChannel = FileChannel.open(fFile.toPath(), StandardOpenOption.READ);
+        } catch (IOException e) {
+            throw new CTFReaderException(e);
+        }
         fPacketReader = new CTFStreamInputPacketReader(this);
         /*
          * Get the iterator on the packet index.
@@ -94,10 +108,15 @@ public class CTFStreamInputReader implements AutoCloseable {
     }
 
     /**
-     * Dispose the StreamInputReader
+     * Dispose the StreamInputReader, closes the file channel and its packet
+     * reader
+     *
+     * @throws IOException
+     *             If an I/O error occurs
      */
     @Override
-    public void close() {
+    public void close() throws IOException {
+        fFileChannel.close();
         fPacketReader.close();
     }
 
@@ -296,6 +315,7 @@ public class CTFStreamInputReader implements AutoCloseable {
                 goToNextPacket();
             } catch (CTFReaderException e) {
                 // do nothing here
+                Activator.log(e.getMessage());
             }
         }
         if (fPacketReader.getCurrentPacket() == null) {
@@ -422,6 +442,15 @@ public class CTFStreamInputReader implements AutoCloseable {
         return fStreamInput.getIndex().getEntries().get(getPacketIndex());
     }
 
+    /**
+     * Get the file channel wrapped by this reader
+     *
+     * @return the file channel
+     */
+    FileChannel getFc() {
+        return fFileChannel;
+    }
+
     /**
      * @return the packetReader
      */
@@ -435,7 +464,7 @@ public class CTFStreamInputReader implements AutoCloseable {
         int result = 1;
         result = (prime * result) + fId;
         result = (prime * result)
-                + fStreamInput.hashCode();
+                + fFile.hashCode();
         return result;
     }
 
@@ -454,7 +483,7 @@ public class CTFStreamInputReader implements AutoCloseable {
         if (fId != other.fId) {
             return false;
         }
-        return fStreamInput.equals(other.fStreamInput);
+        return fFile.equals(other.fFile);
     }
 
     @Override
@@ -462,4 +491,5 @@ public class CTFStreamInputReader implements AutoCloseable {
         // this helps debugging
         return fId + ' ' + fCurrentEvent.toString();
     }
+
 }
index c21fd709e8dfbe791f4adbff074b983b41444d0c..0c2ee5a5cbddf2c4b368bb62e4526834fb9fd198 100644 (file)
@@ -270,7 +270,6 @@ public class CTFTrace implements IDefinitionScope, AutoCloseable {
         return getStream(streamId).getEventDeclaration((int) id);
     }
 
-
     /**
      * Get an event by it's ID
      *
@@ -477,7 +476,6 @@ public class CTFTrace implements IDefinitionScope, AutoCloseable {
         return (fPath != null) ? fPath.getPath() : ""; //$NON-NLS-1$
     }
 
-
     // ------------------------------------------------------------------------
     // Operations
     // ------------------------------------------------------------------------
@@ -522,7 +520,7 @@ public class CTFTrace implements IDefinitionScope, AutoCloseable {
                 FileChannel fc = fis.getChannel()) {
             /* Map one memory page of 4 kiB */
             byteBuffer = fc.map(MapMode.READ_ONLY, 0, (int) Math.min(fc.size(), 4096L));
-            if( byteBuffer == null){
+            if (byteBuffer == null) {
                 throw new IllegalStateException("Failed to allocate memory"); //$NON-NLS-1$
             }
         } catch (IOException e) {
@@ -758,6 +756,7 @@ public class CTFTrace implements IDefinitionScope, AutoCloseable {
 
     /**
      * Gets the current first packet start time
+     *
      * @return the current start time
      * @since 3.0
      */
@@ -773,6 +772,7 @@ public class CTFTrace implements IDefinitionScope, AutoCloseable {
 
     /**
      * Gets the current last packet end time
+     *
      * @return the current end time
      * @since 3.0
      */
@@ -956,15 +956,21 @@ public class CTFTrace implements IDefinitionScope, AutoCloseable {
      *             The file must exist
      * @since 3.0
      */
+    // TODO: remove suppress warning
+    @SuppressWarnings("resource")
     public void addStream(long id, File streamFile) throws CTFReaderException {
         CTFStream stream = null;
+        final File file = streamFile;
+        if (file == null) {
+            throw new CTFReaderException("cannot create a stream with no file"); //$NON-NLS-1$
+        }
         if (fStreams.containsKey(id)) {
             stream = fStreams.get(id);
         } else {
             stream = new CTFStream(this);
             fStreams.put(id, stream);
         }
-        stream.addInput(new CTFStreamInput(stream, streamFile));
+        stream.addInput(new CTFStreamInput(stream, file));
     }
 }
 
index aa30abcccf6eb5ffd0525373062035e8ede25f35..81d6f12a370ec0aa8149f44fb9561829973acf8f 100644 (file)
@@ -13,6 +13,7 @@
 
 package org.eclipse.linuxtools.ctf.core.trace;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -134,7 +135,11 @@ public class CTFTraceReader implements AutoCloseable {
     public void close() {
         for (CTFStreamInputReader reader : fStreamInputReaders) {
             if (reader != null) {
-                reader.close();
+                try {
+                    reader.close();
+                } catch (IOException e) {
+                    Activator.logError(e.getMessage(), e);
+                }
             }
         }
         fStreamInputReaders.clear();
This page took 0.03657 seconds and 5 git commands to generate.