ensureStreamsAreClosed() seems like an over the top name. Why not simply closeAll()? Or close().
Gary On Sun, Nov 18, 2018, 09:02 <bode...@apache.org wrote: > Repository: commons-compress > Updated Branches: > refs/heads/master 979260717 -> f132d6c50 > > > COMPRESS-470 make sure all ScatterZipOutputStreams are closed > > > Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo > Commit: > http://git-wip-us.apache.org/repos/asf/commons-compress/commit/2f75fbb5 > Tree: > http://git-wip-us.apache.org/repos/asf/commons-compress/tree/2f75fbb5 > Diff: > http://git-wip-us.apache.org/repos/asf/commons-compress/diff/2f75fbb5 > > Branch: refs/heads/master > Commit: 2f75fbb5269bd5b2c7b799b7a4f93992fa1f9196 > Parents: 9792607 > Author: Stefan Bodewig <bode...@apache.org> > Authored: Sun Nov 18 16:59:31 2018 +0100 > Committer: Stefan Bodewig <bode...@apache.org> > Committed: Sun Nov 18 16:59:31 2018 +0100 > > ---------------------------------------------------------------------- > src/changes/changes.xml | 4 ++++ > .../archivers/zip/ParallelScatterZipCreator.java | 16 ++++++++++++++++ > .../archivers/zip/ScatterZipOutputStream.java | 5 +++++ > 3 files changed, 25 insertions(+) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/commons-compress/blob/2f75fbb5/src/changes/changes.xml > ---------------------------------------------------------------------- > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > index 5b9619a..e16e763 100644 > --- a/src/changes/changes.xml > +++ b/src/changes/changes.xml > @@ -58,6 +58,10 @@ The <action> type attribute can be > add,update,fix,remove. > TarArchiveInputStream has a new constructor-arg lenient that > can be used to accept certain broken archives. > </action> > + <action issue="COMPRESS-470" type="fix" date="2018-11-18"> > + Fixed another potential resource leak in > + ParallelScatterZipCreator#writeTo. > + </action> > </release> > <release version="1.18" date="2018-08-16" > description="Release 1.18"> > > > http://git-wip-us.apache.org/repos/asf/commons-compress/blob/2f75fbb5/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java > ---------------------------------------------------------------------- > diff --git > a/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java > b/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java > index a381d0a..f2a1679 100644 > --- > a/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java > +++ > b/src/main/java/org/apache/commons/compress/archivers/zip/ParallelScatterZipCreator.java > @@ -239,6 +239,7 @@ public class ParallelScatterZipCreator { > public void writeTo(final ZipArchiveOutputStream targetStream) > throws IOException, InterruptedException, ExecutionException { > > + try { > // Make sure we catch any exceptions from parallel phase > try { > for (final Future<?> future : futures) { > @@ -261,6 +262,9 @@ public class ParallelScatterZipCreator { > } > > scatterDoneAt = System.currentTimeMillis(); > + } finally { > + ensureStreamsAreClosed(); > + } > } > > /** > @@ -271,5 +275,17 @@ public class ParallelScatterZipCreator { > public ScatterStatistics getStatisticsMessage() { > return new ScatterStatistics(compressionDoneAt - startedAt, > scatterDoneAt - compressionDoneAt); > } > + > + private void ensureStreamsAreClosed() { > + synchronized (streams) { > + for (final ScatterZipOutputStream scatterStream : streams) { > + try { > + scatterStream.close(); > + } catch (IOException ex) { > + // no way to properly log this > + } > + } > + } > + } > } > > > > http://git-wip-us.apache.org/repos/asf/commons-compress/blob/2f75fbb5/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 7001c84..006c531 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 > @@ -29,6 +29,7 @@ import java.io.IOException; > import java.io.InputStream; > import java.util.Queue; > import java.util.concurrent.ConcurrentLinkedQueue; > +import java.util.concurrent.atomic.AtomicBoolean; > import java.util.zip.Deflater; > > /** > @@ -49,6 +50,7 @@ public class ScatterZipOutputStream implements Closeable > { > private final Queue<CompressedEntry> items = new > ConcurrentLinkedQueue<>(); > private final ScatterGatherBackingStore backingStore; > private final StreamCompressor streamCompressor; > + private AtomicBoolean isClosed = new AtomicBoolean(); > > private static class CompressedEntry { > final ZipArchiveEntryRequest zipArchiveEntryRequest; > @@ -124,6 +126,9 @@ public class ScatterZipOutputStream implements > Closeable { > */ > @Override > public void close() throws IOException { > + if (!isClosed.compareAndSet(false, true)) { > + return; > + } > try { > backingStore.close(); > } finally { > >
