Skip to content

Commit

Permalink
Merge pull request #7313 from GlobalDataverseCommunityConsortium/IQSS…
Browse files Browse the repository at this point in the history 
…/7309-S3_resource_leaks

Iqss/7309 s3 resource leaks
  • Loading branch information
@kcondon
kcondon authored Oct 8, 2020
2 parents 62da27f + ec791bd commit 6af5099
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 53 deletions.
22 changes: 15 additions & 7 deletions src/main/java/edu/harvard/iq/dataverse/dataaccess/DataConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
import java.io.IOException;

import java.util.logging.Logger;

import org.apache.commons.io.IOUtils;

import java.util.List;
import java.util.Map;
import java.util.HashMap;
Expand Down Expand Up @@ -171,7 +174,9 @@ public static File downloadFromStorageIO(StorageIO<DataFile> storageIO) {
} else {
try {
storageIO.open();
return downloadFromByteChannel(storageIO.getReadChannel(), storageIO.getSize());
try (ReadableByteChannel tabFileChannel = storageIO.getReadChannel()) {
return downloadFromByteChannel(tabFileChannel, storageIO.getSize());
}
} catch (IOException ex) {
logger.warning("caught IOException trying to store tabular file " + storageIO.getDataFile().getStorageIdentifier() + " as a temp file.");
}
Expand All @@ -184,12 +189,13 @@ private static File downloadFromByteChannel(ReadableByteChannel tabFileChannel,
logger.fine("opening datafFileIO for the source tabular file...");

File tabFile = File.createTempFile("tempTabFile", ".tmp");
FileChannel tempFileChannel = new FileOutputStream(tabFile).getChannel();
tempFileChannel.transferFrom(tabFileChannel, 0, size);
return tabFile;
try (FileChannel tempFileChannel = new FileOutputStream(tabFile).getChannel();) {
tempFileChannel.transferFrom(tabFileChannel, 0, size);
return tabFile;
}
} catch (IOException ioex) {
logger.warning("caught IOException trying to store tabular file as a temp file.");
}
}
return null;
}

