Fix thread safety, clean up Javadoc

Signed-off-by: Mark Chesney <[email protected]>
qos-ch · Jan 25, 2025 · e369c21 · e369c21
1 parent 5610c96
commit e369c21
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 65 deletions.
diff --git a/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java b/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java
@@ -1,6 +1,6 @@
 /**
  * Logback: the reliable, generic, fast and flexible logging framework.
- * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ * Copyright (C) 1999-2024, QOS.ch. All rights reserved.
  *
  * This program and the accompanying materials are dual-licensed under
  * either the terms of the Eclipse Public License v1.0 as published by
@@ -31,15 +31,15 @@ public class BasicStatusManager implements StatusManager {
     int count = 0;
 
     // protected access was requested in http://jira.qos.ch/browse/LBCORE-36
-    final protected List<Status> statusList = new ArrayList<Status>();
-    final protected CyclicBuffer<Status> tailBuffer = new CyclicBuffer<Status>(TAIL_SIZE);
-    final protected LogbackLock statusListLock = new LogbackLock();
+    protected final List<Status> statusList = new ArrayList<>();
+    protected final CyclicBuffer<Status> tailBuffer = new CyclicBuffer<>(TAIL_SIZE);
+    protected final LogbackLock statusListLock = new LogbackLock();
 
     int level = Status.INFO;
 
     // protected access was requested in http://jira.qos.ch/browse/LBCORE-36
-    final protected List<StatusListener> statusListenerList = new ArrayList<StatusListener>();
-    final protected LogbackLock statusListenerListLock = new LogbackLock();
+    protected final List<StatusListener> statusListenerList = new ArrayList<>();
+    protected final LogbackLock statusListenerListLock = new LogbackLock();
 
     // Note on synchronization
     // This class contains two separate locks statusListLock and
@@ -48,21 +48,17 @@ public class BasicStatusManager implements StatusManager {
     // without cycles. They are exposed to derived classes which should be careful
     // not to create deadlock cycles.
 
-    /**
-     * Add a new status object.
-     * 
-     * @param newStatus the status message to add
-     */
+    @Override
     public void add(Status newStatus) {
         // LBCORE-72: fire event before the count check
         fireStatusAddEvent(newStatus);
 
-        count++;
-        if (newStatus.getLevel() > level) {
-            level = newStatus.getLevel();
-        }
-
         synchronized (statusListLock) {
+            count++;
+            int newLevel = newStatus.getLevel();
+            if (newLevel > level) {
+                level = newLevel;
+            }
             if (statusList.size() < MAX_HEADER_COUNT) {
                 statusList.add(newStatus);
             } else {
@@ -72,9 +68,10 @@ public void add(Status newStatus) {
 
     }
 
+    @Override
     public List<Status> getCopyOfStatusList() {
         synchronized (statusListLock) {
-            List<Status> tList = new ArrayList<Status>(statusList);
+            List<Status> tList = new ArrayList<>(statusList);
             tList.addAll(tailBuffer.asList());
             return tList;
         }
@@ -88,28 +85,37 @@ private void fireStatusAddEvent(Status status) {
         }
     }
 
+    @Override
     public void clear() {
         synchronized (statusListLock) {
             count = 0;
+            level = Status.INFO;
             statusList.clear();
             tailBuffer.clear();
         }
     }
 
+    @Override
     public int getLevel() {
-        return level;
+        synchronized (statusListLock) {
+            return level;
+        }
     }
 
+    @Override
     public int getCount() {
-        return count;
+        synchronized (statusListLock) {
+            return count;
+        }
     }
 
     /**
+     * {@inheritDoc}
+     * <p>
      * This implementation does not allow duplicate installations of
-     * OnConsoleStatusListener
-     * 
-     * @param listener
+     * {@link OnConsoleStatusListener}.
      */
+    @Override
     public boolean add(StatusListener listener) {
         synchronized (statusListenerListLock) {
             if (listener instanceof OnConsoleStatusListener) {
@@ -130,15 +136,17 @@ private boolean checkForPresence(List<StatusListener> statusListenerList, Class<
         return false;
     }
 
+    @Override
     public void remove(StatusListener listener) {
         synchronized (statusListenerListLock) {
             statusListenerList.remove(listener);
         }
     }
 
+    @Override
     public List<StatusListener> getCopyOfStatusListenerList() {
         synchronized (statusListenerListLock) {
-            return new ArrayList<StatusListener>(statusListenerList);
+            return new ArrayList<>(statusListenerList);
         }
     }
 

diff --git a/logback-core/src/main/java/ch/qos/logback/core/status/StatusManager.java b/logback-core/src/main/java/ch/qos/logback/core/status/StatusManager.java
@@ -1,6 +1,6 @@
 /**
  * Logback: the reliable, generic, fast and flexible logging framework.
- * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ * Copyright (C) 1999-2024, QOS.ch. All rights reserved.
  *
  * This program and the accompanying materials are dual-licensed under
  * either the terms of the Eclipse Public License v1.0 as published by
@@ -16,71 +16,75 @@
 import java.util.List;
 
 /**
- * Internal error messages (statii) are managed by instances of this interface.
- * 
- * @author Ceki G&uuml;lc&uuml;
+ * A component which accepts status events and notifies registered listeners.
+ * Maintains event statistics, an event buffer, and a listener registry.
+ *
+ * @author Ceki Gülcü
+ * @author Mark Chesney
  */
 public interface StatusManager {
 
     /**
-     * Add a new status message.
-     * 
-     * @param status
+     * Notifies registered listeners of the specified status event and adds it to
+     * the end of the event buffer.
+     *
+     * @param status a status event
      */
     void add(Status status);
 
     /**
-     * Obtain a copy of the status list maintained by this StatusManager.
-     * 
-     * @return
+     * Returns a point-in-time snapshot of the event buffer.
+     *
+     * @return a snapshot of the event buffer
      */
     List<Status> getCopyOfStatusList();
 
     /**
-     * Return the highest level of all the statii.
-     * 
-     * @return
+     * Returns the highest level of statuses seen since instantiation.
+     *
+     * @return the highest level of statuses seen
      */
-    // int getLevel();
+    int getLevel();
 
     /**
-     * Return the number of status entries.
-     * 
-     * @return
+     * Returns the number of events processed since instantiation or the last reset.
+     *
+     * @return the number of events processed
      */
     int getCount();
 
     /**
-     * Add a status listener.
-     * 
-     * @param listener
-     */
-
-    /**
-     * Add a status listener. The StatusManager may decide to skip installation if
-     * an earlier instance was already installed.
-     * 
-     * @param listener
-     * @return true if actually added, false if skipped
+     * Registers the specified listener.
+     * <p>
+     * Returns {@code true} if the registered listeners changed, and {@code false}
+     * if the specified listener is already registered, and the implementation does
+     * not permit duplicates.
+     *
+     * @param listener the listener to register
+     * @return {@code true} if the registered listeners changed, {@code false}
+     *         otherwise
      */
     boolean add(StatusListener listener);
 
     /**
-     * ); Remove a status listener.
-     * 
-     * @param listener
+     * Deregisters the specified listener, if registered.
+     * <p>
+     * If the implementation permits duplicates, only the first occurrence is
+     * deregistered.
+     *
+     * @param listener the listener to deregister
      */
     void remove(StatusListener listener);
 
     /**
-     * Clear the list of status messages.
+     * Resets event statistics and empties the event buffer.
      */
     void clear();
 
     /**
-     * Obtain a copy of the status listener list maintained by this StatusManager
-     * 
-     * @return
+     * Returns a point-in-time snapshot of the registered listeners.
+     *
+     * @return a snapshot of the registered listeners
      */
     List<StatusListener> getCopyOfStatusListenerList();
 

diff --git a/logback-core/src/test/java/ch/qos/logback/core/net/mock/MockContext.java b/logback-core/src/test/java/ch/qos/logback/core/net/mock/MockContext.java
@@ -1,6 +1,6 @@
 /**
  * Logback: the reliable, generic, fast and flexible logging framework.
- * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ * Copyright (C) 1999-2024, QOS.ch. All rights reserved.
  *
  * This program and the accompanying materials are dual-licensed under
  * either the terms of the Eclipse Public License v1.0 as published by
@@ -15,8 +15,6 @@
 
 import java.util.List;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ThreadPoolExecutor;
 
 import ch.qos.logback.core.Context;
 import ch.qos.logback.core.ContextBase;
@@ -53,36 +51,44 @@ public Status getLastStatus() {
         return lastStatus;
     }
 
-    public void setLastStatus(Status lastStatus) {
-        this.lastStatus = lastStatus;
-    }
-
     private class MockStatusManager implements StatusManager {
 
+        @Override
         public void add(Status status) {
             lastStatus = status;
         }
 
+        @Override
         public List<Status> getCopyOfStatusList() {
             throw new UnsupportedOperationException();
         }
 
+        @Override
+        public int getLevel() {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
         public int getCount() {
             throw new UnsupportedOperationException();
         }
 
+        @Override
         public boolean add(StatusListener listener) {
             throw new UnsupportedOperationException();
         }
 
+        @Override
         public void remove(StatusListener listener) {
             throw new UnsupportedOperationException();
         }
 
+        @Override
         public void clear() {
             throw new UnsupportedOperationException();
         }
 
+        @Override
         public List<StatusListener> getCopyOfStatusListenerList() {
             throw new UnsupportedOperationException();
         }