Index: logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java
===================================================================
--- logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java	(revision 1886)
+++ logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java	(working copy)
@@ -39,11 +39,13 @@
       throw new IllegalArgumentException("Null argument disallowed");
     }
     w.lock();
-    if (!appenderList.contains(newAppender)) {
-      appenderList.add(newAppender);
+    try {
+      if (!appenderList.contains(newAppender)) {
+        appenderList.add(newAppender);
+      }
+    } finally {
+      w.unlock();
     }
-    w.unlock();
-
   }
 
   /**
@@ -72,8 +74,11 @@
   public Iterator<Appender<E>> iteratorForAppenders() {
     List<Appender<E>> copy;
     r.lock();
-    copy = new ArrayList<Appender<E>>(appenderList);
-    r.unlock();
+    try {
+      copy = new ArrayList<Appender<E>>(appenderList);
+    } finally {
+      r.unlock();
+    }
     return copy.iterator();
   }
 
@@ -89,15 +94,18 @@
       return null;
     }
     r.lock();
-
-    for (Appender<E> appender : appenderList) {
-      if (name.equals(appender.getName())) {
-        r.unlock();
-        return appender;
+    Appender<E> found=null;
+    try {
+      for (Appender<E>appender : appenderList) {
+        if (name.equals(appender.getName())) {
+          found=appender;
+          break;
+        }
       }
+    } finally {
+      r.unlock();
     }
-    r.unlock();
-    return null;
+    return found;
   }
 
   /**
@@ -111,26 +119,30 @@
       return false;
     }
     r.lock();
-    for (Appender<E> a : appenderList) {
-      if (a == appender) {
-        r.unlock();
-        return true;
+    boolean attached=false;
+    try {
+      for (Appender<E> a : appenderList) {
+        if (a == appender) {
+          attached=true;
+          break;
+        }
       }
+    } finally {
+      r.unlock();
     }
-    r.unlock();
-    return false;
+    return attached;
   }
 
   /**
    * Remove and stop all previously attached appenders.
    */
   public void detachAndStopAllAppenders() {
+    w.lock();
     try {
-     w.lock();
       for (Appender<E> a : appenderList) {
-       a.stop();
-     }
-    appenderList.clear();
+        a.stop();
+      }
+      appenderList.clear();
     } finally {
       w.unlock();
     }
@@ -145,9 +157,13 @@
       return false;
     }
     w.lock();
-    boolean result = appenderList.remove(appender);
-    w.unlock();
-    return result;
+    boolean removed;
+    try {
+	    removed = appenderList.remove(appender);
+    } finally {
+      w.unlock();
+    }
+    return removed;
   }
 
   /**
@@ -159,13 +175,17 @@
       return false;
     }
     w.lock();
-    for (Appender<E> a : appenderList) {
-      if (name.equals((a).getName())) {
-        w.unlock();
-        return appenderList.remove(a);
+    boolean removed=false;
+    try {
+      for (Appender<E> a : appenderList) {
+        if (name.equals((a).getName())) {
+          removed=appenderList.remove(a);
+          break;
+        }
       }
+    } finally {
+      w.unlock();
     }
-    w.unlock();
-    return false;
+    return removed;
   }
 }