Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Iqss/7309 s3 resource leaks #7313

Merged
merged 2 commits into from
Oct 8, 2020
Merged

Iqss/7309 s3 resource leaks #7313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00dc9b9
close any open streams/channels
qqmyers Oct 7, 2020
ec791bd
formatting
qqmyers Oct 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions src/main/java/edu/harvard/iq/dataverse/dataaccess/DataConverter.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW - this private method doesn't get used according to eclipse - do we need it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It really appears an unused leftover method.

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
Open in desktop
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?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guessing this method could/should close it but I didn't see where it was getting called.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not calling these methods at this time. They are part of a builtin infrastructure for automatically matching ingest plugins to specific file types... We are not using it now, but we may go back to it.
It would be the responsibility of the code that calls these methods to close the streams.

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