[logback-dev] svn commit: r1733 - in logback/trunk: logback-classic/src/main/java/ch/qos/logback/classic/jmx logback-core/src/main/java/ch/qos/logback/core logback-core/src/main/java/ch/qos/logback/core/status logback-core/src/main/java/ch/qos/logback/core/util logback-core/src/test/java/ch/qos/logback/core/joran/action logback-core/src/test/java/ch/qos/logback/core/status

noreply.ceki at qos.ch noreply.ceki at qos.ch
Wed Aug 6 18:51:10 CEST 2008


Author: ceki
Date: Wed Aug  6 18:51:10 2008
New Revision: 1733

Added:
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/status/StatusListener.java
Modified:
   logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/jmx/Configurator.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/status/StatusManager.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/StatusPrinter.java
   logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/PropertyActionTest.java
   logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java

Log:
Relates to LBCLASSIC-51 LBCLASSIC-59 LBCLASSIC-58

Fixed synchronization problems. The iterator() method has been removed from 
the StatusManager interface. It has been replaced by getCopyOfStatusList
which, as the name indicates, returns a copy of the statusList contained in
the StatusManager instance. This avoids concurrency problems encountered
when the developer forget to synchronize on the statusManager before iterating
on its status list. 

Added a listener interface to status manager as suggested in LBCLASSIC-59 
by Anton Tagunov. 

Support for status listeners in Joran is still missing.

