This test covers a scenario where we write a smaller amount of data first, then write a + * large amount that crosses the threshold (transitioning from "not at threshold" to "over the + * threshold"). (We then write some more afterward.) This differs from the existing + * testThreshold() which writes exactly enough bytes to fill the buffer, then immediately writes + * more bytes. + * + *
Note: Direct testing of the {@link IOException} scenario during write/flush is challenging + * without mocking. This test verifies that normal operation with threshold crossing still works + * correctly with the fix in place. + */ + public void testThresholdCrossing_resourceManagement() throws Exception { + FileBackedOutputStream out = new FileBackedOutputStream(/* fileThreshold= */ 10); + ByteSource source = out.asByteSource(); + + byte[] chunk1 = newPreFilledByteArray(8); // Below threshold + byte[] chunk2 = newPreFilledByteArray(5); // Crosses threshold + byte[] chunk3 = newPreFilledByteArray(20); // More data to file + + out.write(chunk1); + assertThat(out.getFile()).isNull(); + + out.write(chunk2); + assertThat(out.getFile()).isNotNull(); + assertThat(source.read()).isEqualTo(concat(chunk1, chunk2)); + + out.write(chunk3); + assertThat(source.read()).isEqualTo(concat(chunk1, chunk2, chunk3)); + + out.reset(); + } } diff --git a/android/guava/src/com/google/common/io/FileBackedOutputStream.java b/android/guava/src/com/google/common/io/FileBackedOutputStream.java index ee7cc83c5d1d..520926b0f486 100644 --- a/android/guava/src/com/google/common/io/FileBackedOutputStream.java +++ b/android/guava/src/com/google/common/io/FileBackedOutputStream.java @@ -238,13 +238,21 @@ private void update(int len) throws IOException { // this is insurance. temp.deleteOnExit(); } + FileOutputStream transfer = null; try { - FileOutputStream transfer = new FileOutputStream(temp); + transfer = new FileOutputStream(temp); transfer.write(memory.getBuffer(), 0, memory.getCount()); transfer.flush(); // We've successfully transferred the data; switch to writing to file out = transfer; } catch (IOException e) { + if (transfer != null) { + try { + transfer.close(); + } catch (IOException closeException) { + e.addSuppressed(closeException); + } + } temp.delete(); throw e; } diff --git a/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java b/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java index c16e3fa475f9..86caf6d6582e 100644 --- a/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java +++ b/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java @@ -17,6 +17,7 @@ package com.google.common.io; import static com.google.common.base.StandardSystemProperty.OS_NAME; +import static com.google.common.primitives.Bytes.concat; import static com.google.common.truth.Truth.assertThat; import static java.lang.Math.min; import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; @@ -161,4 +162,39 @@ private static boolean isAndroid() { private static boolean isWindows() { return OS_NAME.value().startsWith("Windows"); } + + /** + * Test that verifies the resource leak fix for Issue #5756. + * + *
This test covers a scenario where we write a smaller amount of data first, then write a + * large amount that crosses the threshold (transitioning from "not at threshold" to "over the + * threshold"). (We then write some more afterward.) This differs from the existing + * testThreshold() which writes exactly enough bytes to fill the buffer, then immediately writes + * more bytes. + * + *
Note: Direct testing of the {@link IOException} scenario during write/flush is challenging + * without mocking. This test verifies that normal operation with threshold crossing still works + * correctly with the fix in place. + */ + public void testThresholdCrossing_resourceManagement() throws Exception { + FileBackedOutputStream out = new FileBackedOutputStream(/* fileThreshold= */ 10); + ByteSource source = out.asByteSource(); + + byte[] chunk1 = newPreFilledByteArray(8); // Below threshold + byte[] chunk2 = newPreFilledByteArray(5); // Crosses threshold + byte[] chunk3 = newPreFilledByteArray(20); // More data to file + + out.write(chunk1); + assertThat(out.getFile()).isNull(); + + out.write(chunk2); + assertThat(out.getFile()).isNotNull(); + assertThat(source.read()).isEqualTo(concat(chunk1, chunk2)); + + out.write(chunk3); + assertThat(source.read()).isEqualTo(concat(chunk1, chunk2, chunk3)); + + out.reset(); + } } diff --git a/guava/src/com/google/common/io/FileBackedOutputStream.java b/guava/src/com/google/common/io/FileBackedOutputStream.java index ee7cc83c5d1d..520926b0f486 100644 --- a/guava/src/com/google/common/io/FileBackedOutputStream.java +++ b/guava/src/com/google/common/io/FileBackedOutputStream.java @@ -238,13 +238,21 @@ private void update(int len) throws IOException { // this is insurance. temp.deleteOnExit(); } + FileOutputStream transfer = null; try { - FileOutputStream transfer = new FileOutputStream(temp); + transfer = new FileOutputStream(temp); transfer.write(memory.getBuffer(), 0, memory.getCount()); transfer.flush(); // We've successfully transferred the data; switch to writing to file out = transfer; } catch (IOException e) { + if (transfer != null) { + try { + transfer.close(); + } catch (IOException closeException) { + e.addSuppressed(closeException); + } + } temp.delete(); throw e; }