LUCENE-8254: LRUQueryCache can leak locks

lucene/CHANGES.txt

@@ -162,6 +162,10 @@ Bug Fixes
   index file names for updated doc values fields (Simon Willnauer,
   Michael McCandless, Nhat Nguyen)
 
+* LUCENE-8254: LRUQueryCache could cause IndexReader to hang on close, when
+  shared with another reader with no CacheHelper (Alan Woodward, Simon Willnauer,
+  Adrien Grand)
+
 Other
 
 * LUCENE-8228: removed obsolete IndexDeletionPolicy clone() requirements from

lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java

@@ -727,16 +727,17 @@ public class LRUQueryCache implements QueryCache, Accountable {
         return in.scorerSupplier(context);
       }
 
-      // If the lock is already busy, prefer using the uncached version than waiting
-      if (lock.tryLock() == false) {
-        return in.scorerSupplier(context);
-      }
-
       final IndexReader.CacheHelper cacheHelper = context.reader().getCoreCacheHelper();
       if (cacheHelper == null) {
         // this reader has no cache helper
         return in.scorerSupplier(context);
       }
+
+      // If the lock is already busy, prefer using the uncached version than waiting
+      if (lock.tryLock() == false) {
+        return in.scorerSupplier(context);
+      }
+
       DocIdSet docIdSet;
       try {
         docIdSet = get(in.getQuery(), context, cacheHelper);
@@ -807,16 +808,17 @@ public class LRUQueryCache implements QueryCache, Accountable {
         return in.bulkScorer(context);
       }
 
-      // If the lock is already busy, prefer using the uncached version than waiting
-      if (lock.tryLock() == false) {
-        return in.bulkScorer(context);
-      }
-
       final IndexReader.CacheHelper cacheHelper = context.reader().getCoreCacheHelper();
       if (cacheHelper == null) {
         // this reader has no cacheHelper
         return in.bulkScorer(context);
       }
+
+      // If the lock is already busy, prefer using the uncached version than waiting
+      if (lock.tryLock() == false) {
+        return in.bulkScorer(context);
+      }
+
       DocIdSet docIdSet;
       try {
         docIdSet = get(in.getQuery(), context, cacheHelper);

lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java

@@ -62,7 +62,6 @@ import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.RamUsageTester;
 import org.apache.lucene.util.TestUtil;
-import org.junit.Test;
 
 public class TestLRUQueryCache extends LuceneTestCase {
 
@@ -1479,7 +1478,6 @@ public class TestLRUQueryCache extends LuceneTestCase {
     }
   }
 
-  @Test
   public void testDocValuesUpdatesDontBreakCache() throws IOException {
     Directory dir = newDirectory();
     IndexWriterConfig iwc = newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE);
@@ -1545,4 +1543,65 @@ public class TestLRUQueryCache extends LuceneTestCase {
     dir.close();
 
   }
+
+  public void testBulkScorerLocking() throws Exception {
+
+    Directory dir = newDirectory();
+    IndexWriterConfig iwc = newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE);
+    IndexWriter w = new IndexWriter(dir, iwc);
+
+    final int numDocs = atLeast(10);
+    Document emptyDoc = new Document();
+    for (int d = 0; d < numDocs; ++d) {
+      for (int i = random().nextInt(5000); i >= 0; --i) {
+        w.addDocument(emptyDoc);
+      }
+      Document doc = new Document();
+      for (String value : Arrays.asList("foo", "bar", "baz")) {
+        if (random().nextBoolean()) {
+          doc.add(new StringField("field", value, Store.NO));
+        }
+      }
+    }
+    for (int i = TestUtil.nextInt(random(), 3000, 5000); i >= 0; --i) {
+      w.addDocument(emptyDoc);
+    }
+    if (random().nextBoolean()) {
+      w.forceMerge(1);
+    }
+
+    DirectoryReader reader = DirectoryReader.open(w);
+    DirectoryReader noCacheReader = new DummyDirectoryReader(reader);
+
+    LRUQueryCache cache = new LRUQueryCache(1, 100000, context -> true);
+    IndexSearcher searcher = new AssertingIndexSearcher(random(), reader);
+    searcher.setQueryCache(cache);
+    searcher.setQueryCachingPolicy(QueryCachingPolicy.ALWAYS_CACHE);
+
+    Query query = new ConstantScoreQuery(new BooleanQuery.Builder()
+        .add(new BoostQuery(new TermQuery(new Term("field", "foo")), 3), Occur.SHOULD)
+        .add(new BoostQuery(new TermQuery(new Term("field", "bar")), 3), Occur.SHOULD)
+        .add(new BoostQuery(new TermQuery(new Term("field", "baz")), 3), Occur.SHOULD)
+        .build());
+
+    searcher.search(query, 1);
+
+    IndexSearcher noCacheHelperSearcher = new AssertingIndexSearcher(random(), noCacheReader);
+    noCacheHelperSearcher.setQueryCache(cache);
+    noCacheHelperSearcher.setQueryCachingPolicy(QueryCachingPolicy.ALWAYS_CACHE);
+    noCacheHelperSearcher.search(query, 1);
+
+    Thread t = new Thread(() -> {
+      try {
+        noCacheReader.close();
+        w.close();
+        dir.close();
+      }
+      catch (Exception e) {
+        throw new RuntimeException(e);
+      }
+    });
+    t.start();
+    t.join();
+  }
 }