Modified: logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/jmx/Configurator.java
==============================================================================
--- logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/jmx/Configurator.java	(original)
+++ logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/jmx/Configurator.java	Wed Aug  6 18:51:10 2008
@@ -143,7 +143,7 @@
   
   public List<String> getStatuses() {
     List<String> list = new ArrayList<String>();
-    Iterator<Status> it = context.getStatusManager().iterator();
+    Iterator<Status> it = context.getStatusManager().getCopyOfStatusList().iterator();
     while(it.hasNext()) {
       list.add(it.next().toString());
     }

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/BasicStatusManager.java	Wed Aug  6 18:51:10 2008
@@ -10,25 +10,39 @@
 package ch.qos.logback.core;
 
 import java.util.ArrayList;
-import java.util.Iterator;
+import java.util.Collections;
 import java.util.List;
 
 import ch.qos.logback.core.status.Status;
+import ch.qos.logback.core.status.StatusListener;
 import ch.qos.logback.core.status.StatusManager;
 
 public class BasicStatusManager implements StatusManager {
 
   public static final int MAX_COUNT = 200;
 
+  // This method is synchronized on the instance.
+  // Code
   int count = 0;
-  List<Status> statusList = new ArrayList<Status>();
+  
+  // reading SynchronizedCollection the mutex is the returned 
+  // synchronized list, we make use of this fact in getCopyOfStatusList
+  List<Status> statusList = Collections
+      .synchronizedList(new ArrayList<Status>());
   int level = Status.INFO;
 
-  // This method is synchronized on the instance.
-  // Code iterating on this.iterator is expected to
-  // also synchronize on this (the BasicStatusManager instance)
-  public synchronized void add(Status newStatus) {
-    // System.out.println(newStatus);
+  // reading SynchronizedCollection the mutex is the returned 
+  // synchronized list, we make use of this fact in getCopyOfStatusListnerList
+  List<StatusListener> statusListenerList = Collections
+      .synchronizedList(new ArrayList<StatusListener>());
+
+  /**
+   * Add a new status object.
+   * 
+   * @param Status
+   *                the status message to add
+   */
+  public void add(Status newStatus) {
     if (count > MAX_COUNT) {
       return;
     }
@@ -40,8 +54,10 @@
     statusList.add(newStatus);
   }
 
-  public Iterator<Status> iterator() {
-    return statusList.iterator();
+  public synchronized List<Status> getCopyOfStatusList() {
+    synchronized (statusList) {
+      return new ArrayList<Status>(statusList);
+    }
   }
 
   public int getLevel() {
@@ -52,4 +68,18 @@
     return count;
   }
 
+  public void add(StatusListener listener) {
+    statusListenerList.add(listener);
+  }
+
+  public void remove(StatusListener listener) {
+    statusListenerList.add(listener);
+  }
+
+  public List<StatusListener> getCopyOfStatusListenerList() {
+    synchronized (statusListenerList) {
+      return new ArrayList<StatusListener>(statusListenerList);
+    }
+  }
+
 }

Added: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/status/StatusListener.java
==============================================================================
--- (empty file)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/status/StatusListener.java	Wed Aug  6 18:51:10 2008
@@ -0,0 +1,5 @@
+package ch.qos.logback.core.status;
+
+public interface StatusListener {
+  void addStatusEvent(Status status);
+}

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/status/StatusManager.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/status/StatusManager.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/status/StatusManager.java	Wed Aug  6 18:51:10 2008
@@ -9,16 +9,52 @@
  */
 package ch.qos.logback.core.status;
 
-import java.util.Iterator;
-
+import java.util.List;
 
+/**
+ * Internal error messages (statii) are managed by instances of this interface.
+ * 
+ * @author Ceki Gulcu
+ */
 public interface StatusManager {
+
+  /**
+   * Add a new status message.
+   * 
+   * @param status
+   */
   public void add(Status status);
-  public Iterator<Status> iterator();
+
+  /**
+   * Obtain a copy of the status list maintained by this StatusManager.
+   * 
+   * @return
+   */
+  public List<Status> getCopyOfStatusList();
+
+  /**
+   * Return the highest level of all the statii.
+   * 
+   * @return
+   */
   public int getLevel();
+
   /**
-   * Return the number of entries.
+   * Return the number of status entries.
+   * 
    * @return
    */
   public int getCount();
+
+  public void add(StatusListener listener);
+
+  public void remove(StatusListener listener);
+
+  /**
+   * Obtain a copy of the status listener list maintained by this StatusManager
+   * 
+   * @return
+   */
+  public List<StatusListener> getCopyOfStatusListenerList();
+
 }

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/StatusPrinter.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/StatusPrinter.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/StatusPrinter.java	Wed Aug  6 18:51:10 2008
@@ -55,13 +55,10 @@
   }
 
   public static void buildStr(StringBuilder sb, StatusManager sm) {
-
-    synchronized (sm) {
-      Iterator it = sm.iterator();
-      while (it.hasNext()) {
-        Status s = (Status) it.next();
-        buildStr(sb, "", s);
-      }
+    Iterator it = sm.getCopyOfStatusList().iterator();
+    while (it.hasNext()) {
+      Status s = (Status) it.next();
+      buildStr(sb, "", s);
     }
   }
 

Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/PropertyActionTest.java
==============================================================================
--- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/PropertyActionTest.java	(original)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/PropertyActionTest.java	Wed Aug  6 18:51:10 2008
@@ -83,13 +83,13 @@
   }
   
   private boolean checkError() {
-    Iterator it = context.getStatusManager().iterator();
+    Iterator it = context.getStatusManager().getCopyOfStatusList().iterator();
     ErrorStatus es = (ErrorStatus)it.next();
     return PropertyAction.INVALID_ATTRIBUTES.equals(es.getMessage());
   }
   
   private boolean checkFileErrors() {
-    Iterator it = context.getStatusManager().iterator();
+    Iterator it = context.getStatusManager().getCopyOfStatusList().iterator();
     ErrorStatus es1 = (ErrorStatus)it.next();
     boolean result1 = "Could not read properties file [toto].".equals(es1.getMessage());
     ErrorStatus es2 = (ErrorStatus)it.next();

Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java
==============================================================================
--- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java	(original)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/status/StatusChecker.java	Wed Aug  6 18:51:10 2008
@@ -16,42 +16,39 @@
 import ch.qos.logback.core.status.Status;
 import ch.qos.logback.core.status.StatusManager;
 
-
 public class StatusChecker {
 
   StatusManager sm;
-  
+
   public StatusChecker(StatusManager sm) {
     this.sm = sm;
   }
-  
+
   public boolean containsMatch(String regex) {
-    
+
     Pattern p = Pattern.compile(regex);
- 
-    
-    Iterator stati = sm.iterator();
-    while(stati.hasNext()) {
+
+    Iterator stati = sm.getCopyOfStatusList().iterator();
+    while (stati.hasNext()) {
       Status status = (Status) stati.next();
       String msg = status.getMessage();
       Matcher matcher = p.matcher(msg);
-      if(matcher.lookingAt()) {
+      if (matcher.lookingAt()) {
         return true;
       } else {
-        System.out.println("no match:"+msg);
-        System.out.println("regex   :"+regex);
+        System.out.println("no match:" + msg);
+        System.out.println("regex   :" + regex);
       }
     }
     return false;
   }
-  
-  
+
   public boolean containsException(Class exceptionType) {
-    Iterator stati = sm.iterator();
-    while(stati.hasNext()) {
+    Iterator stati = sm.getCopyOfStatusList().iterator();
+    while (stati.hasNext()) {
       Status status = (Status) stati.next();
       Throwable t = status.getThrowable();
-      if(t != null && t.getClass().getName().equals(exceptionType.getName())) {
+      if (t != null && t.getClass().getName().equals(exceptionType.getName())) {
         return true;
       }
     }


More information about the logback-dev mailing list