LUCENE-5654: Fix close methods that would suppress OOME or similar

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1593152 13f79535-47bb-0310-9956-ffa450edef68
apache · May 7, 2014 · 56ab1d1 · 56ab1d1
1 parent 41532ab
commit 56ab1d1
Show file tree

Hide file tree

Showing 17 changed files with 117 additions and 249 deletions.
diff --git a/lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/BinaryDictionary.java b/lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/BinaryDictionary.java
@@ -54,12 +54,12 @@ public abstract class BinaryDictionary implements Dictionary {
 
   protected BinaryDictionary() throws IOException {
     InputStream mapIS = null, dictIS = null, posIS = null;
-    IOException priorE = null;
     int[] targetMapOffsets = null, targetMap = null;
     String[] posDict = null;
     String[] inflFormDict = null;
     String[] inflTypeDict = null;
     ByteBuffer buffer = null;
+    boolean success = false;
     try {
       mapIS = getResource(TARGETMAP_FILENAME_SUFFIX);
       mapIS = new BufferedInputStream(mapIS);
@@ -117,10 +117,13 @@ protected BinaryDictionary() throws IOException {
       }
       dictIS.close(); dictIS = null;
       buffer = tmpBuffer.asReadOnlyBuffer();
-    } catch (IOException ioe) {
-      priorE = ioe;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(priorE, mapIS, posIS, dictIS);
+      if (success) {
+        IOUtils.close(mapIS, posIS, dictIS);
+      } else {
+        IOUtils.closeWhileHandlingException(mapIS, posIS, dictIS);
+      }
     }
 
     this.targetMap = targetMap;

diff --git a/...ne/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/CharacterDefinition.java b/...ne/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/CharacterDefinition.java
@@ -62,8 +62,8 @@ private static enum CharacterClass {
   public static final byte KANJINUMERIC = (byte) CharacterClass.KANJINUMERIC.ordinal();
 
   private CharacterDefinition() throws IOException {
-    IOException priorE = null;
     InputStream is = null;
+    boolean success = false;
     try {
       is = BinaryDictionary.getClassResource(getClass(), FILENAME_SUFFIX);
       is = new BufferedInputStream(is);
@@ -75,10 +75,13 @@ private CharacterDefinition() throws IOException {
         invokeMap[i] = (b & 0x01) != 0;
         groupMap[i] = (b & 0x02) != 0;
       }
-    } catch (IOException ioe) {
-      priorE = ioe;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(priorE, is);
+      if (success) {
+        IOUtils.close(is);
+      } else {
+        IOUtils.closeWhileHandlingException(is);
+      }
     }
   }
 

diff --git a/lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/ConnectionCosts.java b/lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/ConnectionCosts.java
@@ -39,9 +39,9 @@ public final class ConnectionCosts {
   private final short[][] costs; // array is backward IDs first since get is called using the same backward ID consecutively. maybe doesn't matter.
 
   private ConnectionCosts() throws IOException {
-    IOException priorE = null;
     InputStream is = null;
     short[][] costs = null;
+    boolean success = false;
     try {
       is = BinaryDictionary.getClassResource(getClass(), FILENAME_SUFFIX);
       is = new BufferedInputStream(is);
@@ -58,10 +58,13 @@ private ConnectionCosts() throws IOException {
           a[i] = (short)accum;
         }
       }
-    } catch (IOException ioe) {
-      priorE = ioe;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(priorE, is);
+      if (success) {
+        IOUtils.close(is);
+      } else {
+        IOUtils.closeWhileHandlingException(is);
+      }
     }
 
     this.costs = costs;

diff --git a/...ne/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/TokenInfoDictionary.java b/...ne/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/TokenInfoDictionary.java
@@ -38,17 +38,20 @@ public final class TokenInfoDictionary extends BinaryDictionary {
 
   private TokenInfoDictionary() throws IOException {
     super();
-    IOException priorE = null;
     InputStream is = null;
     FST<Long> fst = null;
+    boolean success = false;
     try {
       is = getResource(FST_FILENAME_SUFFIX);
       is = new BufferedInputStream(is);
       fst = new FST<>(new InputStreamDataInput(is), PositiveIntOutputs.getSingleton());
-    } catch (IOException ioe) {
-      priorE = ioe;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(priorE, is);
+      if (success) {
+        IOUtils.close(is);
+      } else {
+        IOUtils.closeWhileHandlingException(is);
+      }
     }
     // TODO: some way to configure?
     this.fst = new TokenInfoFST(fst, true);

diff --git a/lucene/codecs/src/java/org/apache/lucene/codecs/memory/FSTOrdTermsWriter.java b/lucene/codecs/src/java/org/apache/lucene/codecs/memory/FSTOrdTermsWriter.java
@@ -219,7 +219,7 @@ public void write(Fields fields) throws IOException {
   @Override
   public void close() throws IOException {
     if (blockOut != null) {
-      IOException ioe = null;
+      boolean success = false;
       try {
         final long blockDirStart = blockOut.getFilePointer();
 
@@ -247,10 +247,13 @@ public void close() throws IOException {
         writeTrailer(blockOut, blockDirStart);
         CodecUtil.writeFooter(indexOut);
         CodecUtil.writeFooter(blockOut);
-      } catch (IOException ioe2) {
-        ioe = ioe2;
+        success = true;
       } finally {
-        IOUtils.closeWhileHandlingException(ioe, blockOut, indexOut, postingsWriter);
+        if (success) {
+          IOUtils.close(blockOut, indexOut, postingsWriter);
+        } else {
+          IOUtils.closeWhileHandlingException(blockOut, indexOut, postingsWriter);
+        }
         blockOut = null;
       }
     }

diff --git a/lucene/codecs/src/java/org/apache/lucene/codecs/memory/FSTTermsWriter.java b/lucene/codecs/src/java/org/apache/lucene/codecs/memory/FSTTermsWriter.java
@@ -198,7 +198,7 @@ public void write(Fields fields) throws IOException {
   @Override
   public void close() throws IOException {
     if (out != null) {
-      IOException ioe = null;
+      boolean success = false;
       try {
         // write field summary
         final long dirStart = out.getFilePointer();
@@ -217,10 +217,13 @@ public void close() throws IOException {
         }
         writeTrailer(out, dirStart);
         CodecUtil.writeFooter(out);
-      } catch (IOException ioe2) {
-        ioe = ioe2;
+        success = true;
       } finally {
-        IOUtils.closeWhileHandlingException(ioe, out, postingsWriter);
+        if (success) {
+          IOUtils.close(out, postingsWriter);
+        } else {
+          IOUtils.closeWhileHandlingException(out, postingsWriter);
+        }
         out = null;
       }
     }

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/BlockTreeTermsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/BlockTreeTermsWriter.java
@@ -1119,7 +1119,7 @@ public void finish(BytesRef minTerm, BytesRef maxTerm) throws IOException {
   @Override
   public void close() throws IOException {
 
-    IOException ioe = null;
+    boolean success = false;
     try {
 
       final long dirStart = out.getFilePointer();
@@ -1148,10 +1148,13 @@ public void close() throws IOException {
       CodecUtil.writeFooter(out);
       writeIndexTrailer(indexOut, indexDirStart);
       CodecUtil.writeFooter(indexOut);
-    } catch (IOException ioe2) {
-      ioe = ioe2;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(ioe, out, indexOut, postingsWriter);
+      if (success) {
+        IOUtils.close(out, indexOut, postingsWriter);
+      } else {
+        IOUtils.closeWhileHandlingException(out, indexOut, postingsWriter);
+      }
     }
   }
 

diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
@@ -4498,29 +4498,25 @@ static final Collection<String> createCompoundFile(InfoStream infoStream, Direct
     // Now merge all added files
     Collection<String> files = info.files();
     CompoundFileDirectory cfsDir = new CompoundFileDirectory(directory, fileName, context, true);
-    IOException prior = null;
+    boolean success = false;
     try {
       for (String file : files) {
         directory.copy(cfsDir, file, file, context);
         checkAbort.work(directory.fileLength(file));
       }
-    } catch(IOException ex) {
-      prior = ex;
+      success = true;
     } finally {
-      boolean success = false;
-      try {
-        IOUtils.closeWhileHandlingException(prior, cfsDir);
-        success = true;
-      } finally {
-        if (!success) {
-          try {
-            directory.deleteFile(fileName);
-          } catch (Throwable t) {
-          }
-          try {
-            directory.deleteFile(IndexFileNames.segmentFileName(info.name, "", IndexFileNames.COMPOUND_FILE_ENTRIES_EXTENSION));
-          } catch (Throwable t) {
-          }
+      if (success) {
+        IOUtils.close(cfsDir);
+      } else {
+        IOUtils.closeWhileHandlingException(cfsDir);
+        try {
+          directory.deleteFile(fileName);
+        } catch (Throwable t) {
+        }
+        try {
+          directory.deleteFile(IndexFileNames.segmentFileName(info.name, "", IndexFileNames.COMPOUND_FILE_ENTRIES_EXTENSION));
+        } catch (Throwable t) {
         }
       }
     }

diff --git a/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
@@ -59,11 +59,9 @@ protected Object doBody(String segmentFileName) throws IOException {
           try {
             readers[i] = new SegmentReader(sis.info(i), IOContext.READ);
             success = true;
-          } catch(IOException ex) {
-            prior = ex;
           } finally {
             if (!success) {
-              IOUtils.closeWhileHandlingException(prior, readers);
+              IOUtils.closeWhileHandlingException(readers);
             }
           }
         }

diff --git a/lucene/core/src/java/org/apache/lucene/store/CompoundFileDirectory.java b/lucene/core/src/java/org/apache/lucene/store/CompoundFileDirectory.java
@@ -123,16 +123,17 @@ public CompoundFileDirectory(Directory directory, String fileName, IOContext con
 
   /** Helper method that reads CFS entries from an input stream */
   private final Map<String, FileEntry> readEntries(Directory dir, String name) throws IOException {
-    IOException priorE = null;
     ChecksumIndexInput entriesStream = null;
+    Map<String,FileEntry> mapping = null;
+    boolean success = false;
     try {
       final String entriesFileName = IndexFileNames.segmentFileName(
                                             IndexFileNames.stripExtension(name), "",
                                              IndexFileNames.COMPOUND_FILE_ENTRIES_EXTENSION);
       entriesStream = dir.openChecksumInput(entriesFileName, IOContext.READONCE);
       version = CodecUtil.checkHeader(entriesStream, CompoundFileWriter.ENTRY_CODEC, CompoundFileWriter.VERSION_START, CompoundFileWriter.VERSION_CURRENT);
       final int numEntries = entriesStream.readVInt();
-      final Map<String, FileEntry> mapping = new HashMap<>(numEntries);
+      mapping = new HashMap<>(numEntries);
       for (int i = 0; i < numEntries; i++) {
         final FileEntry fileEntry = new FileEntry();
         final String id = entriesStream.readString();
@@ -148,14 +149,15 @@ private final Map<String, FileEntry> readEntries(Directory dir, String name) thr
       } else {
         CodecUtil.checkEOF(entriesStream);
       }
-      return mapping;
-    } catch (IOException ioe) {
-      priorE = ioe;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(priorE, entriesStream);
+      if (success) {
+        IOUtils.close(entriesStream);
+      } else {
+        IOUtils.closeWhileHandlingException(entriesStream);
+      }
     }
-    // this is needed until Java 7's real try-with-resources:
-    throw new AssertionError("impossible to get here");
+    return mapping;
   }
 
   public Directory getDirectory() {

diff --git a/lucene/core/src/java/org/apache/lucene/store/CompoundFileWriter.java b/lucene/core/src/java/org/apache/lucene/store/CompoundFileWriter.java
@@ -129,10 +129,10 @@ public void close() throws IOException {
     if (closed) {
       return;
     }
-    IOException priorException = null;
     IndexOutput entryTableOut = null;
     // TODO this code should clean up after itself
     // (remove partial .cfs/.cfe)
+    boolean success = false;
     try {
       if (!pendingEntries.isEmpty() || outputTaken.get()) {
         throw new IllegalStateException("CFS has pending open files");
@@ -142,18 +142,25 @@ public void close() throws IOException {
       getOutput();
       assert dataOut != null;
       CodecUtil.writeFooter(dataOut);
-    } catch (IOException e) {
-      priorException = e;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(priorException, dataOut);
+      if (success) {
+        IOUtils.close(dataOut);
+      } else {
+        IOUtils.closeWhileHandlingException(dataOut);
+      }
     }
+    success = false;
     try {
       entryTableOut = directory.createOutput(entryTableName, IOContext.DEFAULT);
       writeEntryTable(entries.values(), entryTableOut);
-    } catch (IOException e) {
-      priorException = e;
+      success = true;
     } finally {
-      IOUtils.closeWhileHandlingException(priorException, entryTableOut);
+      if (success) {
+        IOUtils.close(entryTableOut);
+      } else {
+        IOUtils.closeWhileHandlingException(entryTableOut);
+      }
     }
   }
 

diff --git a/lucene/core/src/java/org/apache/lucene/store/Directory.java b/lucene/core/src/java/org/apache/lucene/store/Directory.java
@@ -181,24 +181,20 @@ public String toString() {
   public void copy(Directory to, String src, String dest, IOContext context) throws IOException {
     IndexOutput os = null;
     IndexInput is = null;
-    IOException priorException = null;
+    boolean success = false;
     try {
       os = to.createOutput(dest, context);
       is = openInput(src, context);
       os.copyBytes(is, is.length());
-    } catch (IOException ioe) {
-      priorException = ioe;
+      success = true;
     } finally {
-      boolean success = false;
-      try {
-        IOUtils.closeWhileHandlingException(priorException, os, is);
-        success = true;
-      } finally {
-        if (!success) {
-          try {
-            to.deleteFile(dest);
-          } catch (Throwable t) {
-          }
+      if (success) {
+        IOUtils.close(os, is);
+      } else {
+        IOUtils.closeWhileHandlingException(os, is);
+        try {
+          to.deleteFile(dest);
+        } catch (Throwable t) {
         }
       }
     }

diff --git a/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java b/lucene/core/src/java/org/apache/lucene/store/FSDirectory.java
@@ -378,14 +378,17 @@ public void close() throws IOException {
       parent.onIndexOutputClosed(name);
       // only close the file if it has not been closed yet
       if (isOpen) {
-        IOException priorE = null;
+        boolean success = false;
         try {
           super.close();
-        } catch (IOException ioe) {
-          priorE = ioe;
+          success = true;
         } finally {
           isOpen = false;
-          IOUtils.closeWhileHandlingException(priorE, file);
+          if (success) {
+            IOUtils.close(file);
+          } else {
+            IOUtils.closeWhileHandlingException(file);
+          }
         }
       }
     }