LUCENE-3439: catch more cases where IW/IR is used after close; fix sneaky bug where IW can delete files after it was closed

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1173720 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Michael McCandless 2011-09-21 15:33:54 +00:00
parent 7abaef7a7c
commit fb51a69173
11 changed files with 157 additions and 16 deletions

View File

@ -616,6 +616,10 @@ Bug fixes
directly into the FieldCache arrays during sorting, leading to concurrency
issues. (Uwe Schindler, Doron Cohen, Mike McCandless)
* LUCENE-3439: Closing an NRT reader after the writer was closed was
incorrectly invoking the DeletionPolicy and (then possibly deleting
files) on the closed IndexWriter (Robert Muir, Mike McCandless)
New Features
Optimizations

View File

@ -364,6 +364,7 @@ class DirectoryReader extends IndexReader implements Cloneable {
@Override
public final synchronized IndexReader clone(boolean openReadOnly) throws CorruptIndexException, IOException {
// doReopen calls ensureOpen
DirectoryReader newReader = doReopen((SegmentInfos) segmentInfos.clone(), true, openReadOnly);
if (this != newReader) {
@ -566,7 +567,7 @@ class DirectoryReader extends IndexReader implements Cloneable {
@Override
public boolean hasDeletions() {
// Don't call ensureOpen() here (it could affect performance)
ensureOpen();
return hasDeletions;
}
@ -843,7 +844,7 @@ class DirectoryReader extends IndexReader implements Cloneable {
if (writer != null) {
// Since we just closed, writer may now be able to
// delete unused files:
writer.deleteUnusedFiles();
writer.deletePendingFiles();
}
// throw the first exception
@ -868,6 +869,7 @@ class DirectoryReader extends IndexReader implements Cloneable {
@Override
public ReaderContext getTopReaderContext() {
ensureOpen();
return topLevelReaderContext;
}
@ -887,6 +889,7 @@ class DirectoryReader extends IndexReader implements Cloneable {
@Override
public int getTermInfosIndexDivisor() {
ensureOpen();
return termInfosIndexDivisor;
}
@ -897,6 +900,7 @@ class DirectoryReader extends IndexReader implements Cloneable {
*/
@Override
public IndexCommit getIndexCommit() throws IOException {
ensureOpen();
return new ReaderCommit(segmentInfos, directory);
}

View File

@ -306,11 +306,13 @@ public class FilterIndexReader extends IndexReader {
@Override
public Directory directory() {
ensureOpen();
return in.directory();
}
@Override
public Bits getLiveDocs() {
ensureOpen();
return in.getLiveDocs();
}
@ -333,7 +335,6 @@ public class FilterIndexReader extends IndexReader {
public void getTermFreqVector(int docNumber, String field, TermVectorMapper mapper) throws IOException {
ensureOpen();
in.getTermFreqVector(docNumber, field, mapper);
}
@Override
@ -362,7 +363,7 @@ public class FilterIndexReader extends IndexReader {
@Override
public boolean hasDeletions() {
// Don't call ensureOpen() here (it could affect performance)
ensureOpen();
return in.hasDeletions();
}
@ -402,14 +403,15 @@ public class FilterIndexReader extends IndexReader {
protected void doDelete(int n) throws CorruptIndexException, IOException { in.deleteDocument(n); }
@Override
protected void doCommit(Map<String,String> commitUserData) throws IOException { in.commit(commitUserData); }
protected void doCommit(Map<String,String> commitUserData) throws IOException {
in.commit(commitUserData);
}
@Override
protected void doClose() throws IOException {
in.close();
}
@Override
public Collection<String> getFieldNames(IndexReader.FieldOption fieldNames) {
ensureOpen();
@ -441,11 +443,13 @@ public class FilterIndexReader extends IndexReader {
@Override
public ReaderContext getTopReaderContext() {
ensureOpen();
return in.getTopReaderContext();
}
@Override
public Fields fields() throws IOException {
ensureOpen();
return in.fields();
}
@ -480,6 +484,7 @@ public class FilterIndexReader extends IndexReader {
@Override
public PerDocValues perDocValues() throws IOException {
ensureOpen();
return in.perDocValues();
}
}

View File

@ -115,6 +115,7 @@ public abstract class IndexReader implements Cloneable,Closeable {
*
* @lucene.experimental */
public void addReaderFinishedListener(ReaderFinishedListener listener) {
ensureOpen();
readerFinishedListeners.add(listener);
}
@ -122,6 +123,7 @@ public abstract class IndexReader implements Cloneable,Closeable {
*
* @lucene.experimental */
public void removeReaderFinishedListener(ReaderFinishedListener listener) {
ensureOpen();
readerFinishedListeners.remove(listener);
}
@ -172,7 +174,7 @@ public abstract class IndexReader implements Cloneable,Closeable {
DOC_VALUES
}
private boolean closed;
private volatile boolean closed;
protected boolean hasChanges;
private final AtomicInteger refCount = new AtomicInteger();
@ -236,7 +238,8 @@ public abstract class IndexReader implements Cloneable,Closeable {
*/
public void decRef() throws IOException {
ensureOpen();
if (refCount.getAndDecrement() == 1) {
final int rc = refCount.getAndDecrement();
if (rc == 1) {
boolean success = false;
try {
commit();
@ -249,6 +252,8 @@ public abstract class IndexReader implements Cloneable,Closeable {
}
}
readerFinished();
} else if (rc <= 0) {
throw new IllegalStateException("too many decRef calls: refCount was " + rc + " before decrement");
}
}
@ -1330,6 +1335,7 @@ public abstract class IndexReader implements Cloneable,Closeable {
* @throws IOException if there is a low-level IO error
*/
public final synchronized void commit(Map<String, String> commitUserData) throws IOException {
// Don't can ensureOpen since we commit() on close
doCommit(commitUserData);
hasChanges = false;
}
@ -1499,6 +1505,7 @@ public abstract class IndexReader implements Cloneable,Closeable {
* corruption for other readers (like DirectoryReader obtained
* through {@link #open}. Use the parent reader directly. */
public IndexReader[] getSequentialSubReaders() {
ensureOpen();
return null;
}
@ -1531,6 +1538,8 @@ public abstract class IndexReader implements Cloneable,Closeable {
/** Expert */
public Object getCoreCacheKey() {
// Don't can ensureOpen since FC calls this (to evict)
// on close
return this;
}
@ -1569,6 +1578,7 @@ public abstract class IndexReader implements Cloneable,Closeable {
}
public final IndexDocValues docValues(String field) throws IOException {
ensureOpen();
final PerDocValues perDoc = perDocValues();
if (perDoc == null) {
return null;
@ -1580,11 +1590,13 @@ public abstract class IndexReader implements Cloneable,Closeable {
/** @lucene.internal */
void storeFields(Fields fields) {
ensureOpen();
this.fields = fields;
}
/** @lucene.internal */
Fields retrieveFields() {
ensureOpen();
return fields;
}
@ -1592,11 +1604,13 @@ public abstract class IndexReader implements Cloneable,Closeable {
/** @lucene.internal */
void storePerDoc(PerDocValues perDocValues) {
ensureOpen();
this.perDocValues = perDocValues;
}
/** @lucene.internal */
PerDocValues retrievePerDoc() {
ensureOpen();
return perDocValues;
}

View File

@ -233,8 +233,8 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
private Lock writeLock;
private boolean closed;
private boolean closing;
private volatile boolean closed;
private volatile boolean closing;
// Holds all SegmentInfo instances currently involved in
// merges
@ -758,6 +758,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* delCount is returned.
*/
public int numDeletedDocs(SegmentInfo info) throws IOException {
ensureOpen(false);
SegmentReader reader = readerPool.getIfExists(info);
try {
if (reader != null) {
@ -960,6 +961,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* IndexWriterConfig} for details.
*/
public IndexWriterConfig getConfig() {
ensureOpen(false);
return config;
}
@ -1204,6 +1206,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* not counting deletions.
* @see #numDocs */
public synchronized int maxDoc() {
ensureOpen();
int count;
if (docWriter != null)
count = docWriter.getNumDocs();
@ -1221,6 +1224,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* counted you should call {@link #commit()} first.
* @see #numDocs */
public synchronized int numDocs() throws IOException {
ensureOpen();
int count;
if (docWriter != null)
count = docWriter.getNumDocs();
@ -1960,6 +1964,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
}
private final void maybeMerge(int maxNumSegmentsOptimize, boolean optimize) throws CorruptIndexException, IOException {
ensureOpen(false);
updatePendingMerges(maxNumSegmentsOptimize, optimize);
mergeScheduler.merge(this);
}
@ -2143,6 +2148,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* {@link MergePolicy.MergeAbortedException}s.
*/
public synchronized void deleteAll() throws IOException {
ensureOpen();
try {
// Abort any running merges
@ -2229,6 +2235,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* will have completed once this method completes.</p>
*/
public synchronized void waitForMerges() {
ensureOpen(false);
if (infoStream != null) {
message("waitForMerges");
}
@ -2736,6 +2743,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* index to commit.
*/
public final void prepareCommit(Map<String,String> commitUserData) throws CorruptIndexException, IOException {
ensureOpen(false);
if (infoStream != null) {
message("prepareCommit: flush");
@ -4098,10 +4106,16 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* be deleted the next time commit() is called.
*/
public synchronized void deleteUnusedFiles() throws IOException {
ensureOpen(false);
deleter.deletePendingFiles();
deleter.revisitPolicy();
}
// Called by DirectoryReader.doClose
synchronized void deletePendingFiles() throws IOException {
deleter.deletePendingFiles();
}
/**
* Sets the {@link PayloadProcessorProvider} to use when merging payloads.
* Note that the given <code>pcp</code> will be invoked for every segment that
@ -4121,6 +4135,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* {@link #optimize}, you can call {@link #waitForMerges()} before.
*/
public void setPayloadProcessorProvider(PayloadProcessorProvider pcp) {
ensureOpen();
payloadProcessorProvider = pcp;
}
@ -4129,6 +4144,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit {
* merges to process payloads.
*/
public PayloadProcessorProvider getPayloadProcessorProvider() {
ensureOpen();
return payloadProcessorProvider;
}
}

View File

@ -233,6 +233,7 @@ public class MultiReader extends IndexReader implements Cloneable {
@Override
public boolean isOptimized() {
ensureOpen();
return false;
}
@ -265,7 +266,7 @@ public class MultiReader extends IndexReader implements Cloneable {
@Override
public boolean hasDeletions() {
// Don't call ensureOpen() here (it could affect performance)
ensureOpen();
return hasDeletions;
}
@ -358,6 +359,7 @@ public class MultiReader extends IndexReader implements Cloneable {
*/
@Override
public boolean isCurrent() throws CorruptIndexException, IOException {
ensureOpen();
for (int i = 0; i < subReaders.length; i++) {
if (!subReaders[i].isCurrent()) {
return false;
@ -383,6 +385,7 @@ public class MultiReader extends IndexReader implements Cloneable {
@Override
public ReaderContext getTopReaderContext() {
ensureOpen();
return topLevelContext;
}

View File

@ -204,16 +204,19 @@ public class ParallelReader extends IndexReader {
@Override
public Bits getLiveDocs() {
ensureOpen();
return MultiFields.getLiveDocs(readers.get(0));
}
@Override
public Fields fields() {
ensureOpen();
return fields;
}
@Override
public synchronized Object clone() {
// doReopen calls ensureOpen
try {
return doReopen(true);
} catch (Exception ex) {
@ -242,6 +245,7 @@ public class ParallelReader extends IndexReader {
*/
@Override
public synchronized IndexReader reopen() throws CorruptIndexException, IOException {
// doReopen calls ensureOpen
return doReopen(false);
}
@ -325,7 +329,7 @@ public class ParallelReader extends IndexReader {
@Override
public boolean hasDeletions() {
// Don't call ensureOpen() here (it could affect performance)
ensureOpen();
return hasDeletions;
}
@ -461,6 +465,7 @@ public class ParallelReader extends IndexReader {
*/
@Override
public boolean isCurrent() throws CorruptIndexException, IOException {
ensureOpen();
for (final IndexReader reader : readers) {
if (!reader.isCurrent()) {
return false;
@ -476,6 +481,7 @@ public class ParallelReader extends IndexReader {
*/
@Override
public boolean isOptimized() {
ensureOpen();
for (final IndexReader reader : readers) {
if (!reader.isOptimized()) {
return false;
@ -527,8 +533,10 @@ public class ParallelReader extends IndexReader {
}
return fieldSet;
}
@Override
public ReaderContext getTopReaderContext() {
ensureOpen();
return topLevelReaderContext;
}
@ -550,6 +558,7 @@ public class ParallelReader extends IndexReader {
@Override
public PerDocValues perDocValues() throws IOException {
ensureOpen();
return perDocs;
}

View File

@ -135,6 +135,7 @@ public class SegmentReader extends IndexReader implements Cloneable {
@Override
public Bits getLiveDocs() {
ensureOpen();
return liveDocs;
}
@ -184,6 +185,7 @@ public class SegmentReader extends IndexReader implements Cloneable {
* @return New BitVector
*/
protected BitVector cloneDeletedDocs(BitVector bv) {
ensureOpen();
return (BitVector)bv.clone();
}
@ -214,6 +216,7 @@ public class SegmentReader extends IndexReader implements Cloneable {
}
synchronized SegmentReader reopenSegment(SegmentInfo si, boolean doClone, boolean openReadOnly) throws CorruptIndexException, IOException {
ensureOpen();
boolean deletionsUpToDate = (this.si.hasDeletions() == si.hasDeletions())
&& (!si.hasDeletions() || this.si.getDelFileName().equals(si.getDelFileName()));
boolean normsUpToDate = true;
@ -463,13 +466,13 @@ public class SegmentReader extends IndexReader implements Cloneable {
@Override
public Fields fields() throws IOException {
ensureOpen();
return core.fields;
}
@Override
public int docFreq(String field, BytesRef term) throws IOException {
ensureOpen();
Terms terms = core.fields.terms(field);
if (terms != null) {
return terms.docFreq(term);
@ -758,6 +761,7 @@ public class SegmentReader extends IndexReader implements Cloneable {
@Override
public ReaderContext getTopReaderContext() {
ensureOpen();
return readerContext;
}
@ -835,6 +839,7 @@ public class SegmentReader extends IndexReader implements Cloneable {
@Override
public PerDocValues perDocValues() throws IOException {
ensureOpen();
return core.perDocProducer;
}
}

View File

@ -67,16 +67,19 @@ public final class SlowMultiReaderWrapper extends FilterIndexReader {
@Override
public Fields fields() throws IOException {
ensureOpen();
return MultiFields.getFields(in);
}
@Override
public PerDocValues perDocValues() throws IOException {
ensureOpen();
return MultiPerDocValues.getPerDocs(in);
}
@Override
public Bits getLiveDocs() {
ensureOpen();
return MultiFields.getLiveDocs(in);
}
@ -103,6 +106,7 @@ public final class SlowMultiReaderWrapper extends FilterIndexReader {
@Override
public ReaderContext getTopReaderContext() {
ensureOpen();
return readerContext;
}

View File

@ -73,18 +73,16 @@ public class TestIndexReader extends LuceneTestCase
IndexReader r = IndexReader.open(d, false);
r.deleteDocument(5);
r.flush(commitUserData);
IndexCommit c = r.getIndexCommit();
r.close();
SegmentInfos sis = new SegmentInfos();
sis.read(d);
IndexReader r2 = IndexReader.open(d, false);
IndexCommit c = r.getIndexCommit();
assertEquals(c.getUserData(), commitUserData);
assertEquals(sis.getCurrentSegmentFileName(), c.getSegmentsFileName());
assertTrue(c.equals(r.getIndexCommit()));
// Change the index
writer = new IndexWriter(d, newIndexWriterConfig(TEST_VERSION_CURRENT,
new MockAnalyzer(random)).setOpenMode(

View File

@ -0,0 +1,79 @@
package org.apache.lucene.index;
/**
* 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 org.apache.lucene.analysis.MockAnalyzer;
import org.apache.lucene.analysis.MockTokenizer;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.StringField;
import org.apache.lucene.index.codecs.CodecProvider;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.TermRangeQuery;
import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.util._TestUtil;
public class TestReaderClosed extends LuceneTestCase {
private IndexSearcher searcher;
private IndexReader reader;
private Directory dir;
@Override
public void setUp() throws Exception {
super.setUp();
dir = newDirectory();
RandomIndexWriter writer = new RandomIndexWriter(random, dir,
newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random, MockTokenizer.KEYWORD, false))
.setMaxBufferedDocs(_TestUtil.nextInt(random, 50, 1000)));
Document doc = new Document();
Field field = newField("field", "", StringField.TYPE_UNSTORED);
doc.add(field);
// we generate aweful prefixes: good for testing.
// but for preflex codec, the test can be very slow, so use less iterations.
final String codec = CodecProvider.getDefault().getFieldCodec("field");
int num = codec.equals("PreFlex") ? 200 * RANDOM_MULTIPLIER : atLeast(1000);
for (int i = 0; i < num; i++) {
field.setValue(_TestUtil.randomUnicodeString(random, 10));
writer.addDocument(doc);
}
reader = writer.getReader();
searcher = newSearcher(reader);
writer.close();
}
public void test() throws Exception {
TermRangeQuery query = TermRangeQuery.newStringRange("field", "a", "z", true, true);
searcher.search(query, 5);
searcher.close();
reader.close();
try {
searcher.search(query, 5);
} catch (AlreadyClosedException ace) {
// expected
}
}
public void tearDown() throws Exception {
dir.close();
super.tearDown();
}
}