Thanks! I'll check out trunk, take a look and then pass it on to test with our Appender. See comments below. noreply.ceki@qos.ch wrote:
Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java ============================================================================== --- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java (original) +++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java Mon Oct 20 20:51:02 2008
The iterator is guaranteed to be consistent through the for loop. However, unless there is other locking preventing it, an appender can be added while detachAndStopAllAppenders is running. Doing appenderList.clear() outside of the loop will remove the newly added appender without stopping it. In the version I sent you I had appenderList.remove(a); in the loop for that reason. If you accounted for this some other way can you add a comment describing it?
@@ -118,13 +97,9 @@ * Remove and stop all previously attached appenders. */ public void detachAndStopAllAppenders() { - int len = appenderList.size(); - - for (int i = 0; i < len; i++) { - Appender a = (Appender) appenderList.get(i); + for (Appender<E> a : appenderList) { a.stop(); } - appenderList.clear(); }
Added: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/AppenderAttachableImplTest.java ============================================================================== --- (empty file) +++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/AppenderAttachableImplTest.java Mon Oct 20 20:51:02 2008 @@ -0,0 +1,177 @@ +/** + * Logback: the generic, reliable, fast and flexible logging framework. + * + * Copyright (C) 2000-2008, QOS.ch + * + * This library is free software, you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation. + */ +package ch.qos.logback.core.spi; + + +import static org.junit.Assert.*; +import ch.qos.logback.core.appender.NOPAppender; +import ch.qos.logback.core.Appender; +import ch.qos.logback.core.layout.NopLayout; + +import java.util.Iterator; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * This test case verifies all the methods of AppenderAttableImpl work properly. + * + * @author Raplh Goers
Did I misspell my own name? (I do that a lot).