[1/3] commons-compress git commit: COMPRESS-470 make sure all ScatterZipOutputStreams are closed

[Gary Gregory]

ensureStreamsAreClosed() seems like an over the top name. Why not simply
closeAll()? Or close().

Gary

Repository: commons-compress
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
http://git-wip-us.apache.org/repos/asf/commons-compress/commit/2f75fbb5
http://git-wip-us.apache.org/repos/asf/commons-compress/tree/2f75fbb5
http://git-wip-us.apache.org/repos/asf/commons-compress/diff/2f75fbb5
Branch: refs/heads/master
Commit: 2f75fbb5269bd5b2c7b799b7a4f93992fa1f9196
Parents: 9792607
Authored: Sun Nov 18 16:59:31 2018 +0100
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 {

[Stefan Bodewig]

Post by Gary Gregory
ensureStreamsAreClosed() seems like an over the top name. Why not simply
closeAll()? Or close().

Because I've got a track record for choosing bad names to defend :-)

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@commons.apache.org
For additional commands, e-mail: dev-***@commons.apache.org