Expand Down Expand Up @@ -237,8 +243,10 @@ private static File runFormatConversion (DataFile file, File tabFile, String for
try {
StorageIO<DataFile> storageIO = file.getStorageIO();
long size = storageIO.getAuxObjectSize("orig");
File origFile = downloadFromByteChannel((ReadableByteChannel) storageIO.openAuxChannel("orig"), size);
resultInfo = dfs.directConvert(origFile, origFormat);
try (ReadableByteChannel origChannel = (ReadableByteChannel) storageIO.openAuxChannel("orig")) {
File origFile = downloadFromByteChannel(origChannel, size);
resultInfo = dfs.directConvert(origFile, origFormat);
}
} catch (IOException ex) {
ex.printStackTrace();
return null;
Expand Down
16 changes: 10 additions & 6 deletions src/main/java/edu/harvard/iq/dataverse/dataaccess/ImageThumbConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ private static boolean generatePDFThumbnail(StorageIO<DataFile> storageIO, int s
return false;
} finally {
IOUtils.closeQuietly(tempFileChannel);
IOUtils.closeQuietly(pdfFileChannel);
}
sourcePdfFile = tempFile;
}
Expand Down Expand Up @@ -272,7 +273,9 @@ private static boolean generateImageThumbnail(StorageIO<DataFile> storageIO, int

try {
storageIO.open();
return generateImageThumbnailFromInputStream(storageIO, size, storageIO.getInputStream());
try(InputStream inputStream = storageIO.getInputStream()) {
return generateImageThumbnailFromInputStream(storageIO, size, inputStream);
}
} catch (IOException ioex) {
logger.warning("caught IOException trying to open an input stream for " + storageIO.getDataFile().getStorageIdentifier() + ioex);
return false;
Expand Down Expand Up @@ -312,16 +315,18 @@ private static boolean generateWorldMapThumbnail(StorageIO<DataFile> storageIO,
worldMapImageInputStream.close();
return false;
}
return generateImageThumbnailFromInputStream(storageIO, size, worldMapImageInputStream);
} catch (FileNotFoundException fnfe) {
logger.fine("No .img file for this worldmap file yet; giving up. Original Error: " + fnfe);
return false;

} catch (IOException ioex) {
logger.warning("caught IOException trying to open an input stream for worldmap .img file (" + storageIO.getDataFile().getStorageIdentifier() + "). Original Error: " + ioex);
return false;
} finally {
IOUtils.closeQuietly(worldMapImageInputStream);
}

return generateImageThumbnailFromInputStream(storageIO, size, worldMapImageInputStream);

}

/*
Expand Down Expand Up @@ -750,15 +755,14 @@ private static void rescaleImage(BufferedImage fullSizeImage, int width, int hei
g2.drawImage(thumbImage, 0, 0, null);
g2.dispose();

try {
ImageOutputStream ios = ImageIO.createImageOutputStream(outputStream);
try (ImageOutputStream ios = ImageIO.createImageOutputStream(outputStream);) {

writer.setOutput(ios);

// finally, save thumbnail image:
writer.write(lowRes);
writer.dispose();

ios.close();
thumbImage.flush();
//fullSizeImage.flush();
lowRes.flush();
Expand Down
20 changes: 11 additions & 9 deletions src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,18 +578,20 @@ public void saveInputStreamAsAux(InputStream inputStream, String auxItemTag) thr

//Helper method for supporting saving streams with unknown length to S3
//We save those streams to a file and then upload the file
private File createTempFile(Path path, InputStream inputStream) throws IOException {
private File createTempFile(Path path, InputStream inputStream) throws IOException {

File targetFile = new File(path.toUri()); //File needs a name
OutputStream outStream = new FileOutputStream(targetFile);
File targetFile = new File(path.toUri()); // File needs a name
try (OutputStream outStream = new FileOutputStream(targetFile);) {

byte[] buffer = new byte[8 * 1024];
int bytesRead;
while ((bytesRead = inputStream.read(buffer)) != -1) {
outStream.write(buffer, 0, bytesRead);
byte[] buffer = new byte[8 * 1024];
int bytesRead;
while ((bytesRead = inputStream.read(buffer)) != -1) {
outStream.write(buffer, 0, bytesRead);
}

} finally {
IOUtils.closeQuietly(inputStream);
}
IOUtils.closeQuietly(inputStream);
IOUtils.closeQuietly(outStream);
return targetFile;
}

Expand Down
9 changes: 6 additions & 3 deletions src/main/java/edu/harvard/iq/dataverse/dataaccess/StoredOriginalFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import java.nio.channels.Channels;
import java.nio.channels.ReadableByteChannel;
import java.util.logging.Logger;

import org.apache.tika.io.IOUtils;
/**
*
* @author Leonid Andreev
Expand Down Expand Up @@ -56,17 +58,18 @@ public static StorageIO<DataFile> retreive(StorageIO<DataFile> storageIO) {

long storedOriginalSize;
InputStreamIO inputStreamIO;

Channel storedOriginalChannel = null;
try {
storageIO.open();
Channel storedOriginalChannel = storageIO.openAuxChannel(SAVED_ORIGINAL_FILENAME_EXTENSION);
storedOriginalChannel = storageIO.openAuxChannel(SAVED_ORIGINAL_FILENAME_EXTENSION);
storedOriginalSize = dataFile.getDataTable().getOriginalFileSize() != null ?
dataFile.getDataTable().getOriginalFileSize() :
storageIO.getAuxObjectSize(SAVED_ORIGINAL_FILENAME_EXTENSION);
inputStreamIO = new InputStreamIO(Channels.newInputStream((ReadableByteChannel) storedOriginalChannel), storedOriginalSize);
logger.fine("Opened stored original file as Aux "+SAVED_ORIGINAL_FILENAME_EXTENSION);
} catch (IOException ioEx) {
// The original file not saved, or could not be opened.
IOUtils.closeQuietly(storedOriginalChannel);
// The original file not saved, or could not be opened.
logger.fine("Failed to open stored original file as Aux "+SAVED_ORIGINAL_FILENAME_EXTENSION+"!");
return null;
}
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/dataset/DatasetUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public static Dataset persistDatasetLogoToStorageAndCreateThumbnails(Dataset dat
try {
tmpFile = FileUtil.inputStreamToFile(inputStream);
} catch (IOException ex) {
logger.severe(ex.getMessage());
logger.severe(ex.getMessage());
}

StorageIO<Dataset> dataAccess = null;
Expand All @@ -298,11 +298,13 @@ public static Dataset persistDatasetLogoToStorageAndCreateThumbnails(Dataset dat
try {
fullSizeImage = ImageIO.read(tmpFile);
} catch (IOException ex) {
IOUtils.closeQuietly(inputStream);
logger.severe(ex.getMessage());
return null;
}
if (fullSizeImage == null) {
logger.fine("fullSizeImage was null!");
IOUtils.closeQuietly(inputStream);
return null;
}
int width = fullSizeImage.getWidth();
Expand All @@ -311,13 +313,15 @@ public static Dataset persistDatasetLogoToStorageAndCreateThumbnails(Dataset dat
try {
src = new FileInputStream(tmpFile).getChannel();
} catch (FileNotFoundException ex) {
IOUtils.closeQuietly(inputStream);
logger.severe(ex.getMessage());
return null;
}
FileChannel dest = null;
try {
dest = new FileOutputStream(tmpFile).getChannel();
} catch (FileNotFoundException ex) {
IOUtils.closeQuietly(inputStream);
logger.severe(ex.getMessage());
return null;
}
Expand All @@ -329,10 +333,13 @@ public static Dataset persistDatasetLogoToStorageAndCreateThumbnails(Dataset dat
}
File tmpFileForResize = null;
try {
//The stream was used around line 274 above, so this creates an empty file (OK since all it is used for is getting a path, but not reusing it here would make it easier to close it above.)
tmpFileForResize = FileUtil.inputStreamToFile(inputStream);
} catch (IOException ex) {
logger.severe(ex.getMessage());
return null;
} finally {
IOUtils.closeQuietly(inputStream);
}
// We'll try to pre-generate the rescaled versions in both the
// DEFAULT_DATASET_LOGO (currently 140) and DEFAULT_CARDIMAGE_SIZE (48)
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RedetectFileTypeCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ public DataFile execute(CommandContext ctxt) throws CommandException {
} else {
// Need to create a temporary local file:

ReadableByteChannel targetFileChannel = (ReadableByteChannel) storageIO.getReadChannel();
tempFile = File.createTempFile("tempFileTypeCheck", ".tmp");
FileChannel tempFileChannel = new FileOutputStream(tempFile).getChannel();
tempFileChannel.transferFrom(targetFileChannel, 0, storageIO.getSize());

try (ReadableByteChannel targetFileChannel = (ReadableByteChannel) storageIO.getReadChannel();
FileChannel tempFileChannel = new FileOutputStream(tempFile).getChannel();) {
tempFileChannel.transferFrom(targetFileChannel, 0, storageIO.getSize());
}
localFile = tempFile;
}

Expand Down
36 changes: 22 additions & 14 deletions src/main/java/edu/harvard/iq/dataverse/ingest/IngestServiceBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
import edu.harvard.iq.dataverse.ingest.tabulardata.impl.plugins.por.PORFileReader;
import edu.harvard.iq.dataverse.ingest.tabulardata.impl.plugins.por.PORFileReaderSpi;
import edu.harvard.iq.dataverse.util.*;

import org.apache.commons.io.IOUtils;
//import edu.harvard.iq.dvn.unf.*;
import org.dataverse.unf.*;
import java.io.BufferedInputStream;
Expand Down Expand Up @@ -823,12 +825,13 @@ public boolean ingestAsTabular(Long datafile_id) {
localFile = storageIO.getFileSystemPath().toFile();
inputStream = new BufferedInputStream(storageIO.getInputStream());
} else {
ReadableByteChannel dataFileChannel = storageIO.getReadChannel();

localFile = File.createTempFile("tempIngestSourceFile", ".tmp");
FileChannel tempIngestSourceChannel = new FileOutputStream(localFile).getChannel();
try (ReadableByteChannel dataFileChannel = storageIO.getReadChannel();
FileChannel tempIngestSourceChannel = new FileOutputStream(localFile).getChannel();) {

tempIngestSourceChannel.transferFrom(dataFileChannel, 0, storageIO.getSize());
tempIngestSourceChannel.transferFrom(dataFileChannel, 0, storageIO.getSize());
}
inputStream = new BufferedInputStream(new FileInputStream(localFile));
logger.fine("Saved "+storageIO.getSize()+" bytes in a local temp file.");
}
Expand Down Expand Up @@ -895,6 +898,8 @@ public boolean ingestAsTabular(Long datafile_id) {
logger.warning("Ingest failure (Exception " + unknownEx.getClass() + "): "+unknownEx.getMessage()+".");
return false;

} finally {
IOUtils.closeQuietly(inputStream);
}

String originalContentType = dataFile.getContentType();
Expand Down Expand Up @@ -1056,12 +1061,11 @@ private BufferedInputStream openFile(DataFile dataFile) throws IOException {
if (storageIO.isLocalFile()) {
inputStream = new BufferedInputStream(storageIO.getInputStream());
} else {
ReadableByteChannel dataFileChannel = storageIO.getReadChannel();
File tempFile = File.createTempFile("tempIngestSourceFile", ".tmp");
FileChannel tempIngestSourceChannel = new FileOutputStream(tempFile).getChannel();

tempIngestSourceChannel.transferFrom(dataFileChannel, 0, storageIO.getSize());

File tempFile = File.createTempFile("tempIngestSourceFile", ".tmp");
try (ReadableByteChannel dataFileChannel = storageIO.getReadChannel();
FileChannel tempIngestSourceChannel = new FileOutputStream(tempFile).getChannel();) {
tempIngestSourceChannel.transferFrom(dataFileChannel, 0, storageIO.getSize());
}
inputStream = new BufferedInputStream(new FileInputStream(tempFile));
logger.fine("Saved "+storageIO.getSize()+" bytes in a local temp file.");
}
Expand Down Expand Up @@ -1801,10 +1805,14 @@ private void fixMissingOriginalType(long fileId) {
if (savedOriginalFile == null) {
tempFileRequired = true;

ReadableByteChannel savedOriginalChannel = (ReadableByteChannel) storageIO.openAuxChannel(FileUtil.SAVED_ORIGINAL_FILENAME_EXTENSION);
savedOriginalFile = File.createTempFile("tempSavedOriginal", ".tmp");
FileChannel tempSavedOriginalChannel = new FileOutputStream(savedOriginalFile).getChannel();
tempSavedOriginalChannel.transferFrom(savedOriginalChannel, 0, storageIO.getAuxObjectSize(FileUtil.SAVED_ORIGINAL_FILENAME_EXTENSION));
savedOriginalFile = File.createTempFile("tempSavedOriginal", ".tmp");
try (ReadableByteChannel savedOriginalChannel = (ReadableByteChannel) storageIO
.openAuxChannel(FileUtil.SAVED_ORIGINAL_FILENAME_EXTENSION);
FileChannel tempSavedOriginalChannel = new FileOutputStream(savedOriginalFile)
.getChannel();) {
tempSavedOriginalChannel.transferFrom(savedOriginalChannel, 0,
storageIO.getAuxObjectSize(FileUtil.SAVED_ORIGINAL_FILENAME_EXTENSION));
}

}
} catch (Exception ex) {
Expand Down
19 changes: 10 additions & 9 deletions ...n/java/edu/harvard/iq/dataverse/ingest/tabulardata/impl/plugins/dta/DTAFileReaderSpi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public boolean canDecodeInput(Object source) throws IOException {

@Override
public boolean canDecodeInput(BufferedInputStream stream) throws IOException {
//who closes this stream?
if (stream ==null){
throw new IllegalArgumentException("stream == null!");
}
Expand Down Expand Up @@ -185,21 +186,21 @@ public boolean canDecodeInput(File file) throws IOException {
throw new IIOException("cannot read the input file");
}

byte[] hdr4 = new byte[4];
// set-up a FileChannel instance for a given file object
FileChannel srcChannel = new FileInputStream(file).getChannel();

// create a read-only MappedByteBuffer
MappedByteBuffer buff = srcChannel.map(FileChannel.MapMode.READ_ONLY, 0, DTA_HEADER_SIZE);
try (FileChannel srcChannel = new FileInputStream(file).getChannel();) {

//printHexDump(buff, "hex dump of the byte-buffer");
// create a read-only MappedByteBuffer
MappedByteBuffer buff = srcChannel.map(FileChannel.MapMode.READ_ONLY, 0, DTA_HEADER_SIZE);

buff.rewind();
// printHexDump(buff, "hex dump of the byte-buffer");

dbgLog.fine("applying the dta test\n");
buff.rewind();

byte[] hdr4 = new byte[4];
buff.get(hdr4, 0, 4);
dbgLog.fine("applying the dta test\n");

buff.get(hdr4, 0, 4);
}
dbgLog.fine("hex dump: 1st 4bytes =>" +
new String(Hex.encodeHex(hdr4)) + "<-");

Expand Down

0 comments on commit 6af5099

Please sign in to comment.