This fixes a potential race condition in SegmentsReader where a call to numDocs() could return -1 if a document is being deleted at the same time by another thread.
(Caviat: I don't actually have a test case that shows this problem, nor was I able to verify this fix. Found it by reading the code and it seemed easy enough and important enough to put in a fix.) Also, I think there may be a larger problem with this class that happens when close() is called concurrently with any of the methods that iterate over the sub-readers and delegate to them. It looks like the second thread may get an IOException if it calls on a reader that has been closed. Not sure how to fix this without killing performance. Should we just document this as a "feature" and say that its application's responsibility to ensure that all queries are finished before calling close()? =================================================================== RCS file: /home/cvspublic/jakarta-lucene/src/java/org/apache/lucene/index/SegmentsReader.java,v retrieving revision 1.1.1.1 diff -w -u -r1.1.1.1 SegmentsReader.java --- SegmentsReader.java 2001/09/18 16:29:54 1.1.1.1 +++ SegmentsReader.java 2001/10/10 20:10:10 @@ -78,7 +78,18 @@ } public final int numDocs() { - if (numDocs == -1) { // check cache + // Synchro: two goals -- prevent duplicate cache revalidation (minor) + // and prevent invalidation of the cache between checking it and returning + // the value. + + // Atomically copy it. Even if it changes between the + // if and the return, we still return a number that was valid + // when we entered this method. + final int tmpNumDocs = numDocs; + if (tmpNumDocs != -1) return tmpNumDocs; // check cache + + synchronized(this) { + if (numDocs == -1) { int n = 0; // cache miss--recompute for (int i = 0; i < readers.length; i++) n += readers[i].numDocs(); // sum from readers @@ -86,6 +97,7 @@ } return numDocs; } + } public final int maxDoc() { return maxDoc; @@ -102,10 +114,13 @@ } public final void delete(int n) throws IOException { + // Synchro: synchronize with the cache re-validation logic in numDocs + synchronized(this) { numDocs = -1; // invalidate cache int i = readerIndex(n); // find segment num readers[i].delete(n - starts[i]); // dispatch to segment reader } + }