Fixed issue #8

Author: cowtowncoder

README.md

@@ -43,7 +43,7 @@ which would read text from file "input.txt", sort using about 20 megs of heap (n
 
 Project jar is packaged such that it can be used as a primitive 'sort' tool like so:
 
-    java -jar java-merge-sort-0.5.0.jar [input-file]
+    java -jar java-merge-sort-0.8.0.jar [input-file]
 
 where sorted output gets printed to `stdout`; and argument is optional (if missing, reads input from stdout).
 (implementation note: this uses standard `TextFileSorter` mentioned above)

release-notes/VERSION

@@ -1,15 +1,20 @@
 Project: java-merge-sort
 License: Apache License 2.0
-Version: 0.8.0
-Release date: 24-Jul-2013
+Version: 0.8.1
+Release date: xx-Aug-2013
 
-* [Issue#6]: Incorrect handling of '\r'
- (reported, patch contributed by ndikan@github.bom)
+#8: Sorter._merge() not closing streams correctly
+ (reported by tjoneslo@github)
 
 ------------------------------------------------------------------------
 === History: ===
 ------------------------------------------------------------------------
 
+0.8.0 (24-Jul-2013)
+
+* [Issue#6]: Incorrect handling of '\r'
+ (reported, patch contributed by [email protected])
+
 0.7.1 (29-May-2012)
 
 * [Issue#5]: Unnecessary object retention, leading to too high memory usage

src/main/java/com/fasterxml/sort/Merger.java

@@ -61,6 +61,8 @@ protected static class PairwiseMerger<T>
 
         protected T _data1;
         protected T _data2;
+
+        protected boolean _closed;
 
         public PairwiseMerger(Comparator<T> comparator,
                 DataReader<T> reader1, DataReader<T> reader2)
@@ -78,6 +80,8 @@ public T readNext() throws IOException
         {
             if (_data1 == null) {
                 if (_data2 == null) {
+                    // [Issue#8]: Should auto-close merged input when there is no more data
+                    close();
                     return null;
                 }
                 T result = _data2;
@@ -108,9 +112,13 @@ public int estimateSizeInBytes(T item) {
         }
 
         @Override
-        public void close() throws IOException {
-            _reader1.close();
-            _reader2.close();
+        public void close() throws IOException
+        {
+            if (!_closed) {
+                _reader1.close();
+                _reader2.close();
+                _closed = true;
+            }
         }
     }
 }

src/main/java/com/fasterxml/sort/Sorter.java

@@ -326,18 +326,28 @@ protected void _presort(DataReader<T> inputReader, SegmentedBuffer buffer, T nex
         } while (nextValue != null);
     }
 
+    @SuppressWarnings("resource")
     protected File _writePresorted(Object[] items) throws IOException
     {
         File tmp = _config.getTempFileProvider().provide();
         @SuppressWarnings("unchecked")
         DataWriter<Object> writer = (DataWriter<Object>) _writerFactory.constructWriter(new FileOutputStream(tmp));
-        ++_presortFileCount;
-        for (int i = 0, end = items.length; i < end; ++i) {
-            writer.writeEntry(items[i]);
-            // to further reduce transient mem usage, clear out the ref
-            items[i] = null;
+        boolean closed = false;
+        try {
+            ++_presortFileCount;
+            for (int i = 0, end = items.length; i < end; ++i) {
+                writer.writeEntry(items[i]);
+                // to further reduce transient mem usage, clear out the ref
+                items[i] = null;
+            }
+            closed = true;
+            writer.close();
+        } finally {
+            if (!closed) {
+                // better swallow since most likely we are getting an exception already...
+                try { writer.close(); } catch (IOException e) { }
+            }
         }
-        writer.close();
         return tmp;
     }
 
@@ -385,6 +395,7 @@ protected void _writeAll(DataWriter<T> resultWriter, Object[] items)
         }
     }
 
+    @SuppressWarnings("resource")
     protected File _merge(List<File> inputs)
         throws IOException
     {
@@ -397,17 +408,23 @@ protected void _merge(List<File> inputs, DataWriter<T> writer)
         throws IOException
     {
         ArrayList<DataReader<T>> readers = new ArrayList<DataReader<T>>(inputs.size());
+        DataReader<T> merger = null;
         try {
             for (File mergedInput : inputs) {
                 readers.add(_readerFactory.constructReader(new FileInputStream(mergedInput)));
             }
-            DataReader<T> merger = Merger.mergedReader(_comparator, readers);
+            merger = Merger.mergedReader(_comparator, readers);
             T value;
             while ((value = merger.readNext()) != null) {
                 writer.writeEntry(value);
             }
+            merger.close(); // usually not necessary (reader should close on eof) but...
+            merger = null;
             writer.close();
         } finally {
+            if (merger != null) {
+                try { merger.close(); } catch (IOException e) { }
+            }
             for (File input : inputs) {
                 input.delete();
             }