diff --git a/integration/pom.xml b/integration/pom.xml index a34a9a2..1bf94b9 100755 --- a/integration/pom.xml +++ b/integration/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 integration diff --git a/jcl-over-slf4j/pom.xml b/jcl-over-slf4j/pom.xml index 2c3c7a6..9dd9f61 100755 --- a/jcl-over-slf4j/pom.xml +++ b/jcl-over-slf4j/pom.xml @@ -5,7 +5,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 4.0.0 diff --git a/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLocationAwareLog.java b/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLocationAwareLog.java index ef48be6..1c4444f 100644 --- a/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLocationAwareLog.java +++ b/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLocationAwareLog.java @@ -104,7 +104,9 @@ * the message to log. Converted to {@link String} */ public void trace(Object message) { - logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, null); + if (isTraceEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, null); + } } /** @@ -117,7 +119,9 @@ * the exception to log */ public void trace(Object message, Throwable t) { - logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, t); + if (isTraceEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.TRACE_INT, String.valueOf(message), null, t); + } } /** @@ -128,7 +132,9 @@ * the message to log. Converted to {@link String} */ public void debug(Object message) { - logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, null); + if (isDebugEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, null); + } } /** @@ -141,7 +147,9 @@ * the exception to log */ public void debug(Object message, Throwable t) { - logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, t); + if (isDebugEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.DEBUG_INT, String.valueOf(message), null, t); + } } /** @@ -152,7 +160,9 @@ * the message to log. Converted to {@link String} */ public void info(Object message) { - logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, null); + if (isInfoEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, null); + } } /** @@ -165,7 +175,9 @@ * the exception to log */ public void info(Object message, Throwable t) { - logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, t); + if (isInfoEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.INFO_INT, String.valueOf(message), null, t); + } } /** @@ -176,7 +188,9 @@ * the message to log. Converted to {@link String} */ public void warn(Object message) { - logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, null); + if (isWarnEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, null); + } } /** @@ -189,7 +203,9 @@ * the exception to log */ public void warn(Object message, Throwable t) { - logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, t); + if (isWarnEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.WARN_INT, String.valueOf(message), null, t); + } } /** @@ -200,7 +216,9 @@ * the message to log. Converted to {@link String} */ public void error(Object message) { - logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null); + if (isErrorEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null); + } } /** @@ -213,7 +231,9 @@ * the exception to log */ public void error(Object message, Throwable t) { - logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t); + if (isErrorEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t); + } } /** @@ -224,7 +244,9 @@ * the message to log. Converted to {@link String} */ public void fatal(Object message) { - logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null); + if (isErrorEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, null); + } } /** @@ -237,7 +259,9 @@ * the exception to log */ public void fatal(Object message, Throwable t) { - logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t); + if (isErrorEnabled()) { + logger.log(null, FQCN, LocationAwareLogger.ERROR_INT, String.valueOf(message), null, t); + } } /** diff --git a/jcl-over-slf4j/src/test/java/org/apache/commons/logging/InvokeJCLTest.java b/jcl-over-slf4j/src/test/java/org/apache/commons/logging/InvokeJCLTest.java index 1cef216..5d373c8 100644 --- a/jcl-over-slf4j/src/test/java/org/apache/commons/logging/InvokeJCLTest.java +++ b/jcl-over-slf4j/src/test/java/org/apache/commons/logging/InvokeJCLTest.java @@ -27,6 +27,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; import org.junit.Test; @@ -85,4 +86,56 @@ log.fatal(null, e); log.fatal("fatal message", e); } + + @Test + public void testAvoidConvertingObjectToString() { + Log log = LogFactory.getLog(InvokeJCLTest.class); + Exception e = new Exception("just testing"); + + TestMessage fatalMsg = new TestMessage("fatal msg"); + TestMessage errorMsg = new TestMessage("error msg"); + TestMessage warnMsg = new TestMessage("warn msg"); + TestMessage infoMsg = new TestMessage("info msg"); + TestMessage debugMsg = new TestMessage("debug msg"); + TestMessage traceMsg = new TestMessage("trace msg"); + + log.fatal(fatalMsg); + log.fatal(fatalMsg, e); + assertEquals(2, fatalMsg.invokedCount); + + log.error(errorMsg); + log.error(errorMsg, e); + assertEquals(2, errorMsg.invokedCount); + + log.warn(warnMsg); + log.warn(warnMsg, e); + assertEquals(2, warnMsg.invokedCount); + + log.info(infoMsg); + log.info(infoMsg, e); + assertEquals(2, infoMsg.invokedCount); + + log.debug(debugMsg); + log.debug(debugMsg, e); + assertEquals(0, debugMsg.invokedCount); + + log.trace(traceMsg); + log.trace(traceMsg, e); + assertEquals(0, traceMsg.invokedCount); + } + + static class TestMessage { + + private final String msg; + int invokedCount = 0; + + TestMessage(String msg) {this.msg = msg;} + + @Override + public String toString() { + invokedCount++; + return msg; + } + } + } diff --git a/jul-to-slf4j/pom.xml b/jul-to-slf4j/pom.xml index 85ea8d0..174185b 100755 --- a/jul-to-slf4j/pom.xml +++ b/jul-to-slf4j/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 jul-to-slf4j diff --git a/log4j-over-slf4j/pom.xml b/log4j-over-slf4j/pom.xml index 80226f5..e772a74 100755 --- a/log4j-over-slf4j/pom.xml +++ b/log4j-over-slf4j/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 diff --git a/log4j-over-slf4j/src/main/java/org/apache/log4j/ConsoleAppender.java b/log4j-over-slf4j/src/main/java/org/apache/log4j/ConsoleAppender.java index 40e8f80..c193507 100644 --- a/log4j-over-slf4j/src/main/java/org/apache/log4j/ConsoleAppender.java +++ b/log4j-over-slf4j/src/main/java/org/apache/log4j/ConsoleAppender.java @@ -15,6 +15,18 @@ */ package org.apache.log4j; +/** + * Skeleton implementation of ConsoleAppender + */ public class ConsoleAppender extends WriterAppender { + public ConsoleAppender() { + } + + public ConsoleAppender(Layout layout) { + } + + public ConsoleAppender(Layout layout, String target) { + } + } diff --git a/osgi-over-slf4j/pom.xml b/osgi-over-slf4j/pom.xml index 9237dec..86a0691 100755 --- a/osgi-over-slf4j/pom.xml +++ b/osgi-over-slf4j/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 osgi-over-slf4j diff --git a/pom.xml b/pom.xml index 8e71283..04c84f2 100755 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 pom SLF4J diff --git a/slf4j-android/pom.xml b/slf4j-android/pom.xml index 1b86e27..38433a1 100644 --- a/slf4j-android/pom.xml +++ b/slf4j-android/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-android diff --git a/slf4j-api/pom.xml b/slf4j-api/pom.xml index 78e96a9..44ce4fb 100755 --- a/slf4j-api/pom.xml +++ b/slf4j-api/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-api diff --git a/slf4j-ext/pom.xml b/slf4j-ext/pom.xml index 9acc1e2..68b6ea8 100755 --- a/slf4j-ext/pom.xml +++ b/slf4j-ext/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-ext diff --git a/slf4j-jcl/pom.xml b/slf4j-jcl/pom.xml index d9fa8c9..298759c 100755 --- a/slf4j-jcl/pom.xml +++ b/slf4j-jcl/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-jcl diff --git a/slf4j-jdk14/pom.xml b/slf4j-jdk14/pom.xml index d343309..82fa83d 100755 --- a/slf4j-jdk14/pom.xml +++ b/slf4j-jdk14/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-jdk14 diff --git a/slf4j-log4j12/pom.xml b/slf4j-log4j12/pom.xml index bc40ac8..2510f29 100755 --- a/slf4j-log4j12/pom.xml +++ b/slf4j-log4j12/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-log4j12 diff --git a/slf4j-migrator/pom.xml b/slf4j-migrator/pom.xml index 2a516a1..72cf981 100755 --- a/slf4j-migrator/pom.xml +++ b/slf4j-migrator/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-migrator diff --git a/slf4j-nop/pom.xml b/slf4j-nop/pom.xml index 8f78462..4d15693 100755 --- a/slf4j-nop/pom.xml +++ b/slf4j-nop/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-nop diff --git a/slf4j-simple/pom.xml b/slf4j-simple/pom.xml index 9de2a31..d20f444 100755 --- a/slf4j-simple/pom.xml +++ b/slf4j-simple/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-simple diff --git a/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLogger.java b/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLogger.java index fe79822..0a3aeea 100644 --- a/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLogger.java +++ b/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLogger.java @@ -157,7 +157,7 @@ protected static final int LOG_LEVEL_OFF = LOG_LEVEL_ERROR + 10; private static boolean INITIALIZED = false; - static SimpleLoggerConfiguration CONFIG_PARAMS = null; + static final private SimpleLoggerConfiguration CONFIG_PARAMS = new SimpleLoggerConfiguration(); static void lazyInit() { if (INITIALIZED) { @@ -170,7 +170,6 @@ // external software might be invoking this method directly. Do not rename // or change its semantics. static void init() { - CONFIG_PARAMS = new SimpleLoggerConfiguration(); CONFIG_PARAMS.init(); } @@ -312,12 +311,22 @@ throw new IllegalStateException("Unrecognized level [" + level + "]"); } + /** + * To avoid intermingling of log messages and associated stack traces, the two + * operations are done in a synchronized block. + * + * @param buf + * @param t + */ void write(StringBuilder buf, Throwable t) { PrintStream targetStream = CONFIG_PARAMS.outputChoice.getTargetPrintStream(); - targetStream.println(buf.toString()); - writeThrowable(t, targetStream); - targetStream.flush(); + synchronized (CONFIG_PARAMS) { + targetStream.println(buf.toString()); + writeThrowable(t, targetStream); + targetStream.flush(); + } + } protected void writeThrowable(Throwable t, PrintStream targetStream) { diff --git a/slf4j-simple/src/test/java/org/slf4j/simple/multiThreadedExecution/MultithereadedExecutionTest.java b/slf4j-simple/src/test/java/org/slf4j/simple/multiThreadedExecution/MultithereadedExecutionTest.java new file mode 100755 index 0000000..3888cbc --- /dev/null +++ b/slf4j-simple/src/test/java/org/slf4j/simple/multiThreadedExecution/MultithereadedExecutionTest.java @@ -0,0 +1,128 @@ +/** + * Copyright (c) 2004-2021 QOS.ch + * 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.simple.multiThreadedExecution; + +import java.io.PrintStream; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Tests that output in multi-threaded environments is not mingled. + * + * See also https://jira.qos.ch/browse/SLF4J-515 + */ +public class MultithereadedExecutionTest { + + private static int THREAD_COUNT = 2; + private static long TEST_DURATION_IN_MILLIS = 100; + + private Thread[] threads = new Thread[THREAD_COUNT]; + + private final PrintStream oldOut = System.out; + StateCheckingPrintStream scps = new StateCheckingPrintStream(oldOut); + + volatile boolean signal = false; + + @Before + public void setup() { + System.setErr(scps); + // System.setProperty(SimpleLogger.LOG_FILE_KEY, "System.err"); + // LoggerFactoryFriend.reset(); + } + + @After + public void tearDown() throws Exception { + // LoggerFactoryFriend.reset(); + // System.clearProperty(SimpleLogger.LOG_FILE_KEY); + System.setErr(oldOut); + } + + @Test + public void test() throws Throwable { + WithException withException = new WithException(); + Other other = new Other(); + threads[0] = new Thread(withException); + threads[1] = new Thread(other); + threads[0].start(); + threads[1].start(); + Thread.sleep(TEST_DURATION_IN_MILLIS); + signal = true; + threads[0].join(); + threads[1].join(); + + if (withException.throwable != null) { + throw withException.throwable; + } + + if (other.throwable != null) { + throw other.throwable; + } + + } + + class WithException implements Runnable { + + volatile Throwable throwable; + Logger logger = LoggerFactory.getLogger(WithException.class); + + public void run() { + int i = 0; + + while (!signal) { + try { + logger.info("Hello {}", i, new Throwable("i=" + i)); + i++; + } catch (Throwable t) { + throwable = t; + MultithereadedExecutionTest.this.signal = true; + return; + } + } + + } + } + + class Other implements Runnable { + volatile Throwable throwable; + Logger logger = LoggerFactory.getLogger(Other.class); + + public void run() { + int i = 0; + while (!signal) { + try { + logger.info("Other {}", i++); + } catch (Throwable t) { + throwable = t; + MultithereadedExecutionTest.this.signal = true; + return; + } + } + } + } +} diff --git a/slf4j-simple/src/test/java/org/slf4j/simple/multiThreadedExecution/StateCheckingPrintStream.java b/slf4j-simple/src/test/java/org/slf4j/simple/multiThreadedExecution/StateCheckingPrintStream.java new file mode 100755 index 0000000..e4dbe14 --- /dev/null +++ b/slf4j-simple/src/test/java/org/slf4j/simple/multiThreadedExecution/StateCheckingPrintStream.java @@ -0,0 +1,141 @@ +/** + * Copyright (c) 2004-2021 QOS.ch + * 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.simple.multiThreadedExecution; + +import java.io.PrintStream; +import java.util.regex.Pattern; + +/** + * This PrintStream checks that output lines are in an expected order. + * + * @author ceki + */ +public class StateCheckingPrintStream extends PrintStream { + + enum State { + INITIAL, UNKNOWN, HELLO, THROWABLE, AT1, AT2, OTHER; + } + + volatile State currentState = State.INITIAL; + + public StateCheckingPrintStream(PrintStream ps) { + super(ps); + } + + public void print(String s) { + } + + public void println(String s) { + + State next = computeState(s); + //System.out.println(next + " " + s); + switch (currentState) { + case INITIAL: + currentState = next; + break; + + case UNKNOWN: + // ignore garbage + currentState = next; + break; + + case OTHER: + if (next == State.UNKNOWN) { + currentState = State.UNKNOWN; + return; + } + + if (next != State.OTHER && next != State.HELLO) { + throw badState(s, currentState, next); + } + currentState = next; + break; + + case HELLO: + if (next != State.THROWABLE) { + throw badState(s, currentState, next); + } + currentState = next; + break; + case THROWABLE: + if (next != State.AT1) { + throw badState(s, currentState, next); + } + currentState = next; + break; + + case AT1: + if (next != State.AT2) { + throw badState(s, currentState, next); + } + currentState = next; + break; + + case AT2: + currentState = next; + break; + default: + throw new IllegalStateException("Unreachable code"); + } + } + + private IllegalStateException badState(String s, State currentState2, State next) { + return new IllegalStateException("Unexpected state " + next + " for current state " + currentState2 + " for " + s); + + } + + String OTHER_PATTERN_STR = ".*Other \\d{1,5}"; + String HELLO_PATTERN_STR = ".*Hello \\d{1,5}"; + String THROWABLE_PATTERN_STR = "java.lang.Throwable: i=\\d{1,5}"; + String AT1_PATTERN_STR = "\\s*at " + this.getClass().getPackage().getName() + ".*"; + String AT2_PATTERN_STR = "\\s*at " + ".*Thread.java.*"; + + Pattern PATTERN_OTHER = Pattern.compile(OTHER_PATTERN_STR); + Pattern PATTERN_HELLO = Pattern.compile(HELLO_PATTERN_STR); + Pattern PATTERN_THROWABLE = Pattern.compile(THROWABLE_PATTERN_STR); + Pattern PATTERN_AT1 = Pattern.compile(AT1_PATTERN_STR); + Pattern PATTERN_AT2 = Pattern.compile(AT2_PATTERN_STR); + + private State computeState(String s) { + + if (PATTERN_OTHER.matcher(s).matches()) { + return State.OTHER; + } else if (PATTERN_HELLO.matcher(s).matches()) { + return State.HELLO; + } else if (PATTERN_THROWABLE.matcher(s).matches()) { + return State.THROWABLE; + } else if (PATTERN_AT1.matcher(s).matches()) { + return State.AT1; + } else if (PATTERN_AT2.matcher(s).matches()) { + return State.AT2; + } else { + return State.UNKNOWN; + } + } + + public void println(Object o) { + println(o.toString()); + } +} \ No newline at end of file diff --git a/slf4j-site/pom.xml b/slf4j-site/pom.xml index 6869e58..21c64fb 100755 --- a/slf4j-site/pom.xml +++ b/slf4j-site/pom.xml @@ -7,7 +7,7 @@ org.slf4j slf4j-parent - 1.7.30 + 1.7.32 slf4j-site