From 098f0dc8b414ca9c9a47cf7ebf4383198fb946fe Mon Sep 17 00:00:00 2001
From: Simon Willnauer <simonw@apache.org>
Date: Mon, 24 Aug 2020 21:29:27 +0200
Subject: [PATCH] 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 +++++++
 .../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 7280810d2a2..e2c646c6121 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -221,6 +221,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 92e7be7d359..cb3db7a0ff7 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 8a1f30cae9a..41909bc2af4 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;