From c6c7a98a7ee1e71382348ae3e41ff7818dfafd65 Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Mon, 16 Dec 2019 21:52:14 +0100 Subject: [PATCH] fix SLF4J-469 --- integration/build.xml | 26 +++- .../slf4j/helpers/LoggerAccessingThread2.java | 65 ++++++++++ ...ndingMultithreadedInitializationTest2.java | 112 ++++++++++++++++++ .../main/java/org/slf4j/LoggerFactory.java | 13 +- .../MultithreadedInitializationTest.java | 2 +- 5 files changed, 208 insertions(+), 10 deletions(-) create mode 100644 integration/src/test/java/org/slf4j/helpers/LoggerAccessingThread2.java create mode 100644 integration/src/test/java/org/slf4j/helpers/NoBindingMultithreadedInitializationTest2.java diff --git a/integration/build.xml b/integration/build.xml index 6f7bb039d..e4ce964c1 100755 --- a/integration/build.xml +++ b/integration/build.xml @@ -56,7 +56,12 @@ + + + + + @@ -102,11 +107,12 @@ @@ -215,7 +221,17 @@ - + + + + + + + + + diff --git a/integration/src/test/java/org/slf4j/helpers/LoggerAccessingThread2.java b/integration/src/test/java/org/slf4j/helpers/LoggerAccessingThread2.java new file mode 100644 index 000000000..86590b3d6 --- /dev/null +++ b/integration/src/test/java/org/slf4j/helpers/LoggerAccessingThread2.java @@ -0,0 +1,65 @@ +/** + * Copyright (c) 2004-2016 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.helpers; + +import java.util.List; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.atomic.AtomicLong; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class LoggerAccessingThread2 extends Thread { + private static int LOOP_LEN = 64; + + final CyclicBarrier barrier; + final int count; + final AtomicLong eventCount; + List loggerList; + + public LoggerAccessingThread2(final CyclicBarrier barrier, List loggerList, final int count, final AtomicLong eventCount) { + this.barrier = barrier; + this.loggerList = loggerList; + this.count = count; + this.eventCount = eventCount; + } + + public void run() { + try { + barrier.await(); + } catch (Exception e) { + e.printStackTrace(); + } + + String loggerNamePrefix = this.getClass().getName(); + for (int i = 0; i < LOOP_LEN; i++) { + Logger logger = LoggerFactory.getLogger(loggerNamePrefix + "-" + count + "-" + i); + loggerList.add(logger); + Thread.yield(); + logger.info("in run method"); + eventCount.getAndIncrement(); + } + } +} diff --git a/integration/src/test/java/org/slf4j/helpers/NoBindingMultithreadedInitializationTest2.java b/integration/src/test/java/org/slf4j/helpers/NoBindingMultithreadedInitializationTest2.java new file mode 100644 index 000000000..6de0f8ec3 --- /dev/null +++ b/integration/src/test/java/org/slf4j/helpers/NoBindingMultithreadedInitializationTest2.java @@ -0,0 +1,112 @@ +/** + * Copyright (c) 2004-2016 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.helpers; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Random; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.atomic.AtomicLong; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.event.EventRecodingLogger; + +import junit.framework.TestCase; + +public class NoBindingMultithreadedInitializationTest2 extends TestCase { + final protected static int THREAD_COUNT = 4 + Runtime.getRuntime().availableProcessors() * 2; + + private final List createdLoggers = Collections.synchronizedList(new ArrayList()); + + protected final AtomicLong eventCount = new AtomicLong(0); + final private CyclicBarrier barrier = new CyclicBarrier(THREAD_COUNT + 1); + + int diff = new Random().nextInt(10000); + + + public NoBindingMultithreadedInitializationTest2(String name) { + super(name); + } + + public void testNoBindingMultiThreadedInitialization() throws InterruptedException, BrokenBarrierException { + @SuppressWarnings("unused") + LoggerAccessingThread2[] accessors = harness(); + + Logger logger = LoggerFactory.getLogger(getClass().getName()); + logger.info("hello"); + eventCount.getAndIncrement(); + + assertAllSubstLoggersAreFixed(); + long recordedEventCount = getRecordedEventCount(); + int LENIENCY_COUNT = 16; + + long expectedEventCount = eventCount.get() + extraLogEvents(); + + assertTrue(expectedEventCount + " >= " + recordedEventCount, expectedEventCount >= recordedEventCount); + assertTrue(expectedEventCount + " < " + recordedEventCount + "+" + LENIENCY_COUNT, + expectedEventCount < recordedEventCount + LENIENCY_COUNT); + } + + protected int extraLogEvents() { + return 0; + } + + private void assertAllSubstLoggersAreFixed() { + for (Logger logger : createdLoggers) { + if (logger instanceof SubstituteLogger) { + SubstituteLogger substLogger = (SubstituteLogger) logger; + if (substLogger.delegate() instanceof EventRecodingLogger) + fail("substLogger " + substLogger.getName() + " has a delegate of type EventRecodingLogger"); + } + } + } + + private LoggerAccessingThread2[] harness() throws InterruptedException, BrokenBarrierException { + LoggerAccessingThread2[] threads = new LoggerAccessingThread2[THREAD_COUNT]; + for (int i = 0; i < THREAD_COUNT; i++) { + threads[i] = new LoggerAccessingThread2(barrier, createdLoggers, i, eventCount); + threads[i].start(); + } + + // trigger barrier + barrier.await(); + + for (int i = 0; i < THREAD_COUNT; i++) { + threads[i].join(); + } + + return threads; + } + + final String loggerName = this.getClass().getName(); + + protected long getRecordedEventCount() { + return eventCount.get(); + } + +} diff --git a/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java b/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java index 30b5c7257..9ed2c19e7 100755 --- a/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java +++ b/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java @@ -150,10 +150,6 @@ private final static void bind() { StaticLoggerBinder.getSingleton(); INITIALIZATION_STATE = SUCCESSFUL_INITIALIZATION; reportActualBinding(staticLoggerBinderPathSet); - fixSubstituteLoggers(); - replayEvents(); - // release all resources in SUBST_FACTORY - SUBST_FACTORY.clear(); } catch (NoClassDefFoundError ncde) { String msg = ncde.getMessage(); if (messageContainsOrgSlf4jImplStaticLoggerBinder(msg)) { @@ -177,9 +173,18 @@ private final static void bind() { } catch (Exception e) { failedBinding(e); throw new IllegalStateException("Unexpected initialization failure", e); + } finally { + postBindCleanUp(); } } + private static void postBindCleanUp() { + fixSubstituteLoggers(); + replayEvents(); + // release all resources in SUBST_FACTORY + SUBST_FACTORY.clear(); + } + private static void fixSubstituteLoggers() { synchronized (SUBST_FACTORY) { SUBST_FACTORY.postInitialization(); diff --git a/slf4j-api/src/test/java/org/slf4j/helpers/MultithreadedInitializationTest.java b/slf4j-api/src/test/java/org/slf4j/helpers/MultithreadedInitializationTest.java index 6cf361348..c9e38eae7 100644 --- a/slf4j-api/src/test/java/org/slf4j/helpers/MultithreadedInitializationTest.java +++ b/slf4j-api/src/test/java/org/slf4j/helpers/MultithreadedInitializationTest.java @@ -22,7 +22,7 @@ abstract public class MultithreadedInitializationTest { private final List createdLoggers = Collections.synchronizedList(new ArrayList()); - final private AtomicLong eventCount = new AtomicLong(0); + protected final AtomicLong eventCount = new AtomicLong(0); final private CyclicBarrier barrier = new CyclicBarrier(THREAD_COUNT + 1); int diff = new Random().nextInt(10000);