Repository: commons-compress Updated Branches: refs/heads/master e9f2dadb9 -> 53f6d42f7
COMPRESS-457 improve resource cleanup in close() implementations Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-compress/commit/16e358e2 Tree: http://git-wip-us.apache.org/repos/asf/commons-compress/tree/16e358e2 Diff: http://git-wip-us.apache.org/repos/asf/commons-compress/diff/16e358e2 Branch: refs/heads/master Commit: 16e358e242b41ac350fbd7ad9a93a0cae3e92564 Parents: e9f2dad Author: Stefan Bodewig <bode...@apache.org> Authored: Sun Jul 1 11:49:24 2018 +0200 Committer: Stefan Bodewig <bode...@apache.org> Committed: Sun Jul 1 11:49:24 2018 +0200 ---------------------------------------------------------------------- src/changes/changes.xml | 5 +++++ .../compress/archivers/ar/ArArchiveOutputStream.java | 3 +++ .../archivers/cpio/CpioArchiveOutputStream.java | 4 +++- .../compress/archivers/sevenz/SevenZOutputFile.java | 3 +++ .../archivers/tar/TarArchiveOutputStream.java | 4 +++- .../archivers/zip/ScatterZipOutputStream.java | 3 +++ .../archivers/zip/ZipArchiveOutputStream.java | 6 ++++++ .../bzip2/BZip2CompressorOutputStream.java | 3 +++ .../deflate64/Deflate64CompressorInputStream.java | 3 +++ .../compressors/gzip/GzipCompressorOutputStream.java | 3 +++ .../lz4/BlockLZ4CompressorOutputStream.java | 3 +++ .../lz4/FramedLZ4CompressorInputStream.java | 3 +++ .../lz4/FramedLZ4CompressorOutputStream.java | 3 +++ .../pack200/Pack200CompressorOutputStream.java | 3 +++ .../pack200/TempFileCachingStreamBridge.java | 3 +++ .../snappy/FramedSnappyCompressorInputStream.java | 3 +++ .../snappy/FramedSnappyCompressorOutputStream.java | 3 +++ .../snappy/SnappyCompressorOutputStream.java | 3 +++ .../parallel/FileBasedScatterGatherBackingStore.java | 3 +++ .../compress/utils/FixedLengthBlockOutputStream.java | 3 +++ .../compress/archivers/ArchiveOutputStreamTest.java | 15 +++++++++++++++ 21 files changed, 80 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 4838c77..5a620cb 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -54,6 +54,11 @@ The <action> type attribute can be add,update,fix,remove. Changed the OSGi Import-Package to also optionally import javax.crypto so encrypted archives can be read. </action> + <action issue="COMPRESS-457" type="fix" date="2018-07-01"> + Changed various implementations of the close method to better + ensure all held resources get closed even if exceptions are + thrown during the closing the stream. + </action> </release> <release version="1.17" date="2018-06-03" description="Release 1.17"> http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java index ffca90b..b7988bf 100644 --- a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java @@ -206,11 +206,14 @@ public class ArArchiveOutputStream extends ArchiveOutputStream { */ @Override public void close() throws IOException { + try { if(!finished) { finish(); } + } finally { out.close(); prevEntry = null; + } } @Override http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveOutputStream.java index ed8e2d0..9fe399b 100644 --- a/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveOutputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveOutputStream.java @@ -482,14 +482,16 @@ public class CpioArchiveOutputStream extends ArchiveOutputStream implements */ @Override public void close() throws IOException { + try { if(!finished) { finish(); } - + } finally { if (!this.closed) { out.close(); this.closed = true; } + } } private void pad(final int count) throws IOException{ http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java index 2ed2175..c997067 100644 --- a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZOutputFile.java @@ -130,10 +130,13 @@ public class SevenZOutputFile implements Closeable { */ @Override public void close() throws IOException { + try { if (!finished) { finish(); } + } finally { channel.close(); + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java index 382f06f..fd74d46 100644 --- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java @@ -302,14 +302,16 @@ public class TarArchiveOutputStream extends ArchiveOutputStream { */ @Override public void close() throws IOException { + try { if (!finished) { finish(); } - + } finally { if (!closed) { out.close(); closed = true; } + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java index 7076e2a..b801bf8 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ScatterZipOutputStream.java @@ -124,8 +124,11 @@ public class ScatterZipOutputStream implements Closeable { */ @Override public void close() throws IOException { + try { backingStore.close(); + } finally { streamCompressor.close(); + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java index 6a8cacc..8ca1538 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java @@ -958,10 +958,13 @@ public class ZipArchiveOutputStream extends ArchiveOutputStream { */ @Override public void close() throws IOException { + try { if (!finished) { finish(); } + } finally { destroy(); + } } /** @@ -1597,12 +1600,15 @@ public class ZipArchiveOutputStream extends ArchiveOutputStream { * corrupt archives so they can clean up any temporary files.</p> */ void destroy() throws IOException { + try { if (channel != null) { channel.close(); } + } finally { if (out != null) { out.close(); } + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorOutputStream.java b/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorOutputStream.java index ba2beb1..02fe273 100644 --- a/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorOutputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorOutputStream.java @@ -501,8 +501,11 @@ public class BZip2CompressorOutputStream extends CompressorOutputStream public void close() throws IOException { if (!closed) { final OutputStream outShadow = this.out; + try { finish(); + } finally { outShadow.close(); + } } } http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/deflate64/Deflate64CompressorInputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/deflate64/Deflate64CompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/deflate64/Deflate64CompressorInputStream.java index 883647b..5ed6e58 100644 --- a/src/main/java/org/apache/commons/compress/compressors/deflate64/Deflate64CompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/deflate64/Deflate64CompressorInputStream.java @@ -95,11 +95,14 @@ public class Deflate64CompressorInputStream extends CompressorInputStream implem @Override public void close() throws IOException { + try { closeDecoder(); + } finally { if (originalStream != null) { originalStream.close(); originalStream = null; } + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStream.java b/src/main/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStream.java index d3f4012..711612b 100644 --- a/src/main/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/gzip/GzipCompressorOutputStream.java @@ -204,10 +204,13 @@ public class GzipCompressorOutputStream extends CompressorOutputStream { @Override public void close() throws IOException { if (!closed) { + try { finish(); + } finally { deflater.end(); out.close(); closed = true; + } } } http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorOutputStream.java b/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorOutputStream.java index 2cce3a1..f5b9253 100644 --- a/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorOutputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorOutputStream.java @@ -144,8 +144,11 @@ public class BlockLZ4CompressorOutputStream extends CompressorOutputStream { @Override public void close() throws IOException { + try { finish(); + } finally { os.close(); + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java index c4c4f49..e366815 100644 --- a/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java @@ -126,11 +126,14 @@ public class FramedLZ4CompressorInputStream extends CompressorInputStream /** {@inheritDoc} */ @Override public void close() throws IOException { + try { if (currentBlock != null) { currentBlock.close(); currentBlock = null; } + } finally { in.close(); + } } /** {@inheritDoc} */ http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorOutputStream.java b/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorOutputStream.java index 0b11eff..a2d23e2 100644 --- a/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorOutputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorOutputStream.java @@ -226,8 +226,11 @@ public class FramedLZ4CompressorOutputStream extends CompressorOutputStream { @Override public void close() throws IOException { + try { finish(); + } finally { out.close(); + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/pack200/Pack200CompressorOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/pack200/Pack200CompressorOutputStream.java b/src/main/java/org/apache/commons/compress/compressors/pack200/Pack200CompressorOutputStream.java index ff43a94..6c3bff9 100644 --- a/src/main/java/org/apache/commons/compress/compressors/pack200/Pack200CompressorOutputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/pack200/Pack200CompressorOutputStream.java @@ -114,12 +114,15 @@ public class Pack200CompressorOutputStream extends CompressorOutputStream { @Override public void close() throws IOException { + try { finish(); + } finally { try { streamBridge.stop(); } finally { originalOutput.close(); } + } } public void finish() throws IOException { http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/pack200/TempFileCachingStreamBridge.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/pack200/TempFileCachingStreamBridge.java b/src/main/java/org/apache/commons/compress/compressors/pack200/TempFileCachingStreamBridge.java index dc612aa..c056e5d 100644 --- a/src/main/java/org/apache/commons/compress/compressors/pack200/TempFileCachingStreamBridge.java +++ b/src/main/java/org/apache/commons/compress/compressors/pack200/TempFileCachingStreamBridge.java @@ -45,8 +45,11 @@ class TempFileCachingStreamBridge extends StreamBridge { return new FilterInputStream(Files.newInputStream(f.toPath())) { @Override public void close() throws IOException { + try { super.close(); + } finally { f.delete(); + } } }; } http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java index 12974ab..8949d5e 100644 --- a/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java @@ -144,11 +144,14 @@ public class FramedSnappyCompressorInputStream extends CompressorInputStream /** {@inheritDoc} */ @Override public void close() throws IOException { + try { if (currentCompressedChunk != null) { currentCompressedChunk.close(); currentCompressedChunk = null; } + } finally { in.close(); + } } /** {@inheritDoc} */ http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorOutputStream.java b/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorOutputStream.java index 08cd619..932069b 100644 --- a/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorOutputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorOutputStream.java @@ -102,8 +102,11 @@ public class FramedSnappyCompressorOutputStream extends CompressorOutputStream { @Override public void close() throws IOException { + try { finish(); + } finally { out.close(); + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorOutputStream.java b/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorOutputStream.java index 93a9d80..bfe47ce 100644 --- a/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorOutputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorOutputStream.java @@ -130,8 +130,11 @@ public class SnappyCompressorOutputStream extends CompressorOutputStream { @Override public void close() throws IOException { + try { finish(); + } finally { os.close(); + } } /** http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/parallel/FileBasedScatterGatherBackingStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/parallel/FileBasedScatterGatherBackingStore.java b/src/main/java/org/apache/commons/compress/parallel/FileBasedScatterGatherBackingStore.java index 92b447f..9a9a1db 100644 --- a/src/main/java/org/apache/commons/compress/parallel/FileBasedScatterGatherBackingStore.java +++ b/src/main/java/org/apache/commons/compress/parallel/FileBasedScatterGatherBackingStore.java @@ -67,7 +67,10 @@ public class FileBasedScatterGatherBackingStore implements ScatterGatherBackingS @Override public void close() throws IOException { + try { closeForWriting(); + } finally { target.delete(); + } } } http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java b/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java index d9f2f80..17e8a0c 100644 --- a/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java +++ b/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java @@ -183,8 +183,11 @@ public class FixedLengthBlockOutputStream extends OutputStream implements Writab @Override public void close() throws IOException { if (closed.compareAndSet(false, true)) { + try { flushBlock(); + } finally { out.close(); + } } } http://git-wip-us.apache.org/repos/asf/commons-compress/blob/16e358e2/src/test/java/org/apache/commons/compress/archivers/ArchiveOutputStreamTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/compress/archivers/ArchiveOutputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/ArchiveOutputStreamTest.java index 179569c..0ed878b 100644 --- a/src/test/java/org/apache/commons/compress/archivers/ArchiveOutputStreamTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/ArchiveOutputStreamTest.java @@ -176,12 +176,17 @@ public class ArchiveOutputStreamTest extends AbstractTestCase { fail("Should have raised IOException - close() called before closeArchiveEntry()"); } catch (final IOException expected) { } + + aos1 = createArchiveWithDummyEntry(archiveType, out1, dummy); aos1.closeArchiveEntry(); try { aos1.closeArchiveEntry(); fail("Should have raised IOException - closeArchiveEntry() called with no open entry"); } catch (final IOException expected) { } + + aos1 = createArchiveWithDummyEntry(archiveType, out1, dummy); + aos1.closeArchiveEntry(); aos1.finish(); aos1.close(); try { @@ -190,4 +195,14 @@ public class ArchiveOutputStreamTest extends AbstractTestCase { } catch (final IOException expected) { } } + + private ArchiveOutputStream createArchiveWithDummyEntry(String archiveType, OutputStream out1, File dummy) + throws Exception { + ArchiveOutputStream aos1 = factory.createArchiveOutputStream(archiveType, out1); + aos1.putArchiveEntry(aos1.createArchiveEntry(dummy, "dummy")); + try (InputStream is = new FileInputStream(dummy)) { + IOUtils.copy(is, aos1); + } + return aos1; + } }
