Skip to content

Commit

Permalink
Robustness fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukehutch
lukehutch committed Nov 30, 2019
1 parent e4ae3bd commit 56c2e32
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 35 deletions.
63 changes: 41 additions & 22 deletions src/main/java/nonapi/io/github/classgraph/fastzipfilereader/MappedByteBufferResources.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,32 @@ public MappedByteBufferResources(final InputStream inputStream, final String tem

} else {
// bytesRead == 0 => ran out of buffer space, spill over to disk
final File tempFile = nestedJarHandler.makeTempFile(tempFileBaseName, /* onlyUseLeafname = */ true);
mappedFileIsTempFile = true;
if (log != null) {
log.log("Could not fit downloaded URL into max RAM buffer size of " + MAX_JAR_RAM_SIZE
+ " bytes, downloading to temporary file: " + tempFileBaseName + " -> " + tempFile);
+ " bytes, downloading to temporary file: " + tempFileBaseName + " -> " + this.mappedFile);
}
Files.write(tempFile.toPath(), buf, StandardOpenOption.WRITE);
try (OutputStream os = new BufferedOutputStream(new FileOutputStream(tempFile, /* append = */ true))) {
try {
this.mappedFile = nestedJarHandler.makeTempFile(tempFileBaseName, /* onlyUseLeafname = */ true);
} catch (final IOException e) {
if (log != null) {
log.log("Could not create temporary file: " + e);
}
throw e;
}
this.mappedFileIsTempFile = true;

// Write the full buffer to the temporary file
Files.write(this.mappedFile.toPath(), buf, StandardOpenOption.WRITE);

// Copy the rest of the InputStream to the end of the temporary file
try (OutputStream os = new BufferedOutputStream(
new FileOutputStream(this.mappedFile, /* append = */ true))) {
for (int bytesReadCtd; (bytesReadCtd = inputStream.read(buf, 0, buf.length)) > 0;) {
os.write(buf, 0, bytesReadCtd);
}
}
// Map the file to a MappedByteBuffer
mapFile(tempFile);
mapFile();
}
}

