[logback-dev] [GIT] Logback: the generic, reliable, fast and flexible logging framework. branch, master, updated. v_0.9.28-21-ge068bd3

added by portage for gitosis-gentoo git-noreply at pixie.qos.ch
Tue Mar 8 22:37:05 CET 2011


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Logback: the generic, reliable, fast and flexible logging framework.".

The branch, master has been updated
       via  e068bd3b9754e8d62e4ca799480bc3a6c7e14683 (commit)
      from  0cc65fce0f69854255d2798d93c7ff11a08be0ce (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://git.qos.ch/gitweb/?p=logback.git;a=commit;h=e068bd3b9754e8d62e4ca799480bc3a6c7e14683
http://github.com/ceki/logback/commit/e068bd3b9754e8d62e4ca799480bc3a6c7e14683

commit e068bd3b9754e8d62e4ca799480bc3a6c7e14683
Author: Ceki Gulcu <ceki at qos.ch>
Date:   Tue Mar 8 22:17:10 2011 +0100

    - refactoring
    - avoid clearing the old map reference which may be still referenced elsewhere

diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/util/LogbackMDCAdapter.java b/logback-classic/src/main/java/ch/qos/logback/classic/util/LogbackMDCAdapter.java
index d8e5a80..b8ffb82 100644
--- a/logback-classic/src/main/java/ch/qos/logback/classic/util/LogbackMDCAdapter.java
+++ b/logback-classic/src/main/java/ch/qos/logback/classic/util/LogbackMDCAdapter.java
@@ -35,7 +35,7 @@ import org.slf4j.spi.MDCAdapter;
  *
  * @author Ceki G&uuml;lc&uuml;
  */
-public class LogbackMDCAdapter implements MDCAdapter {
+public final class LogbackMDCAdapter implements MDCAdapter {
 
 
   // We no longer use CopyOnInheritThreadLocal in order to solve LBCLASSIC-183
@@ -47,19 +47,31 @@ public class LogbackMDCAdapter implements MDCAdapter {
   private static final int WRITE_OPERATION = 1;
   private static final int READ_OPERATION = 2;
 
+  // keeps track of the last operation performed
   final ThreadLocal<Integer> lastOperation = new ThreadLocal<Integer>();
 
-
-
   public LogbackMDCAdapter() {
   }
 
   private Integer getAndSetLastOperation(int op) {
     Integer lastOp = lastOperation.get();
-    lastOperation.set(WRITE_OPERATION);
+    lastOperation.set(op);
     return lastOp;
   }
 
+  private boolean wasLastOpReadOrNull(Integer lastOp) {
+    return lastOp == null || lastOp.intValue() == READ_OPERATION;
+  }
+
+  private HashMap<String, String> duplicateAndInsertNewMap(HashMap<String, String> oldMap) {
+    HashMap<String, String> newMap = new HashMap<String, String>();
+    if (oldMap != null)
+      newMap.putAll(oldMap);
+    // the newMap replaces the old one for serialisation's sake
+    copyOnInheritThreadLocal.set(newMap);
+    return newMap;
+  }
+
   /**
    * Put a context value (the <code>val</code> parameter) as identified with the
    * <code>key</code> parameter into the current thread's context map. Note that
@@ -68,12 +80,6 @@ public class LogbackMDCAdapter implements MDCAdapter {
    * <p/>
    * If the current thread does not have a context map it is created as a side
    * effect of this call.
-   * <p/>
-   * <p/>
-   * Each time a value is added, a new instance of the map is created. This is
-   * to be certain that the serialization process will operate on the updated
-   * map and not send a reference to the old map, thus not allowing the remote
-   * logback component to see the latest changes.
    *
    * @throws IllegalArgumentException in case the "key" parameter is null
    */
@@ -85,13 +91,8 @@ public class LogbackMDCAdapter implements MDCAdapter {
     HashMap<String, String> oldMap = copyOnInheritThreadLocal.get();
     Integer lastOp = getAndSetLastOperation(WRITE_OPERATION);
 
-    if(lastOp == null || lastOp.intValue() == READ_OPERATION || oldMap ==  null) {
-      HashMap<String, String> newMap = new HashMap<String, String>();
-      if (oldMap != null) {
-        newMap.putAll(oldMap);
-      }
-      // the newMap replaces the old one for serialisation's sake
-      copyOnInheritThreadLocal.set(newMap);
+    if (wasLastOpReadOrNull(lastOp) || oldMap == null) {
+      HashMap<String, String> newMap = duplicateAndInsertNewMap(oldMap);
       newMap.put(key, val);
     } else {
       oldMap.put(key, val);
@@ -99,61 +100,49 @@ public class LogbackMDCAdapter implements MDCAdapter {
   }
 
   /**
-   * Get the context identified by the <code>key</code> parameter.
-   * <p/>
-   * <p/>
-   * This method has no side effects.
-   */
-  public String get(String key) {
-    HashMap<String, String> hashMap = copyOnInheritThreadLocal.get();
-
-    if ((hashMap != null) && (key != null)) {
-      return hashMap.get(key);
-    } else {
-      return null;
-    }
-  }
-
-
-  /**
    * Remove the the context identified by the <code>key</code> parameter.
    * <p/>
-   * <p/>
-   * Each time a value is removed, a new instance of the map is created. This is
-   * to be certain that the serialization process will operate on the updated
-   * map and not send a reference to the old map, thus not allowing the remote
-   * logback component to see the latest changes.
    */
   public void remove(String key) {
     if (key == null) {
       return;
     }
     HashMap<String, String> oldMap = copyOnInheritThreadLocal.get();
-    if(oldMap == null) return;
+    if (oldMap == null) return;
 
     Integer lastOp = getAndSetLastOperation(WRITE_OPERATION);
 
-    if(lastOp == null || lastOp.intValue() == READ_OPERATION) {
-      HashMap<String, String> newMap = new HashMap<String, String>();
-      newMap.putAll(oldMap);
-      // the newMap replaces the old one for serialisation's sake
-      copyOnInheritThreadLocal.set(newMap);
+    if (wasLastOpReadOrNull(lastOp)) {
+      HashMap<String, String> newMap = duplicateAndInsertNewMap(oldMap);
       newMap.remove(key);
     } else {
       oldMap.remove(key);
     }
   }
 
+
   /**
    * Clear all entries in the MDC.
    */
   public void clear() {
-    HashMap<String, String> hashMap = copyOnInheritThreadLocal.get();
-    Integer lastOp = getAndSetLastOperation(WRITE_OPERATION);
+    lastOperation.set(WRITE_OPERATION);
     copyOnInheritThreadLocal.remove();
   }
 
   /**
+   * Get the context identified by the <code>key</code> parameter.
+   * <p/>
+   */
+  public String get(String key) {
+    Map<String, String> map = getPropertyMap();
+    if ((map != null) && (key != null)) {
+      return map.get(key);
+    } else {
+      return null;
+    }
+  }
+
+  /**
    * Get the current thread's MDC as a map. This method is intended to be used
    * internally.
    */
@@ -163,47 +152,40 @@ public class LogbackMDCAdapter implements MDCAdapter {
   }
 
   /**
-   * Return a copy of the current thread's context map. Returned value may be
+   * Returns the keys in the MDC as a {@link Set}. The returned value can be
    * null.
    */
-  public Map getCopyOfContextMap() {
-    HashMap<String, String> hashMap = copyOnInheritThreadLocal.get();
-    if (hashMap == null) {
-      return null;
+  public Set<String> getKeys() {
+    Map<String, String> map = getPropertyMap();
+
+    if (map != null) {
+      return map.keySet();
     } else {
-      return new HashMap<String, String>(hashMap);
+      return null;
     }
   }
 
   /**
-   * Returns the keys in the MDC as a {@link Set}. The returned value can be
+   * Return a copy of the current thread's context map. Returned value may be
    * null.
    */
-  public Set<String> getKeys() {
-    lastOperation.set(READ_OPERATION);
+  public Map getCopyOfContextMap() {
     HashMap<String, String> hashMap = copyOnInheritThreadLocal.get();
-
-    if (hashMap != null) {
-      return hashMap.keySet();
-    } else {
+    if (hashMap == null) {
       return null;
+    } else {
+      return new HashMap<String, String>(hashMap);
     }
   }
 
   @SuppressWarnings("unchecked")
   public void setContextMap(Map contextMap) {
-    HashMap<String, String> oldMap = copyOnInheritThreadLocal.get();
+    lastOperation.set(WRITE_OPERATION);
 
     HashMap<String, String> newMap = new HashMap<String, String>();
     newMap.putAll(contextMap);
 
     // the newMap replaces the old one for serialisation's sake
     copyOnInheritThreadLocal.set(newMap);
-
-    // hints for the garbage collector
-    if (oldMap != null) {
-      oldMap.clear();
-      oldMap = null;
-    }
   }
 }
diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/util/LogbackMDCAdapterTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/util/LogbackMDCAdapterTest.java
index 767b490..7799af7 100644
--- a/logback-classic/src/test/java/ch/qos/logback/classic/util/LogbackMDCAdapterTest.java
+++ b/logback-classic/src/test/java/ch/qos/logback/classic/util/LogbackMDCAdapterTest.java
@@ -19,6 +19,7 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 
 import org.junit.Test;
@@ -61,11 +62,34 @@ public class LogbackMDCAdapterTest {
   }
 
   @Test
-  public void removeInexistnetKey() {
+  public void removeInexistentKey() {
     LogbackMDCAdapter lma = new LogbackMDCAdapter();
     lma.remove("abcdlw0");
   }
 
+
+  @Test
+  public void sequenceWithGet() {
+    LogbackMDCAdapter lma = new LogbackMDCAdapter();
+    lma.put("k0", "v0");
+    Map<String, String> map0 = lma.copyOnInheritThreadLocal.get();
+    lma.get("k0");  // point 0
+    lma.put("k0", "v1");
+    // verify that map0 is that in point 0
+    assertEquals("v0", map0.get("k0"));
+  }
+
+  @Test
+  public void sequenceWithGetPropertyMap() {
+    LogbackMDCAdapter lma = new LogbackMDCAdapter();
+    lma.put("k0", "v0");
+    Map<String, String> map0 = lma.getPropertyMap();  // point 0
+    lma.put("k0", "v1");
+    // verify that map0 is that in point 0
+    assertEquals("v0", map0.get("k0"));
+  }
+
+
   class ChildThreadForMDCAdapter extends Thread {
 
     LogbackMDCAdapter logbackMDCAdapter;
@@ -111,11 +135,11 @@ public class LogbackMDCAdapterTest {
     HashMap<String, String> parentHM = getHashMapFromMDC();
     assertTrue(parentHM != childThread.childHM);
 
-    HashMap<String, String> parentHMWitness = new  HashMap<String, String>();
+    HashMap<String, String> parentHMWitness = new HashMap<String, String>();
     parentHMWitness.put(firstKey, firstKey + B_SUFFIX);
     assertEquals(parentHMWitness, parentHM);
 
-    HashMap<String, String> childHMWitness = new  HashMap<String, String>();
+    HashMap<String, String> childHMWitness = new HashMap<String, String>();
     childHMWitness.put(firstKey, firstKey + A_SUFFIX);
     childHMWitness.put(secondKey, secondKey + A_SUFFIX);
     assertEquals(childHMWitness, childThread.childHM);
@@ -150,13 +174,13 @@ public class LogbackMDCAdapterTest {
   }
 
   HashMap<String, String> getHashMapFromMDCAdapter(LogbackMDCAdapter lma) {
-     InheritableThreadLocal<HashMap<String, String>> copyOnInheritThreadLocal = lma.copyOnInheritThreadLocal;
+    InheritableThreadLocal<HashMap<String, String>> copyOnInheritThreadLocal = lma.copyOnInheritThreadLocal;
     return copyOnInheritThreadLocal.get();
   }
 
   HashMap<String, String> getHashMapFromMDC() {
     LogbackMDCAdapter lma = (LogbackMDCAdapter) MDC.getMDCAdapter();
-     InheritableThreadLocal<HashMap<String, String>> copyOnInheritThreadLocal = lma.copyOnInheritThreadLocal;
+    InheritableThreadLocal<HashMap<String, String>> copyOnInheritThreadLocal = lma.copyOnInheritThreadLocal;
     return copyOnInheritThreadLocal.get();
   }
 }

-----------------------------------------------------------------------

Summary of changes:
 .../logback/classic/util/LogbackMDCAdapter.java    |  116 ++++++++-----------
 .../classic/util/LogbackMDCAdapterTest.java        |   34 +++++-
 2 files changed, 78 insertions(+), 72 deletions(-)


hooks/post-receive
-- 
Logback: the generic, reliable, fast and flexible logging framework.


More information about the logback-dev mailing list