[lucene-solr] branch branch_8x updated: LUCENE-9478: Prevent DWPTDeleteQueue from referencing itself and leaking memory (#1779)

[si...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

simonw pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 81e405f  LUCENE-9478: Prevent DWPTDeleteQueue from referencing itself and leaking memory (#1779)
81e405f is described below

commit 81e405f1c5242bcc3c0fb6a0c8a55b5cfa5889af
Author: Simon Willnauer <si...@apache.org>
AuthorDate: Mon Aug 24 21:29:27 2020 +0200

    LUCENE-9478: Prevent DWPTDeleteQueue from referencing itself and leaking memory (#1779)
    
    In LUCENE-9304 we introduced some fixes that unfortunately hold on to the previous
    DWPTDeleteQueue which is essentially leaking IW memory and cause applications to fail.
    This fixes the memory leak and adds a test to ensure its not leaking memory.
---
 lucene/CHANGES.txt                                  |  8 ++++++++
 .../lucene/index/DocumentsWriterDeleteQueue.java    |  8 +++++++-
 .../index/TestDocumentsWriterDeleteQueue.java       | 21 +++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 064d1a5..007fe9d 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -66,6 +66,14 @@ Other
 ---------------------
 (No changes)
 
+======================= Lucene 8.6.2 =======================
+
+Bug Fixes
+---------------------
+* LUCENE-9478: Prevent DWPTDeleteQueue from referencing itself and leaking memory. The queue
+  passed an implicit this reference to the next queue instance on flush which leaked about 500byte
+  of memory on each full flush, commit or getReader call. (Simon Willnauer)
+
 ======================= Lucene 8.6.1 =======================
 
 Bug Fixes
diff --git a/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterDeleteQueue.java b/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterDeleteQueue.java
index 92e7be7..cb3db7a 100644
--- a/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterDeleteQueue.java
+++ b/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterDeleteQueue.java
@@ -573,6 +573,12 @@ final class DocumentsWriterDeleteQueue implements Accountable, Closeable {
     }
   }
 
+  // we use a static method to get this lambda since we previously introduced a memory leak since it would
+  // implicitly reference this.nextSeqNo which holds on to this del queue. see LUCENE-9478 for reference
+  private static LongSupplier getPrevMaxSeqIdSupplier(AtomicLong nextSeqNo) {
+    return () -> nextSeqNo.get() - 1;
+  }
+
   /**
    * Advances the queue to the next queue on flush. This carries over the the generation to the next queue and
    * set the {@link #getMaxSeqNo()} based on the given maxNumPendingOps. This method can only be called once, subsequently
@@ -593,7 +599,7 @@ final class DocumentsWriterDeleteQueue implements Accountable, Closeable {
     return new DocumentsWriterDeleteQueue(infoStream, generation + 1, seqNo + 1,
         // don't pass ::getMaxCompletedSeqNo here b/c otherwise we keep an reference to this queue
         // and this will be a memory leak since the queues can't be GCed
-        () -> nextSeqNo.get() - 1);
+        getPrevMaxSeqIdSupplier(nextSeqNo));
 
   }
 
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestDocumentsWriterDeleteQueue.java b/lucene/core/src/test/org/apache/lucene/index/TestDocumentsWriterDeleteQueue.java
index 8a1f30c..41909bc 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestDocumentsWriterDeleteQueue.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestDocumentsWriterDeleteQueue.java
@@ -16,6 +16,7 @@
  */
 package org.apache.lucene.index;
 
+import java.lang.ref.WeakReference;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
@@ -36,6 +37,26 @@ import org.apache.lucene.util.ThreadInterruptedException;
  */
 public class TestDocumentsWriterDeleteQueue extends LuceneTestCase {
 
+
+  public void testAdvanceReferencesOriginal() {
+    WeakAndNext weakAndNext = new WeakAndNext();
+    DocumentsWriterDeleteQueue next = weakAndNext.next;
+    assertNotNull(next);
+    System.gc();
+    assertNull(weakAndNext.weak.get());
+  }
+  class WeakAndNext {
+    final WeakReference<DocumentsWriterDeleteQueue> weak;
+    final DocumentsWriterDeleteQueue next;
+
+    WeakAndNext() {
+      DocumentsWriterDeleteQueue deleteQueue = new DocumentsWriterDeleteQueue(null);
+      weak = new WeakReference<>(deleteQueue);
+      next = deleteQueue.advanceQueue(2);
+    }
+  }
+
+
   public void testUpdateDelteSlices() throws Exception {
     DocumentsWriterDeleteQueue queue = new DocumentsWriterDeleteQueue(null);
     final int size = 200 + random().nextInt(500) * RANDOM_MULTIPLIER;