Hi Ceki, some comments on the latest commit: noreply.ceki@qos.ch wrote:
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 @@
[..]
+ // 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>());
statusList should be final if it's later used for synchronization. At least that's what IDEA says...
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>());
As statusList, statusListenerList should be final.
+ + /** + * 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); + } }
I guess the method shouldn't be synchronized anymore since synchronization is done on statusList. [..]
+ public void remove(StatusListener listener) { + statusListenerList.add(listener); + }
Should be statusListenerList.remove(listener); Regards, Joern.