From 4c9f1eec13e5000257f1b07afadff314a83eb057 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 29 Jul 2025 04:11:35 -0400 Subject: [PATCH 1/4] remove IndexReader.registerParentReader() Remove this function and associated WeakhashMap: it is not needed any more. The tests still pass, user still gets ACE (just slightly different exception message). Closes #14999 --- .../lucene/index/BaseCompositeReader.java | 1 - .../apache/lucene/index/FilterLeafReader.java | 1 - .../org/apache/lucene/index/IndexReader.java | 36 +------------------ .../lucene/index/ParallelLeafReader.java | 1 - .../apache/lucene/index/TestReaderClosed.java | 4 +-- 5 files changed, 2 insertions(+), 41 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/BaseCompositeReader.java b/lucene/core/src/java/org/apache/lucene/index/BaseCompositeReader.java index dcda91a4c5d0..d3ae0c2ae114 100644 --- a/lucene/core/src/java/org/apache/lucene/index/BaseCompositeReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/BaseCompositeReader.java @@ -84,7 +84,6 @@ protected BaseCompositeReader(R[] subReaders, Comparator subReadersSorter) th starts[i] = (int) maxDoc; final IndexReader r = subReaders[i]; maxDoc += r.maxDoc(); // compute maxDocs - r.registerParentReader(this); } if (maxDoc > IndexWriter.getActualMaxDocs()) { diff --git a/lucene/core/src/java/org/apache/lucene/index/FilterLeafReader.java b/lucene/core/src/java/org/apache/lucene/index/FilterLeafReader.java index 87d62f22d041..8f1edf4962ee 100644 --- a/lucene/core/src/java/org/apache/lucene/index/FilterLeafReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/FilterLeafReader.java @@ -334,7 +334,6 @@ protected FilterLeafReader(LeafReader in) { throw new NullPointerException("incoming LeafReader must not be null"); } this.in = in; - in.registerParentReader(this); } @Override diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexReader.java b/lucene/core/src/java/org/apache/lucene/index/IndexReader.java index 8e965ee8099c..f14b36ac89af 100644 --- a/lucene/core/src/java/org/apache/lucene/index/IndexReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/IndexReader.java @@ -18,10 +18,7 @@ import java.io.Closeable; import java.io.IOException; -import java.util.Collections; import java.util.List; -import java.util.Set; -import java.util.WeakHashMap; import java.util.concurrent.atomic.AtomicInteger; import org.apache.lucene.store.AlreadyClosedException; @@ -146,24 +143,6 @@ public interface ClosedListener { void onClose(CacheKey key) throws IOException; } - private final Set parentReaders = - Collections.synchronizedSet( - Collections.newSetFromMap(new WeakHashMap())); - - /** - * Expert: This method is called by {@code IndexReader}s which wrap other readers (e.g. {@link - * CompositeReader} or {@link FilterLeafReader}) to register the parent at the child (this reader) - * on construction of the parent. When this reader is closed, it will mark all registered parents - * as closed, too. The references to parent readers are weak only, so they can be GCed once they - * are no longer in use. - * - * @lucene.experimental - */ - public final void registerParentReader(IndexReader reader) { - ensureOpen(); - parentReaders.add(reader); - } - /** * For test framework use only. * @@ -173,18 +152,6 @@ protected void notifyReaderClosedListeners() throws IOException { // nothing to notify in the base impl } - private void reportCloseToParentReaders() throws IOException { - synchronized (parentReaders) { - for (IndexReader parent : parentReaders) { - parent.closedByChild = true; - // cross memory barrier by a fake write: - parent.refCount.addAndGet(0); - // recurse: - parent.reportCloseToParentReaders(); - } - } - } - /** Expert: returns the current refCount for this reader */ public final int getRefCount() { // NOTE: don't ensureOpen, so that callers can see @@ -253,8 +220,7 @@ public final void decRef() throws IOException { final int rc = refCount.decrementAndGet(); if (rc == 0) { closed = true; - try (Closeable _ = this::reportCloseToParentReaders; - Closeable _ = this::notifyReaderClosedListeners) { + try (Closeable _ = this::notifyReaderClosedListeners) { doClose(); } } else if (rc < 0) { diff --git a/lucene/core/src/java/org/apache/lucene/index/ParallelLeafReader.java b/lucene/core/src/java/org/apache/lucene/index/ParallelLeafReader.java index d73f111785a6..f96c5174f375 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ParallelLeafReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ParallelLeafReader.java @@ -196,7 +196,6 @@ public ParallelLeafReader( if (!closeSubReaders) { reader.incRef(); } - reader.registerParentReader(this); } } diff --git a/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java b/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java index ac23b9b52878..261e102c073e 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java @@ -105,9 +105,7 @@ public void testReaderChaining() throws Exception { if (ace == null) { throw new AssertionError("Query failed, but not due to an AlreadyClosedException", e); } - assertEquals( - "this IndexReader cannot be used anymore as one of its child readers was closed", - ace.getMessage()); + assertEquals("this IndexReader is closed", ace.getMessage()); } finally { // close executor: in case of wrap-wrap-wrapping searcher.getIndexReader().close(); From 6c743c75a921384058ad2bee4917288fc0622a7f Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 29 Jul 2025 16:54:09 -0400 Subject: [PATCH 2/4] test: remove testReaderChaining --- .../apache/lucene/index/TestReaderClosed.java | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java b/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java index 261e102c073e..ef96e8eef58d 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java @@ -81,37 +81,6 @@ public void test() throws Exception { } } - // LUCENE-3800 - public void testReaderChaining() throws Exception { - assertTrue(reader.getRefCount() > 0); - LeafReader wrappedReader = new ParallelLeafReader(getOnlyLeafReader(reader)); - - // We wrap with a OwnCacheKeyMultiReader so that closing the underlying reader - // does not terminate the threadpool (if that index searcher uses one) - IndexSearcher searcher = newSearcher(new OwnCacheKeyMultiReader(wrappedReader)); - - TermRangeQuery query = TermRangeQuery.newStringRange("field", "a", "z", true, true); - searcher.search(query, 5); - reader.close(); // close original child reader - try { - searcher.search(query, 5); - } catch (Exception e) { - AlreadyClosedException ace = null; - for (Throwable t = e; t != null; t = t.getCause()) { - if (t instanceof AlreadyClosedException) { - ace = (AlreadyClosedException) t; - } - } - if (ace == null) { - throw new AssertionError("Query failed, but not due to an AlreadyClosedException", e); - } - assertEquals("this IndexReader is closed", ace.getMessage()); - } finally { - // close executor: in case of wrap-wrap-wrapping - searcher.getIndexReader().close(); - } - } - @Override public void tearDown() throws Exception { dir.close(); From 7247370a4f9f3a758313676612ee119b9b661808 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 29 Jul 2025 16:56:23 -0400 Subject: [PATCH 3/4] CHANGES --- lucene/CHANGES.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 06353da69ba3..8c456413cd51 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -50,6 +50,10 @@ Improvements global queue min-score checking. Improves performance and consistency. (Mike Sokolov, Dzung Bui, Ben Trent) +* GITHUB#15002: Remove synchronized WeakHashMap from IndexReader. This was added to help prevent + SIGSEGV with the old unsafe "mmap hack", which has been replaced by MemorySegments. + (Uwe Schindler, Robert Muir) + Optimizations --------------------- * GITHUB#14011: Reduce allocation rate in HNSW concurrent merge. (Viliam Durina) From a8d3e02bbf812b6d005166c9a13bbab6c8c54c53 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 29 Jul 2025 18:31:46 -0400 Subject: [PATCH 4/4] tidy --- .../core/src/test/org/apache/lucene/index/TestReaderClosed.java | 1 - 1 file changed, 1 deletion(-) diff --git a/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java b/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java index ef96e8eef58d..694aefd4e734 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestReaderClosed.java @@ -25,7 +25,6 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.tests.analysis.MockAnalyzer; import org.apache.lucene.tests.analysis.MockTokenizer; -import org.apache.lucene.tests.index.OwnCacheKeyMultiReader; import org.apache.lucene.tests.index.RandomIndexWriter; import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.lucene.tests.util.TestUtil;