From: Matthew Khouzam Date: Thu, 28 Aug 2014 19:24:30 +0000 (-0400) Subject: ctf: make CtfStreamInput not hold onto any resources X-Git-Url: http://git.efficios.com/?a=commitdiff_plain;h=b3151232bc54b1ab83501d106ae5e394a37c70ce;p=deliverable%2Ftracecompass.git ctf: make CtfStreamInput not hold onto any resources Change-Id: I61cb09c4c325479115084c54bba03d4746cd6610 Reviewed-on: https://git.eclipse.org/r/32553 Tested-by: Hudson CI Reviewed-by: Marc-Andre Laperle --- diff --git a/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamInputTest.java b/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamInputTest.java index 4175b16611..efce3c64cd 100644 --- a/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamInputTest.java +++ b/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamInputTest.java @@ -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; } /** diff --git a/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamTest.java b/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamTest.java index 3be9703cd8..62bcf11b47 100644 --- a/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamTest.java +++ b/org.eclipse.linuxtools.ctf.core.tests/src/org/eclipse/linuxtools/ctf/core/tests/trace/CTFStreamTest.java @@ -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; } /** diff --git a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInput.java b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInput.java index 848ed43358..157b1049c0 100644 --- a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInput.java +++ b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInput.java @@ -13,13 +13,11 @@ 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; diff --git a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputPacketReader.java b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputPacketReader.java index 3004025ffd..9509a9929d 100644 --- a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputPacketReader.java +++ b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputPacketReader.java @@ -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) { diff --git a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputReader.java b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputReader.java index ebbe1d7803..c7965b3988 100644 --- a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputReader.java +++ b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFStreamInputReader.java @@ -12,12 +12,17 @@ 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(); } + } diff --git a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTrace.java b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTrace.java index c21fd709e8..0c2ee5a5cb 100644 --- a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTrace.java +++ b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTrace.java @@ -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)); } } diff --git a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTraceReader.java b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTraceReader.java index aa30abcccf..81d6f12a37 100644 --- a/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTraceReader.java +++ b/org.eclipse.linuxtools.ctf.core/src/org/eclipse/linuxtools/ctf/core/trace/CTFTraceReader.java @@ -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();