Expand Down Expand Up @@ -164,7 +176,9 @@ public MappedByteBufferResources(final ByteBuffer byteBuffer, final NestedJarHan
*/
public MappedByteBufferResources(final File file, final NestedJarHandler nestedJarHandler) throws IOException {
this.nestedJarHandler = nestedJarHandler;
mapFile(file);
this.mappedFile = file;
// Map the file to a MappedByteBuffer
mapFile();
}

/**
Expand All @@ -180,32 +194,36 @@ private void wrapByteBuffer(final ByteBuffer byteBuffer) {
byteBufferChunksCached = new AtomicReferenceArray<ByteBuffer>(1);
byteBufferChunksCached.set(0, byteBuffer);

// Don't set file, fileChannel or raf, they are unneeded.
// byteBufferIsMapped will remain false.
// Don't set mappedFile, fileChannel or raf, they are unneeded.
}

/**
* Map a {@link File} to a {@link MappedByteBuffer}.
*
* @param file
* the file
* @throws IOException
* Signals that an I/O exception has occurred.
*/
private void mapFile(final File file) throws IOException {
this.mappedFile = file;
private void mapFile() throws IOException {
try {
raf = new RandomAccessFile(file, "r");
raf = new RandomAccessFile(mappedFile, "r");
length = raf.length();
fileChannel = raf.getChannel();

} catch (final IOException | SecurityException e) {
if (fileChannel != null) {
fileChannel.close();
try {
fileChannel.close();
} catch (final IOException e2) {
// Ignore
}
fileChannel = null;
}
if (raf != null) {
raf.close();
try {
raf.close();
} catch (final IOException e2) {
// Ignore
}
raf = null;
}
throw e;
Expand Down Expand Up @@ -249,7 +267,6 @@ public ByteBuffer newInstance(final Integer chunkIdxI, final LogNode log) throws
return byteBuffer;
}
};

}

/**
Expand Down Expand Up @@ -327,6 +344,7 @@ public void close(final LogNode log) {
chunkIdxToByteBufferSingletonMap = null;
}
if (byteBufferChunksCached != null) {
// Only unmap bytebuffers if they came from a mapped file
if (mappedFile != null) {
for (int i = 0; i < byteBufferChunksCached.length(); i++) {
final ByteBuffer mappedByteBuffer = byteBufferChunksCached.get(i);
Expand All @@ -341,26 +359,27 @@ public void close(final LogNode log) {
if (fileChannel != null) {
try {
fileChannel.close();
fileChannel = null;
} catch (final IOException e) {
// Ignore
}
fileChannel = null;
}
if (raf != null) {
try {
raf.close();
raf = null;
} catch (final IOException e) {
// Ignore
}
raf = null;
}
if (mappedFile != null) {
// If mapped file was a temp file, remove it
if (mappedFileIsTempFile) {
try {
Files.delete(mappedFile.toPath());
} catch (final IOException e) {
nestedJarHandler.removeTempFile(mappedFile);
} catch (IOException | SecurityException e) {
if (log != null) {
log.log("Could not delete temporary file " + mappedFile + " : " + e);
log.log("Removing temporary file failed: " + mappedFile);
}
}
}
Expand Down
47 changes: 34 additions & 13 deletions src/main/java/nonapi/io/github/classgraph/fastzipfilereader/NestedJarHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.zip.Inflater;
Expand Down Expand Up @@ -384,7 +383,7 @@ public RecyclableInflater newInstance() throws RuntimeException {
.newSetFromMap(new ConcurrentHashMap<MappedByteBufferResources, Boolean>());

/** Any temporary files created while scanning. */
private ConcurrentLinkedDeque<File> tempFiles = new ConcurrentLinkedDeque<>();
private Set<File> tempFiles = Collections.newSetFromMap(new ConcurrentHashMap<File, Boolean>());

/** The separator between random temp filename part and leafname. */
public static final String TEMP_FILENAME_LEAF_SEPARATOR = "---";
Expand Down Expand Up @@ -464,22 +463,44 @@ private String sanitizeFilename(final String filename) {
/**
* Create a temporary file, and mark it for deletion on exit.
*
* @param filePath
* @param filePathBase
* The path to derive the temporary filename from.
* @param onlyUseLeafname
* If true, only use the leafname of filePath to derive the temporary filename.
* @return The temporary {@link File}.
* @throws IOException
* If the temporary file could not be created.
*/
File makeTempFile(final String filePath, final boolean onlyUseLeafname) throws IOException {
final File tempFile = File.createTempFile("ClassGraph--",
TEMP_FILENAME_LEAF_SEPARATOR + sanitizeFilename(onlyUseLeafname ? leafname(filePath) : filePath));
File makeTempFile(final String filePathBase, final boolean onlyUseLeafname) throws IOException {
final File tempFile = File.createTempFile("ClassGraph--", TEMP_FILENAME_LEAF_SEPARATOR
+ sanitizeFilename(onlyUseLeafname ? leafname(filePathBase) : filePathBase));
tempFile.deleteOnExit();
tempFiles.add(tempFile);
return tempFile;
}

/**
* Attempt to remove a temporary file.
*
* @param tempFile
* the temp file
* @throws IOException
* If the temporary file could not be removed.
* @throws SecurityException
* If the temporary file is inaccessible.
*/
void removeTempFile(final File tempFile) throws IOException, SecurityException {
if (tempFiles.contains(tempFile)) {
try {
Files.delete(tempFile.toPath());
} finally {
tempFiles.remove(tempFile);
}
} else {
throw new IOException("Not a temp file: " + tempFile);
}
}

/**
* Download a jar from a URL to a temporary file, or to a ByteBuffer if the temporary directory is not writeable
* or full. The downloaded jar is returned wrapped in a {@link PhysicalZipFile} instance.
Expand Down Expand Up @@ -610,16 +631,16 @@ public void close(final LogNode log) {
final LogNode rmLog = tempFiles.isEmpty() || log == null ? null
: log.log("Removing temporary files");
while (!tempFiles.isEmpty()) {
final File tempFile = tempFiles.removeLast();
try {
Files.delete(tempFile.toPath());
} catch (final IOException | SecurityException e) {
if (rmLog != null) {
rmLog.log("Removing temporary file failed: " + e);
for (final File tempFile : new ArrayList<>(tempFiles)) {
try {
removeTempFile(tempFile);
} catch (IOException | SecurityException e) {
if (rmLog != null) {
rmLog.log("Removing temporary file failed: " + tempFile);
}
}
}
}
tempFiles.clear();
tempFiles = null;
}
}
Expand Down

0 comments on commit 56c2e32

Please sign in to comment.