S3Object is not closed properly

[musketyr]

Not closing S3Object properly may lead to worker exhaustion in the HTTP client.

Following method is not closing the S3Object properly:

https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/AmazonS3Client.java#L1548-L1571

There might be similar methods which also doesn't close the instances properly in this class and sibling classes (async, encrypted)

[debora-ito]

Hi @musketyr, to release the connection resources you need to call S3Object.close after you processed the input stream returned by AmazonS3Client.getObject.

Check this example:

    S3Object fullObject = null;
    try {
        AmazonS3 s3Client = AmazonS3ClientBuilder.standard()
                .withRegion(clientRegion)
                .withCredentials(new ProfileCredentialsProvider())
                .build();

        // Get an object.
        System.out.println("Downloading an object");
        fullObject = s3Client.getObject(new GetObjectRequest(bucketName, key));

        // Read the text input stream one line at a time and display each line.
        BufferedReader reader = new BufferedReader(new InputStreamReader(fullObject.getObjectContent()));
        String line = null;
        while ((line = reader.readLine()) != null) {
            System.out.println(line);
        }
        System.out.println();
    }
    catch(AmazonServiceException e) {
        e.printStackTrace();
    }
    catch(SdkClientException e) {
        e.printStackTrace();
    }
    finally {
        // To ensure that the network connection doesn't remain open, 
        // close any open input streams.
        if(fullObject != null) {
            fullObject.close();
        }
    }

For a full code example and AmazonS3Client.getObject() usage variations, please check the docs: https://docs.aws.amazon.com/AmazonS3/latest/dev/RetrievingObjectUsingJava.html

Closing this for now, feel free to reopen if you have any other questions.

[musketyr]

@debora-ito I'm sorry but I think there is an misunderstanding here. The code is located inside your library. The S3Object is only available inside the method call so there is no way how can I close the instance from within my code. Or are you suggesting to send you an PR?

[debora-ito]

Yes @musketyr it was my mistake, I didn't realize it was the getObject() method that returns the object metadata. Reopening this and marking as a bug.

[dagnir]

After taking a closer look, this doesn't look like a bug to me. retryableDownloadS3ObjectToFile bubbles down to call to ServiceUtils#downloadToFile which closes the object in a finally block:

aws-sdk-java/aws-java-sdk-s3/src/main/java/com/amazonaws/services/s3/internal/ServiceUtils.java

Lines 283 to 319 in 03eed5e

	public static void downloadToFile(S3Object s3Object,
	final File dstfile, boolean performIntegrityCheck,
	final boolean appendData,
	final long expectedFileLength)
	{
	createParentDirectoryIfNecessary(dstfile);
	if (!FileLocks.lock(dstfile)) {
	throw new FileLockException("Fail to lock " + dstfile
	+ " for appendData=" + appendData);
	}
	OutputStream outputStream = null;
	try {
	final long actualLen = dstfile.length();
	if (appendData && actualLen != expectedFileLength) {
	// Fail fast to prevent data corruption
	throw new IllegalStateException(
	"Expected file length to append is "
	+ expectedFileLength + " but actual length is "
	+ actualLen + " for file " + dstfile);
	}
	outputStream = new BufferedOutputStream(new FileOutputStream(
	dstfile, appendData));
	byte[] buffer = new byte[1024*10];
	int bytesRead;
	while ((bytesRead = s3Object.getObjectContent().read(buffer)) > -1) {
	outputStream.write(buffer, 0, bytesRead);
	}
	} catch (IOException e) {
	s3Object.getObjectContent().abort();
	throw new SdkClientException(
	"Unable to store object contents to disk: " + e.getMessage(), e);
	} finally {
	closeQuietly(outputStream, LOG);
	FileLocks.unlock(dstfile);
	closeQuietly(s3Object.getObjectContent(), LOG);
	}

Going to go ahead and close this. Please feel free to reopen if you have further questions/concerns.