Index: modules/suggest/src/java/org/apache/lucene/search/suggest/fst/FSTLookup.java =================================================================== --- modules/suggest/src/java/org/apache/lucene/search/suggest/fst/FSTLookup.java (revision 1128811) +++ modules/suggest/src/java/org/apache/lucene/search/suggest/fst/FSTLookup.java (working copy) @@ -510,7 +510,7 @@ this.automaton = new FST(new InputStreamDataInput(is), NoOutputs.getSingleton()); cacheRootArcs(); } finally { - IOUtils.closeSafely(is); + IOUtils.closeSafely(false, is); } return true; } @@ -532,7 +532,7 @@ try { this.automaton.save(new OutputStreamDataOutput(os)); } finally { - IOUtils.closeSafely(os); + IOUtils.closeSafely(false, os); } return true; Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 1128811) +++ lucene/CHANGES.txt (working copy) @@ -427,6 +427,11 @@ with more document deletions is requested before a reader with fewer deletions, provided they share some segments. (yonik) +* LUCENE-3147,LUCENE-3152: Fixed open file handles leaks in many places in the + code. Now MockDirectoryWrapper (in test-framework) tracks all open files, + including locks, and fails if the test fails to release all of them. + (Mike McCandless, Robert Muir, Shai Erera, Simon Willnauer) + ======================= Lucene 3.x (not yet released) ================ (No changes) Index: lucene/src/test/org/apache/lucene/search/TestFieldCache.java =================================================================== --- lucene/src/test/org/apache/lucene/search/TestFieldCache.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/search/TestFieldCache.java (working copy) @@ -217,6 +217,7 @@ IndexReader r = IndexReader.open(writer, true); FieldCache.DocTerms terms = FieldCache.DEFAULT.getTerms(r, "foobar"); FieldCache.DocTermsIndex termsIndex = FieldCache.DEFAULT.getTermsIndex(r, "foobar"); + writer.close(); r.close(); dir.close(); } Index: lucene/src/test/org/apache/lucene/index/TestIndexReader.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexReader.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/index/TestIndexReader.java (working copy) @@ -1688,6 +1688,7 @@ r2.close(); assertTrue(ints == ints2); + writer.close(); dir.close(); } @@ -1735,6 +1736,7 @@ assertTrue(((SegmentReader) subs[1]).readOnly); assertTrue(ints == ints2); + writer.close(); dir.close(); } Index: lucene/src/test/org/apache/lucene/index/TestIndexWriter.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriter.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriter.java (working copy) @@ -2123,6 +2123,10 @@ allowInterrupt = true; } } catch (ThreadInterruptedException re) { + if (VERBOSE) { + System.out.println("TEST: got interrupt"); + re.printStackTrace(System.out); + } Throwable e = re.getCause(); assertTrue(e instanceof InterruptedException); if (finish) { @@ -2720,7 +2724,7 @@ // or, at most the write.lock file final int extraFileCount; if (files.length == 1) { - assertEquals("write.lock", files[0]); + assertTrue(files[0].endsWith("write.lock")); extraFileCount = 1; } else { assertEquals(0, files.length); Index: lucene/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java (working copy) @@ -919,7 +919,7 @@ assertTrue(failure.failOnCommit && failure.failOnDeleteFile); w.rollback(); assertFalse(dir.fileExists("1.fnx")); - // FIXME: on windows, this often fails! assertEquals(0, dir.listAll().length); + assertEquals(0, dir.listAll().length); dir.close(); } } Index: lucene/src/test/org/apache/lucene/index/TestCrash.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestCrash.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/index/TestCrash.java (working copy) @@ -176,6 +176,7 @@ reader = IndexReader.open(dir, false); assertEquals(157, reader.numDocs()); reader.close(); + dir.clearCrash(); dir.close(); } Index: lucene/src/test/org/apache/lucene/index/TestIndexWriterWithThreads.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriterWithThreads.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriterWithThreads.java (working copy) @@ -65,6 +65,10 @@ writer.updateDocument(new Term("id", ""+(idUpto++)), doc); addCount++; } catch (IOException ioe) { + if (VERBOSE) { + System.out.println("TEST: expected exc:"); + ioe.printStackTrace(System.out); + } //System.out.println(Thread.currentThread().getName() + ": hit exc"); //ioe.printStackTrace(System.out); if (ioe.getMessage().startsWith("fake disk full at") || @@ -218,6 +222,9 @@ int NUM_THREADS = 3; for(int iter=0;iter<2;iter++) { + if (VERBOSE) { + System.out.println("TEST: iter=" + iter); + } MockDirectoryWrapper dir = newDirectory(); IndexWriter writer = new IndexWriter( @@ -228,6 +235,7 @@ setMergePolicy(newLogMergePolicy(4)) ); ((ConcurrentMergeScheduler) writer.getConfig().getMergeScheduler()).setSuppressExceptions(); + writer.setInfoStream(VERBOSE ? System.out : null); IndexerThread[] threads = new IndexerThread[NUM_THREADS]; Index: lucene/src/test/org/apache/lucene/index/TestIndexWriterDelete.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriterDelete.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriterDelete.java (working copy) @@ -833,6 +833,10 @@ try { modifier.addDocument(doc); } catch (IOException io) { + if (VERBOSE) { + System.out.println("TEST: got expected exc:"); + io.printStackTrace(System.out); + } break; } } Index: lucene/src/test/org/apache/lucene/store/TestLockFactory.java =================================================================== --- lucene/src/test/org/apache/lucene/store/TestLockFactory.java (revision 1128811) +++ lucene/src/test/org/apache/lucene/store/TestLockFactory.java (working copy) @@ -255,15 +255,21 @@ // write.lock is stored in index): public void testDefaultFSLockFactoryPrefix() throws IOException { - // Make sure we get null prefix: + // Make sure we get null prefix, which wont happen if setLockFactory is ever called. File dirName = _TestUtil.getTempDir("TestLockFactory.10"); - Directory dir = newFSDirectory(dirName); - String prefix = dir.getLockFactory().getLockPrefix(); - - assertTrue("Default lock prefix should be null", null == prefix); - + Directory dir = new SimpleFSDirectory(dirName); + assertNull("Default lock prefix should be null", dir.getLockFactory().getLockPrefix()); dir.close(); + + dir = new MMapDirectory(dirName); + assertNull("Default lock prefix should be null", dir.getLockFactory().getLockPrefix()); + dir.close(); + + dir = new NIOFSDirectory(dirName); + assertNull("Default lock prefix should be null", dir.getLockFactory().getLockPrefix()); + dir.close(); + _TestUtil.rmDir(dirName); } Index: lucene/src/java/org/apache/lucene/index/DocInverterPerField.java =================================================================== --- lucene/src/java/org/apache/lucene/index/DocInverterPerField.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/DocInverterPerField.java (working copy) @@ -53,8 +53,11 @@ @Override void abort() { - consumer.abort(); - endConsumer.abort(); + try { + consumer.abort(); + } finally { + endConsumer.abort(); + } } @Override Index: lucene/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java =================================================================== --- lucene/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java (working copy) @@ -177,7 +177,7 @@ this.parent = parent; this.fieldInfos = fieldInfos; this.writer = parent.indexWriter; - this.infoStream = parent.indexWriter.getInfoStream(); + this.infoStream = parent.infoStream; this.docState = new DocState(this); this.docState.similarityProvider = parent.indexWriter.getConfig() .getSimilarityProvider(); @@ -550,6 +550,7 @@ super(blockSize); } + @Override public byte[] getByteBlock() { bytesUsed.addAndGet(blockSize); return new byte[blockSize]; @@ -562,7 +563,7 @@ } } - }; + } void setInfoStream(PrintStream infoStream) { this.infoStream = infoStream; Index: lucene/src/java/org/apache/lucene/index/FieldsWriter.java =================================================================== --- lucene/src/java/org/apache/lucene/index/FieldsWriter.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/FieldsWriter.java (working copy) @@ -113,7 +113,7 @@ void close() throws IOException { if (directory != null) { try { - IOUtils.closeSafely(fieldsStream, indexStream); + IOUtils.closeSafely(false, fieldsStream, indexStream); } finally { fieldsStream = indexStream = null; } Index: lucene/src/java/org/apache/lucene/index/SegmentInfos.java =================================================================== --- lucene/src/java/org/apache/lucene/index/SegmentInfos.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/SegmentInfos.java (working copy) @@ -40,6 +40,7 @@ import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; import org.apache.lucene.store.NoSuchDirectoryException; +import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.ThreadInterruptedException; /** @@ -323,18 +324,14 @@ SegmentInfosWriter infosWriter = codecs.getSegmentInfosWriter(); segnOutput = infosWriter.writeInfos(directory, segmentFileName, this); infosWriter.prepareCommit(segnOutput); - success = true; pendingSegnOutput = segnOutput; + success = true; } finally { if (!success) { // We hit an exception above; try to close the file // but suppress any exception: + IOUtils.closeSafely(true, segnOutput); try { - segnOutput.close(); - } catch (Throwable t) { - // Suppress so we keep throwing the original exception - } - try { // Try not to leave a truncated segments_N file in // the index: directory.deleteFile(segmentFileName); @@ -945,9 +942,12 @@ } finally { genOutput.close(); } + } catch (ThreadInterruptedException t) { + throw t; } catch (Throwable t) { // It's OK if we fail to write this file since it's // used only as one of the retry fallbacks. + // nocommit if this is thread interrupted we should rethrow } } @@ -962,7 +962,6 @@ prepareCommit(dir); finishCommit(dir); } - public String toString(Directory directory) { StringBuilder buffer = new StringBuilder(); Index: lucene/src/java/org/apache/lucene/index/DocumentsWriterFlushControl.java =================================================================== --- lucene/src/java/org/apache/lucene/index/DocumentsWriterFlushControl.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/DocumentsWriterFlushControl.java (working copy) @@ -16,6 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -204,7 +205,7 @@ } // don't assert on numDocs since we could hit an abort excp. while selecting that dwpt for flushing } - + synchronized void doOnAbort(ThreadState state) { try { if (state.flushPending) { @@ -449,10 +450,21 @@ try { for (DocumentsWriterPerThread dwpt : flushQueue) { doAfterFlush(dwpt); + try { + dwpt.abort(); + } catch (IOException ex) { + // continue + } } for (BlockedFlush blockedFlush : blockedFlushes) { - flushingWriters.put(blockedFlush.dwpt, Long.valueOf(blockedFlush.bytes)); + flushingWriters + .put(blockedFlush.dwpt, Long.valueOf(blockedFlush.bytes)); doAfterFlush(blockedFlush.dwpt); + try { + blockedFlush.dwpt.abort(); + } catch (IOException ex) { + // continue + } } } finally { fullFlush = false; @@ -511,4 +523,4 @@ return stallControl.anyStalledThreads(); } -} \ No newline at end of file +} Index: lucene/src/java/org/apache/lucene/index/TermsHash.java =================================================================== --- lucene/src/java/org/apache/lucene/index/TermsHash.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/TermsHash.java (working copy) @@ -54,7 +54,6 @@ final boolean trackAllocations; - public TermsHash(final DocumentsWriterPerThread docWriter, final TermsHashConsumer consumer, boolean trackAllocations, final TermsHash nextTermsHash) { this.docState = docWriter.docState; this.docWriter = docWriter; @@ -108,11 +107,11 @@ } for (final Map.Entry entry : fieldsToFlush.entrySet()) { - TermsHashPerField perField = (TermsHashPerField) entry.getValue(); - childFields.put(entry.getKey(), perField.consumer); - if (nextTermsHash != null) { - nextChildFields.put(entry.getKey(), perField.nextPerField); - } + TermsHashPerField perField = (TermsHashPerField) entry.getValue(); + childFields.put(entry.getKey(), perField.consumer); + if (nextTermsHash != null) { + nextChildFields.put(entry.getKey(), perField.nextPerField); + } } consumer.flush(childFields, state); @@ -134,12 +133,9 @@ @Override void finishDocument() throws IOException { - try { - consumer.finishDocument(this); - } finally { - if (nextTermsHash != null) { - nextTermsHash.consumer.finishDocument(nextTermsHash); - } + consumer.finishDocument(this); + if (nextTermsHash != null) { + nextTermsHash.consumer.finishDocument(nextTermsHash); } } Index: lucene/src/java/org/apache/lucene/index/SegmentMerger.java =================================================================== --- lucene/src/java/org/apache/lucene/index/SegmentMerger.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/SegmentMerger.java (working copy) @@ -22,7 +22,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.regex.Pattern; // for assert import org.apache.lucene.document.Document; import org.apache.lucene.index.IndexReader.FieldOption; @@ -34,6 +33,7 @@ import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; import org.apache.lucene.util.Bits; +import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.MultiBits; import org.apache.lucene.util.ReaderUtil; @@ -546,14 +546,13 @@ } codec = segmentWriteState.segmentCodecs.codec(); final FieldsConsumer consumer = codec.fieldsConsumer(segmentWriteState); - - // NOTE: this is silly, yet, necessary -- we create a - // MultiBits as our skip docs only to have it broken - // apart when we step through the docs enums in - // MultiDocsEnum. - mergeState.multiDeletedDocs = new MultiBits(bits, bitsStarts); - try { + // NOTE: this is silly, yet, necessary -- we create a + // MultiBits as our skip docs only to have it broken + // apart when we step through the docs enums in + // MultiDocsEnum. + mergeState.multiDeletedDocs = new MultiBits(bits, bitsStarts); + consumer.merge(mergeState, new MultiFields(fields.toArray(Fields.EMPTY_ARRAY), slices.toArray(ReaderUtil.Slice.EMPTY_ARRAY))); @@ -579,6 +578,7 @@ private void mergeNorms() throws IOException { IndexOutput output = null; + boolean success = false; try { for (FieldInfo fi : fieldInfos) { if (fi.isIndexed && !fi.omitNorms) { @@ -612,10 +612,9 @@ } } } + success = true; } finally { - if (output != null) { - output.close(); - } + IOUtils.closeSafely(!success, output); } } } Index: lucene/src/java/org/apache/lucene/index/DocFieldProcessor.java =================================================================== --- lucene/src/java/org/apache/lucene/index/DocFieldProcessor.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/DocFieldProcessor.java (working copy) @@ -71,7 +71,15 @@ childFields.put(f.getFieldInfo(), f); } - fieldsWriter.flush(state); + boolean success = false; + try { + fieldsWriter.flush(state); + success = true; + } finally { + if (!success) { + abort(); + } + } consumer.flush(childFields, state); // Important to save after asking consumer to flush so @@ -84,20 +92,45 @@ @Override public void abort() { - for(int i=0;i it = consumers.iterator(); - IOException err = null; - while (it.hasNext()) { - try { - it.next().close(); - } catch (IOException ioe) { - // keep first IOException we hit but keep - // closing the rest - if (err == null) { - err = ioe; - } - } - } - if (err != null) { - throw err; - } + IOUtils.closeSafely(false, consumers); } } @@ -122,14 +116,7 @@ // If we hit exception (eg, IOE because writer was // committing, or, for any other reason) we must // go back and close all FieldsProducers we opened: - for(FieldsProducer fp : producers.values()) { - try { - fp.close(); - } catch (Throwable t) { - // Suppress all exceptions here so we continue - // to throw the original one - } - } + IOUtils.closeSafely(true, producers.values()); } } } @@ -177,22 +164,7 @@ @Override public void close() throws IOException { - Iterator it = codecs.values().iterator(); - IOException err = null; - while (it.hasNext()) { - try { - it.next().close(); - } catch (IOException ioe) { - // keep first IOException we hit but keep - // closing the rest - if (err == null) { - err = ioe; - } - } - } - if (err != null) { - throw err; - } + IOUtils.closeSafely(false, codecs.values()); } @Override Index: lucene/src/java/org/apache/lucene/index/DocumentsWriter.java =================================================================== --- lucene/src/java/org/apache/lucene/index/DocumentsWriter.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/DocumentsWriter.java (working copy) @@ -228,14 +228,19 @@ } final Iterator threadsIterator = perThreadPool.getActivePerThreadsIterator(); - while (threadsIterator.hasNext()) { - ThreadState perThread = threadsIterator.next(); + final ThreadState perThread = threadsIterator.next(); perThread.lock(); try { if (perThread.isActive()) { // we might be closed - perThread.perThread.abort(); - perThread.perThread.checkAndResetHasAborted(); + try { + perThread.perThread.abort(); + } catch (IOException ex) { + // continue + } finally { + perThread.perThread.checkAndResetHasAborted(); + flushControl.doOnAbort(perThread); + } } else { assert closed; } @@ -243,7 +248,6 @@ perThread.unlock(); } } - success = true; } finally { if (infoStream != null) { Index: lucene/src/java/org/apache/lucene/index/TermVectorsWriter.java =================================================================== --- lucene/src/java/org/apache/lucene/index/TermVectorsWriter.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/TermVectorsWriter.java (working copy) @@ -31,15 +31,22 @@ private FieldInfos fieldInfos; public TermVectorsWriter(Directory directory, String segment, - FieldInfos fieldInfos) - throws IOException { - // Open files for TermVector storage - tvx = directory.createOutput(IndexFileNames.segmentFileName(segment, "", IndexFileNames.VECTORS_INDEX_EXTENSION)); - tvx.writeInt(TermVectorsReader.FORMAT_CURRENT); - tvd = directory.createOutput(IndexFileNames.segmentFileName(segment, "", IndexFileNames.VECTORS_DOCUMENTS_EXTENSION)); - tvd.writeInt(TermVectorsReader.FORMAT_CURRENT); - tvf = directory.createOutput(IndexFileNames.segmentFileName(segment, "", IndexFileNames.VECTORS_FIELDS_EXTENSION)); - tvf.writeInt(TermVectorsReader.FORMAT_CURRENT); + FieldInfos fieldInfos) throws IOException { + boolean success = false; + try { + // Open files for TermVector storage + tvx = directory.createOutput(IndexFileNames.segmentFileName(segment, "", IndexFileNames.VECTORS_INDEX_EXTENSION)); + tvx.writeInt(TermVectorsReader.FORMAT_CURRENT); + tvd = directory.createOutput(IndexFileNames.segmentFileName(segment, "", IndexFileNames.VECTORS_DOCUMENTS_EXTENSION)); + tvd.writeInt(TermVectorsReader.FORMAT_CURRENT); + tvf = directory.createOutput(IndexFileNames.segmentFileName(segment, "", IndexFileNames.VECTORS_FIELDS_EXTENSION)); + tvf.writeInt(TermVectorsReader.FORMAT_CURRENT); + success = true; + } finally { + if (!success) { + IOUtils.closeSafely(true, tvx, tvd, tvf); + } + } this.fieldInfos = fieldInfos; } @@ -51,8 +58,7 @@ * @param vectors * @throws IOException */ - public final void addAllDocVectors(TermFreqVector[] vectors) - throws IOException { + public final void addAllDocVectors(TermFreqVector[] vectors) throws IOException { tvx.writeLong(tvd.getFilePointer()); tvx.writeLong(tvf.getFilePointer()); @@ -187,6 +193,6 @@ final void close() throws IOException { // make an effort to close all streams we can but remember and re-throw // the first exception encountered in this process - IOUtils.closeSafely(tvx, tvd, tvf); + IOUtils.closeSafely(false, tvx, tvd, tvf); } } Index: lucene/src/java/org/apache/lucene/index/codecs/pulsing/PulsingCodec.java =================================================================== --- lucene/src/java/org/apache/lucene/index/codecs/pulsing/PulsingCodec.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/codecs/pulsing/PulsingCodec.java (working copy) @@ -38,6 +38,7 @@ import org.apache.lucene.index.codecs.TermsIndexWriterBase; import org.apache.lucene.index.codecs.standard.StandardCodec; import org.apache.lucene.store.Directory; +import org.apache.lucene.util.IOUtils; /** This codec "inlines" the postings for terms that have * low docFreq. It wraps another codec, which is used for @@ -81,7 +82,7 @@ success = true; } finally { if (!success) { - pulsingWriter.close(); + IOUtils.closeSafely(true, pulsingWriter); } } @@ -93,11 +94,7 @@ return ret; } finally { if (!success) { - try { - pulsingWriter.close(); - } finally { - indexWriter.close(); - } + IOUtils.closeSafely(true, pulsingWriter, indexWriter); } } } Index: lucene/src/java/org/apache/lucene/index/codecs/pulsing/PulsingPostingsWriterImpl.java =================================================================== --- lucene/src/java/org/apache/lucene/index/codecs/pulsing/PulsingPostingsWriterImpl.java (revision 1128811) +++ lucene/src/java/org/apache/lucene/index/codecs/pulsing/PulsingPostingsWriterImpl.java (working copy) @@ -71,8 +71,6 @@ * for this term) is <= maxPositions, then the postings are * inlined into terms dict */ public PulsingPostingsWriterImpl(int maxPositions, PostingsWriterBase wrappedPostingsWriter) throws IOException { - super(); - pending = new Position[maxPositions]; for(int i=0;i unSyncedFiles; private Set createdFiles; - Set openFilesForWrite = new HashSet(); + private Set openFilesForWrite = new HashSet(); + Set openLocks = Collections.synchronizedSet(new HashSet()); volatile boolean crashed; private ThrottledIndexOutput throttledOutput; private Throttling throttling = Throttling.SOMETIMES; // use this for tracking files for crash. // additionally: provides debugging information in case you leave one open - Map openFileHandles = Collections.synchronizedMap(new IdentityHashMap()); + private Map openFileHandles = Collections.synchronizedMap(new IdentityHashMap()); // NOTE: we cannot initialize the Map here due to the // order in which our constructor actually does this // member initialization vs when it calls super. It seems // like super is called, then our members are initialized: - Map openFiles; + private Map openFiles; // Only tracked if noDeleteOpenFile is true: if an attempt // is made to delete an open file, we enroll it here. - Set openFilesDeleted; + private Set openFilesDeleted; private synchronized void init() { if (openFiles == null) { @@ -107,6 +108,12 @@ this.randomState = new Random(random.nextInt()); this.throttledOutput = new ThrottledIndexOutput(ThrottledIndexOutput .mBitsToBytes(40 + randomState.nextInt(10)), 5 + randomState.nextInt(5), null); + // force wrapping of lockfactory + try { + setLockFactory(new MockLockFactoryWrapper(this, delegate.getLockFactory())); + } catch (IOException e) { + throw new RuntimeException(e); + } init(); } @@ -127,7 +134,7 @@ SOMETIMES, /** never throttle output */ NEVER - }; + } public void setThrottling(Throttling throttling) { this.throttling = throttling; @@ -208,6 +215,7 @@ public synchronized void clearCrash() throws IOException { crashed = false; + openLocks.clear(); } public void setMaxSizeInBytes(long maxSize) { @@ -362,9 +370,10 @@ ramdir.fileMap.put(name, file); } } + //System.out.println(Thread.currentThread().getName() + ": MDW: create " + name); IndexOutput io = new MockIndexOutputWrapper(this, delegate.createOutput(name), name); - openFileHandles.put(io, new RuntimeException("unclosed IndexOutput")); + addFileHandle(io, name, false); openFilesForWrite.add(name); // throttling REALLY slows down tests, so don't do it very often for SOMETIMES. @@ -379,6 +388,18 @@ } } + private void addFileHandle(Closeable c, String name, boolean input) { + Integer v = openFiles.get(name); + if (v != null) { + v = Integer.valueOf(v.intValue()+1); + openFiles.put(name, v); + } else { + openFiles.put(name, Integer.valueOf(1)); + } + + openFileHandles.put(c, new RuntimeException("unclosed Index" + (input ? "Input" : "Output") + ": " + name)); + } + @Override public synchronized IndexInput openInput(String name) throws IOException { maybeYield(); @@ -391,16 +412,8 @@ throw fillOpenTrace(new IOException("MockDirectoryWrapper: file \"" + name + "\" is still open for writing"), name, false); } - if (openFiles.containsKey(name)) { - Integer v = openFiles.get(name); - v = Integer.valueOf(v.intValue()+1); - openFiles.put(name, v); - } else { - openFiles.put(name, Integer.valueOf(1)); - } - IndexInput ii = new MockIndexInputWrapper(this, name, delegate.openInput(name)); - openFileHandles.put(ii, new RuntimeException("unclosed IndexInput")); + addFileHandle(ii, name, true); return ii; } @@ -447,6 +460,9 @@ // super() does not throw IOException currently: throw new RuntimeException("MockDirectoryWrapper: cannot close: there are still open files: " + openFiles, cause); } + if (noDeleteOpenFile && openLocks.size() > 0) { + throw new RuntimeException("MockDirectoryWrapper: cannot close: there are still open locks: " + openLocks); + } open = false; if (checkIndexOnClose) { if (LuceneTestCase.VERBOSE) { @@ -465,6 +481,31 @@ delegate.close(); } + private synchronized void removeOpenFile(Closeable c, String name) { + Integer v = openFiles.get(name); + // Could be null when crash() was called + if (v != null) { + if (v.intValue() == 1) { + openFiles.remove(name); + openFilesDeleted.remove(name); + } else { + v = Integer.valueOf(v.intValue()-1); + openFiles.put(name, v); + } + } + + openFileHandles.remove(c); + } + + public synchronized void removeIndexOutput(IndexOutput out, String name) { + openFilesForWrite.remove(name); + removeOpenFile(out, name); + } + + public synchronized void removeIndexInput(IndexInput in, String name) { + removeOpenFile(in, name); + } + private CodecProvider codecProvider; // We pass this CodecProvider to checkIndex when dir is closed... Index: lucene/src/test-framework/org/apache/lucene/store/MockIndexInputWrapper.java =================================================================== --- lucene/src/test-framework/org/apache/lucene/store/MockIndexInputWrapper.java (revision 1128811) +++ lucene/src/test-framework/org/apache/lucene/store/MockIndexInputWrapper.java (working copy) @@ -31,8 +31,7 @@ private IndexInput delegate; private boolean isClone; - /** Construct an empty output buffer. - * @throws IOException */ + /** Construct an empty output buffer. */ public MockIndexInputWrapper(MockDirectoryWrapper dir, String name, IndexInput delegate) { this.name = name; this.dir = dir; @@ -41,24 +40,17 @@ @Override public void close() throws IOException { - delegate.close(); - // Pending resolution on LUCENE-686 we may want to - // remove the conditional check so we also track that - // all clones get closed: - if (!isClone) { - synchronized(dir) { - Integer v = dir.openFiles.get(name); - // Could be null when MockRAMDirectory.crash() was called - if (v != null) { - if (v.intValue() == 1) { - dir.openFiles.remove(name); - dir.openFilesDeleted.remove(name); - } else { - v = Integer.valueOf(v.intValue()-1); - dir.openFiles.put(name, v); - } - } - dir.openFileHandles.remove(this); + try { + // turn on the following to look for leaks closing inputs, + // after fixing TestTransactions + // dir.maybeThrowDeterministicException(); + } finally { + delegate.close(); + // Pending resolution on LUCENE-686 we may want to + // remove the conditional check so we also track that + // all clones get closed: + if (!isClone) { + dir.removeIndexInput(this, name); } } } Index: lucene/src/test-framework/org/apache/lucene/store/MockIndexOutputWrapper.java =================================================================== --- lucene/src/test-framework/org/apache/lucene/store/MockIndexOutputWrapper.java (revision 1128811) +++ lucene/src/test-framework/org/apache/lucene/store/MockIndexOutputWrapper.java (working copy) @@ -57,10 +57,7 @@ dir.maxUsedSize = size; } } - synchronized(dir) { - dir.openFileHandles.remove(this); - dir.openFilesForWrite.remove(name); - } + dir.removeIndexOutput(this, name); } } Index: lucene/src/test-framework/org/apache/lucene/store/MockLockFactoryWrapper.java =================================================================== --- lucene/src/test-framework/org/apache/lucene/store/MockLockFactoryWrapper.java (revision 0) +++ lucene/src/test-framework/org/apache/lucene/store/MockLockFactoryWrapper.java (revision 0) @@ -0,0 +1,87 @@ +package org.apache.lucene.store; + +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.IOException; + +public class MockLockFactoryWrapper extends LockFactory { + MockDirectoryWrapper dir; + LockFactory delegate; + + public MockLockFactoryWrapper(MockDirectoryWrapper dir, LockFactory delegate) { + this.dir = dir; + this.delegate = delegate; + } + + @Override + public void setLockPrefix(String lockPrefix) { + delegate.setLockPrefix(lockPrefix); + } + + @Override + public String getLockPrefix() { + return delegate.getLockPrefix(); + } + + @Override + public Lock makeLock(String lockName) { + return new MockLock(delegate.makeLock(lockName), lockName); + } + + @Override + public void clearLock(String lockName) throws IOException { + delegate.clearLock(lockName); + dir.openLocks.remove(lockName); + } + + @Override + public String toString() { + return "MockLockFactoryWrapper(" + delegate.toString() + ")"; + } + + private class MockLock extends Lock { + private Lock delegateLock; + private String name; + + MockLock(Lock delegate, String name) { + this.delegateLock = delegate; + this.name = name; + } + + @Override + public boolean obtain() throws IOException { + if (delegateLock.obtain()) { + dir.openLocks.add(name); + return true; + } else { + return false; + } + } + + @Override + public void release() throws IOException { + delegateLock.release(); + dir.openLocks.remove(name); + } + + @Override + public boolean isLocked() throws IOException { + return delegateLock.isLocked(); + } + } +} Property changes on: lucene/src/test-framework/org/apache/lucene/store/MockLockFactoryWrapper.java ___________________________________________________________________ Added: svn:eol-style + native Index: lucene/src/test-framework/org/apache/lucene/util/ThrottledIndexOutput.java =================================================================== --- lucene/src/test-framework/org/apache/lucene/util/ThrottledIndexOutput.java (revision 1128811) +++ lucene/src/test-framework/org/apache/lucene/util/ThrottledIndexOutput.java (working copy) @@ -73,9 +73,11 @@ @Override public void close() throws IOException { - sleep(closeDelayMillis + getDelay(true)); - delegate.close(); - + try { + sleep(closeDelayMillis + getDelay(true)); + } finally { + delegate.close(); + } } @Override Index: lucene/src/test-framework/org/apache/lucene/util/LuceneTestCase.java =================================================================== --- lucene/src/test-framework/org/apache/lucene/util/LuceneTestCase.java (revision 1128811) +++ lucene/src/test-framework/org/apache/lucene/util/LuceneTestCase.java (working copy) @@ -469,6 +469,7 @@ if (ste.getClassName().indexOf("org.apache.lucene") == -1) break; System.err.println("\t" + ste); } + fail("could not remove temp dir: " + entry.getKey()); } } } @@ -971,7 +972,10 @@ clazz = Class.forName(fsdirClass).asSubclass(FSDirectory.class); } - MockDirectoryWrapper dir = new MockDirectoryWrapper(random, newFSDirectoryImpl(clazz, f, lf)); + MockDirectoryWrapper dir = new MockDirectoryWrapper(random, newFSDirectoryImpl(clazz, f)); + if (lf != null) { + dir.setLockFactory(lf); + } stores.put(dir, Thread.currentThread().getStackTrace()); return dir; } catch (Exception e) { @@ -1105,7 +1109,7 @@ } private static Directory newFSDirectoryImpl( - Class clazz, File file, LockFactory lockFactory) + Class clazz, File file) throws IOException { FSDirectory d = null; try { @@ -1116,9 +1120,6 @@ } catch (Exception e) { d = FSDirectory.open(file); } - if (lockFactory != null) { - d.setLockFactory(lockFactory); - } return d; } @@ -1140,7 +1141,7 @@ tmpFile.delete(); tmpFile.mkdir(); registerTempFile(tmpFile); - return newFSDirectoryImpl(clazz.asSubclass(FSDirectory.class), tmpFile, null); + return newFSDirectoryImpl(clazz.asSubclass(FSDirectory.class), tmpFile); } // try empty ctor Index: lucene/contrib/misc/src/java/org/apache/lucene/store/NRTCachingDirectory.java =================================================================== --- lucene/contrib/misc/src/java/org/apache/lucene/store/NRTCachingDirectory.java (revision 1128811) +++ lucene/contrib/misc/src/java/org/apache/lucene/store/NRTCachingDirectory.java (working copy) @@ -269,7 +269,7 @@ in = cache.openInput(fileName); in.copyBytes(out, in.length()); } finally { - IOUtils.closeSafely(in, out); + IOUtils.closeSafely(false, in, out); } synchronized(this) { cache.deleteFile(fileName);