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=e068bd3b9754e8d62e4ca7994... http://github.com/ceki/logback/commit/e068bd3b9754e8d62e4ca799480bc3a6c7e146... commit e068bd3b9754e8d62e4ca799480bc3a6c7e14683 Author: Ceki Gulcu <ceki@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ülcü */ -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.