[JENKINS-68417] Revert LogRecord tricks

core/src/main/java/hudson/util/RingBufferLogHandler.java

@@ -24,42 +24,31 @@
 
 package hudson.util;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-import java.lang.ref.SoftReference;
+import hudson.remoting.ProxyException;
 import java.util.AbstractList;
 import java.util.List;
-import java.util.Objects;
+import java.util.logging.Formatter;
 import java.util.logging.Handler;
-import java.util.logging.Level;
 import java.util.logging.LogRecord;
+import java.util.logging.SimpleFormatter;
+import jenkins.util.JenkinsJVM;
 
 /**
  * Log {@link Handler} that stores the log records into a ring buffer.
  *
  * @author Kohsuke Kawaguchi
  */
-@SuppressFBWarnings(
-        value = "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT",
-        justification = "to guard against potential future compiler optimizations")
 public class RingBufferLogHandler extends Handler {
 
     private static final int DEFAULT_RING_BUFFER_SIZE = Integer.getInteger(RingBufferLogHandler.class.getName() + ".defaultSize", 256);
 
-    private static final class LogRecordRef extends SoftReference<LogRecord> {
-        LogRecordRef(LogRecord referent) {
-            super(referent);
-        }
-    }
-
-    static {
-        // Preload the LogRecordRef class to avoid an ABBA deadlock between agent class loading and logging.
-        LogRecord r = new LogRecord(Level.INFO, "<preloading>");
-        // We call Objects.hash() to guard against potential future compiler optimizations.
-        Objects.hash(new LogRecordRef(r).get());
-    }
+    /**
+     * Just to access {@link Formatter#formatMessage} which is not {@code static} though it could have been.
+     */
+    private static final Formatter dummyFormatter = new SimpleFormatter();
 
     private int start = 0;
-    private final LogRecordRef[] records;
+    private final LogRecord[] records;
     private int size;
 
     /**
@@ -73,7 +62,7 @@ public RingBufferLogHandler() {
     }
 
     public RingBufferLogHandler(int ringSize) {
-        records = new LogRecordRef[ringSize];
+        records = new LogRecord[ringSize];
     }
 
     /**
@@ -86,13 +75,36 @@ public static int getDefaultRingBufferSize() {
     }
 
     @Override
-    public synchronized void publish(LogRecord record) {
-        int len = records.length;
-        records[(start + size) % len] = new LogRecordRef(record);
-        if (size == len) {
-            start = (start + 1) % len;
-        } else {
-            size++;
+    public void publish(LogRecord record) {
+        if (record == null) {
+            return;
+        }
+        if (JenkinsJVM.isJenkinsJVM() && record.getParameters() != null) {
+            try {
+                LogRecord clone = new LogRecord(record.getLevel(), dummyFormatter.formatMessage(record));
+                clone.setLoggerName(record.getLoggerName());
+                clone.setMillis(record.getMillis());
+                clone.setSequenceNumber(record.getSequenceNumber());
+                clone.setSourceClassName(record.getSourceClassName());
+                clone.setSourceMethodName(record.getSourceMethodName());
+                clone.setThreadID(record.getThreadID());
+                Throwable t = record.getThrown();
+                if (t != null) {
+                    clone.setThrown(new ProxyException(t));
+                }
+                record = clone;
+            } catch (Exception e) {
+                e.printStackTrace();
+            }
+        }
+        synchronized (this) {
+            int len = records.length;
+            records[(start + size) % len] = record;
+            if (size == len) {
+                start = (start + 1) % len;
+            } else {
+                size++;
+            }
         }
     }
 
@@ -114,9 +126,7 @@ public List<LogRecord> getView() {
             public LogRecord get(int index) {
                 // flip the order
                 synchronized (RingBufferLogHandler.this) {
-                    LogRecord r = records[(start + (size - (index + 1))) % records.length].get();
-                    // We cannot just omit collected entries due to the List interface.
-                    return r != null ? r : new LogRecord(Level.OFF, "<discarded>");
+                    return records[(start + (size - (index + 1))) % records.length];
                 }
             }
 

test/src/test/java/hudson/util/RingBufferLogHandlerTest.java

@@ -24,6 +24,8 @@
 
 package hudson.util;
 
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
 
@@ -33,10 +35,10 @@ public class RingBufferLogHandlerTest {
     @Issue("JENKINS-9120")
     public void tooMuchRecordsShouldNotCrashHandler() {
         final RingBufferLogHandler handler = new RingBufferLogHandler();
-
+        LogRecord lr = new LogRecord(Level.INFO, "xxx");
         for (long i = 0; i < (long) Integer.MAX_VALUE + 300; i++) {
             // throws ArrayIndexOutOfBoundsException after int-overflow
-            handler.publish(null);
+            handler.publish(lr);
         }
     }
 }

test/src/test/java/jenkins/model/JenkinsLogRecordsTest.java

@@ -37,14 +37,19 @@ private static void _logRecordsArePresentOnController(JenkinsRule r) throws Thro
             containsInRelativeOrder("Completed initialization", "Started initialization"));
         VersionNumber javaVersion = new VersionNumber(System.getProperty("java.specification.version"));
         if (javaVersion.isOlderThan(new VersionNumber("9")) || javaVersion.isNewerThanOrEqualTo(new VersionNumber("17"))) { // TODO https://github.com/jenkinsci/jenkins-test-harness/issues/359
-            LogRecord lr = new LogRecord(Level.INFO, "collect me");
-            Logger.getLogger(Jenkins.class.getName()).log(lr);
-            WeakReference<LogRecord> ref = new WeakReference<>(lr);
-            lr = null;
+            Object x = new Object() {
+                @Override
+                public String toString() {
+                    return "collect me";
+                }
+            };
+            Logger.getLogger(Jenkins.class.getName()).log(Level.INFO, "formatting {0}", x);
+            WeakReference<Object> ref = new WeakReference<>(x);
+            x = null;
             MemoryAssert.assertGC(ref, true);
-            assertThat("Records collected",
+            assertThat("Record parameters formatted before collection",
                 logRecords.stream().map(LogRecord::getMessage).collect(Collectors.toList()),
-                hasItem("<discarded>"));
+                hasItem("formatting collect me"));
         }
     }
 }