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
   }
+  }

Reply via email to