(solr) branch main updated: SOLR-17063: Do not retain log param references in LogWatcher (#2058)

[ma...@apache.org]

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

magibney pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 6b74f86fc46 SOLR-17063: Do not retain log param references in LogWatcher (#2058)
6b74f86fc46 is described below

commit 6b74f86fc46cbac8e5e4278597de9e3a7334849c
Author: Michael Gibney <mi...@michaelgibney.net>
AuthorDate: Fri Dec 8 16:21:09 2023 -0500

    SOLR-17063: Do not retain log param references in LogWatcher (#2058)
---
 solr/CHANGES.txt                                   |  2 ++
 .../java/org/apache/solr/logging/LogWatcher.java   | 24 ++++++++++++++--------
 .../org/apache/solr/logging/jul/JulWatcher.java    |  5 -----
 .../apache/solr/logging/log4j2/Log4j2Watcher.java  |  7 +------
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 68d6924b608..e0896a29a28 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -123,6 +123,8 @@ Improvements
 
 * SOLR-16577: Ensure core load failures are always logged. (Haythem Khiri, David Smiley)
 
+* SOLR-17063: Do not retain log param references in LogWatcher (Michael Gibney)
+
 Optimizations
 ---------------------
 * SOLR-17084: LBSolrClient (used by CloudSolrClient) now returns the count of core tracked as not live AKA zombies
diff --git a/solr/core/src/java/org/apache/solr/logging/LogWatcher.java b/solr/core/src/java/org/apache/solr/logging/LogWatcher.java
index e023d99d0c6..86ca1b86dbd 100644
--- a/solr/core/src/java/org/apache/solr/logging/LogWatcher.java
+++ b/solr/core/src/java/org/apache/solr/logging/LogWatcher.java
@@ -18,6 +18,8 @@ package org.apache.solr.logging;
 
 import java.lang.invoke.MethodHandles;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -38,7 +40,7 @@ public abstract class LogWatcher<E> {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  protected CircularList<E> history;
+  protected CircularList<SolrDocument> history;
   protected long last = -1;
 
   /**
@@ -64,10 +66,18 @@ public abstract class LogWatcher<E> {
   public abstract String getThreshold();
 
   public void add(E event, long timstamp) {
-    history.add(event);
+    history.add(unmodifiable(toSolrDocument(event)));
     last = timstamp;
   }
 
+  /**
+   * Since we store the derivative {@link SolrDocument} instances and potentially return the same
+   * instance to multiple callers, we should guard against modification.
+   */
+  private static SolrDocument unmodifiable(SolrDocument doc) {
+    return new SolrDocument(Collections.unmodifiableMap(doc.getFieldValueMap()));
+  }
+
   public long getLastEvent() {
     return last;
   }
@@ -82,25 +92,23 @@ public abstract class LogWatcher<E> {
     }
 
     SolrDocumentList docs = new SolrDocumentList();
-    Iterator<E> iter = history.iterator();
+    Iterator<SolrDocument> iter = history.iterator();
     while (iter.hasNext()) {
-      E e = iter.next();
-      long ts = getTimestamp(e);
+      SolrDocument logEventEntry = iter.next();
+      long ts = ((Date) logEventEntry.getFirstValue("time")).getTime();
       if (ts == since) {
         if (found != null) {
           found.set(true);
         }
       }
       if (ts > since) {
-        docs.add(toSolrDocument(e));
+        docs.add(logEventEntry);
       }
     }
     docs.setNumFound(docs.size()); // make it not look too funny
     return docs;
   }
 
-  public abstract long getTimestamp(E event);
-
   public abstract SolrDocument toSolrDocument(E event);
 
   public abstract void registerListener(ListenerConfig cfg);
diff --git a/solr/core/src/java/org/apache/solr/logging/jul/JulWatcher.java b/solr/core/src/java/org/apache/solr/logging/jul/JulWatcher.java
index c025ef59df0..271e1358fd1 100644
--- a/solr/core/src/java/org/apache/solr/logging/jul/JulWatcher.java
+++ b/solr/core/src/java/org/apache/solr/logging/jul/JulWatcher.java
@@ -144,11 +144,6 @@ public class JulWatcher extends LogWatcher<LogRecord> {
     log.addHandler(handler);
   }
 
-  @Override
-  public long getTimestamp(LogRecord event) {
-    return event.getMillis();
-  }
-
   @Override
   public SolrDocument toSolrDocument(LogRecord event) {
     SolrDocument doc = new SolrDocument();
diff --git a/solr/core/src/java/org/apache/solr/logging/log4j2/Log4j2Watcher.java b/solr/core/src/java/org/apache/solr/logging/log4j2/Log4j2Watcher.java
index e5ecc60bccf..9e266baa5ad 100644
--- a/solr/core/src/java/org/apache/solr/logging/log4j2/Log4j2Watcher.java
+++ b/solr/core/src/java/org/apache/solr/logging/log4j2/Log4j2Watcher.java
@@ -261,7 +261,7 @@ public class Log4j2Watcher extends LogWatcher<LogEvent> {
   public void registerListener(ListenerConfig cfg) {
     if (history != null) throw new IllegalStateException("History already registered");
 
-    history = new CircularList<LogEvent>(cfg.size);
+    history = new CircularList<>(cfg.size);
 
     Level threshold = (cfg.threshold != null) ? Level.toLevel(cfg.threshold) : Level.WARN;
     ThresholdFilter filter =
@@ -281,11 +281,6 @@ public class Log4j2Watcher extends LogWatcher<LogEvent> {
     ctx.updateLoggers();
   }
 
-  @Override
-  public long getTimestamp(LogEvent event) {
-    return event.getTimeMillis();
-  }
-
   @Override
   public SolrDocument toSolrDocument(LogEvent event) {
     SolrDocument doc = new SolrDocument();