diff --git a/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java b/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java index 6f07c7d2..fc0c578f 100644 --- a/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java +++ b/slf4j-api/src/main/java/org/slf4j/helpers/BasicMDCAdapter.java @@ -1,5 +1,5 @@ /** - * Copyright (c) 2004-2011 QOS.ch + * Copyright (c) 2004-2011 QOS.ch, Copyright (C) 2015 Google Inc. * All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining @@ -44,7 +44,15 @@ import java.util.Map; public class BasicMDCAdapter implements MDCAdapter { private InheritableThreadLocal> inheritableThreadLocal - = new InheritableThreadLocal>(); + = new InheritableThreadLocal>() { + @Override + protected Map childValue(Map parentValue) { + if (parentValue == null) { + return null; + } + return new HashMap(parentValue); + } + }; static boolean isJDK14() { try { @@ -71,13 +79,14 @@ public class BasicMDCAdapter implements MDCAdapter { * @throws IllegalArgumentException * in case the "key" parameter is null */ + @Override public void put(String key, String val) { if (key == null) { throw new IllegalArgumentException("key cannot be null"); } - Map map = (Map) inheritableThreadLocal.get(); + Map map = inheritableThreadLocal.get(); if (map == null) { - map = Collections.synchronizedMap(new HashMap()); + map = new HashMap(); inheritableThreadLocal.set(map); } map.put(key, val); @@ -86,10 +95,11 @@ public class BasicMDCAdapter implements MDCAdapter { /** * Get the context identified by the key parameter. */ + @Override public String get(String key) { - Map Map = (Map) inheritableThreadLocal.get(); + Map Map = inheritableThreadLocal.get(); if ((Map != null) && (key != null)) { - return (String) Map.get(key); + return Map.get(key); } else { return null; } @@ -98,8 +108,9 @@ public class BasicMDCAdapter implements MDCAdapter { /** * Remove the the context identified by the key parameter. */ + @Override public void remove(String key) { - Map map = (Map) inheritableThreadLocal.get(); + Map map = inheritableThreadLocal.get(); if (map != null) { map.remove(key); } @@ -108,8 +119,9 @@ public class BasicMDCAdapter implements MDCAdapter { /** * Clear all entries in the MDC. */ + @Override public void clear() { - Map map = (Map) inheritableThreadLocal.get(); + Map map = inheritableThreadLocal.get(); if (map != null) { map.clear(); // the InheritableThreadLocal.remove method was introduced in JDK 1.5 @@ -129,34 +141,32 @@ public class BasicMDCAdapter implements MDCAdapter { * @return the keys in the MDC */ public Set getKeys() { - Map map = (Map) inheritableThreadLocal.get(); + Map map = inheritableThreadLocal.get(); if (map != null) { return map.keySet(); } else { return null; } } + /** * Return a copy of the current thread's context map. * Returned value may be null. * */ + @Override public Map getCopyOfContextMap() { - Map oldMap = (Map) inheritableThreadLocal.get(); + Map oldMap = inheritableThreadLocal.get(); if (oldMap != null) { - Map newMap = Collections.synchronizedMap(new HashMap()); - synchronized (oldMap) { - newMap.putAll(oldMap); - } - return newMap; + return new HashMap(oldMap); } else { return null; } } + @Override public void setContextMap(Map contextMap) { - Map map = Collections.synchronizedMap(new HashMap(contextMap)); - inheritableThreadLocal.set(map); + inheritableThreadLocal.set(new HashMap(contextMap)); } } diff --git a/slf4j-api/src/test/java/org/slf4j/helpers/BasicMDCAdapterTest.java b/slf4j-api/src/test/java/org/slf4j/helpers/BasicMDCAdapterTest.java new file mode 100644 index 00000000..39adef92 --- /dev/null +++ b/slf4j-api/src/test/java/org/slf4j/helpers/BasicMDCAdapterTest.java @@ -0,0 +1,139 @@ +/** + * Copyright (c) 2004-2013 QOS.ch, Copyright (C) 2015 Google Inc. + * All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +package org.slf4j.helpers; + +import junit.framework.TestCase; + +import org.slf4j.spi.MDCAdapter; + +import java.lang.Thread.UncaughtExceptionHandler; +import java.util.Map; + +/** + * Tests for {@link BasicMDCAdapter} + * + * @author Lukasz Cwik + */ +public class BasicMDCAdapterTest extends TestCase { + MDCAdapter mdc = new BasicMDCAdapter(); + + @Override + protected void tearDown() throws Exception { + mdc.clear(); + } + + public void testSettingAndGettingWithMDC() { + assertNull(mdc.get("testKey")); + mdc.put("testKey", "testValue"); + assertEquals(mdc.get("testKey"), "testValue"); + } + + public void testOverwritingAKeyInMDC() { + assertNull(mdc.get("testKey")); + mdc.put("testKey", "testValue"); + mdc.put("testKey", "differentTestValue"); + assertEquals(mdc.get("testKey"), "differentTestValue"); + } + + public void testClearingMDC() { + mdc.put("testKey", "testValue"); + assertFalse(mdc.getCopyOfContextMap().isEmpty()); + mdc.clear(); + assertNull(mdc.getCopyOfContextMap()); + } + + public void testGetCopyOfContextMapFromMDC() { + mdc.put("testKey", "testValue"); + Map copy = mdc.getCopyOfContextMap(); + mdc.put("anotherTestKey", "anotherTestValue"); + assertFalse(copy.size() == mdc.getCopyOfContextMap().size()); + } + + public void testMDCInheritsValuesFromParentThread() throws Exception { + mdc.put("parentKey", "parentValue"); + runAndWait(new Runnable() { + @Override + public void run() { + mdc.put("childKey", "childValue"); + assertEquals("parentValue", mdc.get("parentKey")); + } + }); + } + + public void testMDCDoesntGetValuesFromChildThread() throws Exception { + mdc.put("parentKey", "parentValue"); + runAndWait(new Runnable() { + @Override + public void run() { + mdc.put("childKey", "childValue"); + } + }); + assertEquals("parentValue", mdc.get("parentKey")); + assertNull(mdc.get("childKey")); + } + + public void testMDCChildThreadCanOverwriteParentThread() throws Exception { + mdc.put("sharedKey", "parentValue"); + runAndWait(new Runnable() { + @Override + public void run() { + assertEquals("parentValue", mdc.get("sharedKey")); + mdc.put("sharedKey", "childValue"); + assertEquals("childValue", mdc.get("sharedKey")); + } + }); + assertEquals("parentValue", mdc.get("sharedKey")); + } + + private void runAndWait(Runnable runnable) throws Exception { + RecordingExceptionHandler handler = new RecordingExceptionHandler(); + Thread thread = new Thread(runnable); + thread.setUncaughtExceptionHandler(handler); + thread.start(); + try { + thread.join(); + } catch(Throwable t) { + fail("Unexpected failure in child thread:" + t.getMessage()); + } + assertFalse(handler.getMessage(), handler.hadException()); + } + + /** A {@link UncaughtExceptionHandler} that records whether the thread threw an exception. */ + private static class RecordingExceptionHandler implements UncaughtExceptionHandler { + private Throwable exception; + @Override + public void uncaughtException(Thread t, Throwable e) { + exception = e; + } + + boolean hadException() { + return exception != null; + } + + String getMessage() { + return exception != null ? exception.getMessage() : ""; + } + } +}