I actually don't know any good reason why they have different names. Most of the code used to exist in log4j-core, but a bunch of it was useful in log4j-api, so it's been slowly moved to the util package there.
On 8 September 2016 at 13:07, Gary Gregory <[email protected]> wrote: > I think org.apache.logging.log4j.util.LoaderUtil > and org.apache.logging.log4j.core.util.Loader. > > It's also not clear why one is called "Util" and the other not. > > This is all probably nit-picking since these are both internal classes but > it will help anyone looking at the code including us. > > It's always nice to document intent. > > Gary > > On Thu, Sep 8, 2016 at 10:30 AM, Matt Sicker <[email protected]> wrote: > >> Which ones? >> >> On 8 September 2016 at 12:27, Gary Gregory <[email protected]> >> wrote: >> >>> Good to know. Can the Javadocs be improved? >>> >>> Gary >>> >>> On Thu, Sep 8, 2016 at 9:39 AM, Matt Sicker <[email protected]> wrote: >>> >>>> I'm not using the loader utils here because we already have the >>>> ClassLoader required. The use of LoaderUtil tends to be where we don't know >>>> which ClassLoader to use. >>>> >>>> On 8 September 2016 at 11:09, Gary Gregory <[email protected]> >>>> wrote: >>>> >>>>> Hi Matt, >>>>> >>>>> When I look at the change in ThrowableProxy that go away from using >>>>> our Loader utils and go back to loading classes directly I find it >>>>> confusing: Are the Loader utils broken or are these call sites the wrong >>>>> use case for the utils? We (you IIRC) have made a point of using these >>>>> Loader utils (API and Core) to load classes all over, which is nice since >>>>> we get consistent pattern. So I find it confusing to see it done both ways >>>>> in the code overall. Does this warrant a comment in the code? It might >>>>> not, >>>>> I'm just asking the question. >>>>> >>>>> Thank you, >>>>> Gary >>>>> >>>>> >>>>> On Thu, Sep 8, 2016 at 8:29 AM, <[email protected]> wrote: >>>>> >>>>>> Repository: logging-log4j2 >>>>>> Updated Branches: >>>>>> refs/heads/master 99c0d011d -> 284067cbd >>>>>> >>>>>> >>>>>> Fix class loader deadlock when using async logging and extended stack >>>>>> trace pattern >>>>>> >>>>>> This fixes LOG4J2-1457. >>>>>> >>>>>> >>>>>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >>>>>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit >>>>>> /284067cb >>>>>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2 >>>>>> 84067cb >>>>>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2 >>>>>> 84067cb >>>>>> >>>>>> Branch: refs/heads/master >>>>>> Commit: 284067cbdb5f13c41225e22c2a60d79cd43f1383 >>>>>> Parents: 99c0d01 >>>>>> Author: Matt Sicker <[email protected]> >>>>>> Authored: Thu Sep 8 10:30:12 2016 -0500 >>>>>> Committer: Matt Sicker <[email protected]> >>>>>> Committed: Thu Sep 8 10:30:12 2016 -0500 >>>>>> >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> .../logging/log4j/core/impl/ThrowableProxy.java | 9 ++-- >>>>>> .../async/AsyncLoggerClassLoadDeadlock.java | 32 ++++++++++++++ >>>>>> .../async/AsyncLoggerClassLoadDeadlockTest.java | 45 >>>>>> ++++++++++++++++++++ >>>>>> .../test/resources/AsyncLoggerConsoleTest.xml | 16 +++++++ >>>>>> src/changes/changes.xml | 3 ++ >>>>>> 5 files changed, 100 insertions(+), 5 deletions(-) >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> >>>>>> >>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2 >>>>>> 84067cb/log4j-core/src/main/java/org/apache/logging/log4j/co >>>>>> re/impl/ThrowableProxy.java >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> diff --git a/log4j-core/src/main/java/org >>>>>> /apache/logging/log4j/core/impl/ThrowableProxy.java >>>>>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/imp >>>>>> l/ThrowableProxy.java >>>>>> index a052e4d..e12e14a 100644 >>>>>> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/imp >>>>>> l/ThrowableProxy.java >>>>>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/imp >>>>>> l/ThrowableProxy.java >>>>>> @@ -30,7 +30,6 @@ import java.util.Stack; >>>>>> >>>>>> import org.apache.logging.log4j.core.pattern.PlainTextRenderer; >>>>>> import org.apache.logging.log4j.core.pattern.TextRenderer; >>>>>> -import org.apache.logging.log4j.core.util.Loader; >>>>>> import org.apache.logging.log4j.status.StatusLogger; >>>>>> import org.apache.logging.log4j.util.LoaderUtil; >>>>>> import org.apache.logging.log4j.util.ReflectionUtil; >>>>>> @@ -537,7 +536,7 @@ public class ThrowableProxy implements >>>>>> Serializable { >>>>>> Class<?> clazz; >>>>>> if (lastLoader != null) { >>>>>> try { >>>>>> - clazz = Loader.initializeClass(className, >>>>>> lastLoader); >>>>>> + clazz = lastLoader.loadClass(className); >>>>>> if (clazz != null) { >>>>>> return clazz; >>>>>> } >>>>>> @@ -548,16 +547,16 @@ public class ThrowableProxy implements >>>>>> Serializable { >>>>>> try { >>>>>> clazz = LoaderUtil.loadClass(className); >>>>>> } catch (final ClassNotFoundException | NoClassDefFoundError >>>>>> e) { >>>>>> - return initializeClass(className); >>>>>> + return loadClass(className); >>>>>> } catch (final SecurityException e) { >>>>>> return null; >>>>>> } >>>>>> return clazz; >>>>>> } >>>>>> >>>>>> - private Class<?> initializeClass(final String className) { >>>>>> + private Class<?> loadClass(final String className) { >>>>>> try { >>>>>> - return Loader.initializeClass(className, >>>>>> this.getClass().getClassLoader()); >>>>>> + return this.getClass().getClassLoader >>>>>> ().loadClass(className); >>>>>> } catch (final ClassNotFoundException | NoClassDefFoundError >>>>>> | SecurityException e) { >>>>>> return null; >>>>>> } >>>>>> >>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2 >>>>>> 84067cb/log4j-core/src/test/java/org/apache/logging/log4j/co >>>>>> re/async/AsyncLoggerClassLoadDeadlock.java >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> diff --git a/log4j-core/src/test/java/org >>>>>> /apache/logging/log4j/core/async/AsyncLoggerClassLoadDeadlock.java >>>>>> b/log4j-core/src/test/java/org/apache/logging/log4j/core/asy >>>>>> nc/AsyncLoggerClassLoadDeadlock.java >>>>>> new file mode 100644 >>>>>> index 0000000..f8341f9 >>>>>> --- /dev/null >>>>>> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/asy >>>>>> nc/AsyncLoggerClassLoadDeadlock.java >>>>>> @@ -0,0 +1,32 @@ >>>>>> +/* >>>>>> + * Licensed to the Apache Software Foundation (ASF) under one or more >>>>>> + * contributor license agreements. See the NOTICE file distributed >>>>>> with >>>>>> + * this work for additional information regarding copyright >>>>>> ownership. >>>>>> + * The ASF licenses this file to You under the Apache license, >>>>>> Version 2.0 >>>>>> + * (the "License"); you may not use this file except in compliance >>>>>> with >>>>>> + * the License. You may obtain a copy of the License at >>>>>> + * >>>>>> + * http://www.apache.org/licenses/LICENSE-2.0 >>>>>> + * >>>>>> + * Unless required by applicable law or agreed to in writing, >>>>>> software >>>>>> + * distributed under the License is distributed on an "AS IS" BASIS, >>>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >>>>>> implied. >>>>>> + * See the license for the specific language governing permissions >>>>>> and >>>>>> + * limitations under the license. >>>>>> + */ >>>>>> +package org.apache.logging.log4j.core.async; >>>>>> + >>>>>> +import org.apache.logging.log4j.LogManager; >>>>>> +import org.apache.logging.log4j.Logger; >>>>>> + >>>>>> +class AsyncLoggerClassLoadDeadlock { >>>>>> + static { >>>>>> + final Logger log = LogManager.getLogger("com.foo. >>>>>> bar.deadlock"); >>>>>> + final Exception e = new Exception(); >>>>>> + // the key to reproducing the problem is to fill up the ring >>>>>> buffer so that >>>>>> + // log.info call will block on ring buffer as well >>>>>> + for (int i = 0; i < >>>>>> AsyncLoggerClassLoadDeadlockTest.RING_BUFFER_SIZE >>>>>> * 2; ++i) { >>>>>> + log.info("clinit", e); >>>>>> + } >>>>>> + } >>>>>> +} >>>>>> >>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2 >>>>>> 84067cb/log4j-core/src/test/java/org/apache/logging/log4j/co >>>>>> re/async/AsyncLoggerClassLoadDeadlockTest.java >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> diff --git a/log4j-core/src/test/java/org >>>>>> /apache/logging/log4j/core/async/AsyncLoggerClassLoadDeadlockTest.java >>>>>> b/log4j-core/src/test/java/org/apache/logging/log4j/core/asy >>>>>> nc/AsyncLoggerClassLoadDeadlockTest.java >>>>>> new file mode 100644 >>>>>> index 0000000..990312d >>>>>> --- /dev/null >>>>>> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/asy >>>>>> nc/AsyncLoggerClassLoadDeadlockTest.java >>>>>> @@ -0,0 +1,45 @@ >>>>>> +/* >>>>>> + * Licensed to the Apache Software Foundation (ASF) under one or more >>>>>> + * contributor license agreements. See the NOTICE file distributed >>>>>> with >>>>>> + * this work for additional information regarding copyright >>>>>> ownership. >>>>>> + * The ASF licenses this file to You under the Apache license, >>>>>> Version 2.0 >>>>>> + * (the "License"); you may not use this file except in compliance >>>>>> with >>>>>> + * the License. You may obtain a copy of the License at >>>>>> + * >>>>>> + * http://www.apache.org/licenses/LICENSE-2.0 >>>>>> + * >>>>>> + * Unless required by applicable law or agreed to in writing, >>>>>> software >>>>>> + * distributed under the License is distributed on an "AS IS" BASIS, >>>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >>>>>> implied. >>>>>> + * See the license for the specific language governing permissions >>>>>> and >>>>>> + * limitations under the license. >>>>>> + */ >>>>>> +package org.apache.logging.log4j.core.async; >>>>>> + >>>>>> +import org.apache.logging.log4j.core.config.ConfigurationFactory; >>>>>> +import org.junit.BeforeClass; >>>>>> +import org.junit.Test; >>>>>> + >>>>>> +import static org.junit.Assert.assertNotNull; >>>>>> + >>>>>> +/** >>>>>> + * Test class loading deadlock condition from the LOG4J2-1457 >>>>>> + */ >>>>>> +public class AsyncLoggerClassLoadDeadlockTest { >>>>>> + >>>>>> + static final int RING_BUFFER_SIZE = 128; >>>>>> + >>>>>> + @BeforeClass >>>>>> + public static void beforeClass() { >>>>>> + System.setProperty("Log4jContextSelector", >>>>>> "org.apache.logging.log4j.core.async.AsyncLoggerContextSelector"); >>>>>> + System.setProperty("AsyncLogger.RingBufferSize", >>>>>> String.valueOf(RING_BUFFER_SIZE)); >>>>>> + >>>>>> System.setProperty(ConfigurationFactory.CONFIGURATION_FILE_PROPERTY, >>>>>> "AsyncLoggerConsoleTest.xml"); >>>>>> + } >>>>>> + >>>>>> + @Test(timeout = 30000) >>>>>> + public void testClassLoaderDeadlock() throws Exception { >>>>>> + //touch the class so static init will be called >>>>>> + final AsyncLoggerClassLoadDeadlock temp = new >>>>>> AsyncLoggerClassLoadDeadlock(); >>>>>> + assertNotNull(temp); >>>>>> + } >>>>>> +} >>>>>> >>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2 >>>>>> 84067cb/log4j-core/src/test/resources/AsyncLoggerConsoleTest.xml >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> diff --git a/log4j-core/src/test/resources/AsyncLoggerConsoleTest.xml >>>>>> b/log4j-core/src/test/resources/AsyncLoggerConsoleTest.xml >>>>>> new file mode 100644 >>>>>> index 0000000..90f4e9e >>>>>> --- /dev/null >>>>>> +++ b/log4j-core/src/test/resources/AsyncLoggerConsoleTest.xml >>>>>> @@ -0,0 +1,16 @@ >>>>>> +<?xml version="1.0" encoding="UTF-8"?> >>>>>> +<Configuration status="warn"> >>>>>> + <Appenders> >>>>>> + <Console name="Console" target="SYSTEM_OUT"> >>>>>> + <PatternLayout> >>>>>> + <pattern>%xEx{0}</pattern> >>>>>> + </PatternLayout> >>>>>> + </Console> >>>>>> + </Appenders> >>>>>> + >>>>>> + <Loggers> >>>>>> + <Root level="info"> >>>>>> + <AppenderRef ref="Console"/> >>>>>> + </Root> >>>>>> + </Loggers> >>>>>> +</Configuration> >>>>>> >>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2 >>>>>> 84067cb/src/changes/changes.xml >>>>>> ------------------------------------------------------------ >>>>>> ---------- >>>>>> diff --git a/src/changes/changes.xml b/src/changes/changes.xml >>>>>> index 2bfc311..fa53129 100644 >>>>>> --- a/src/changes/changes.xml >>>>>> +++ b/src/changes/changes.xml >>>>>> @@ -24,6 +24,9 @@ >>>>>> </properties> >>>>>> <body> >>>>>> <release version="2.7" date="2016-MM-DD" description="GA Release >>>>>> 2.7"> >>>>>> + <action issue="LOG4J2-1457" dev="mattsicker" type="fix" >>>>>> due-to="Leon Finker"> >>>>>> + Class loader deadlock when using async logging and extended >>>>>> stack trace pattern. >>>>>> + </action> >>>>>> <action issue="LOG4J2-1563" dev="ggregory" type="fix" >>>>>> due-to="Jason Tedor"> >>>>>> Log4j 2.6.2 can lose exceptions when a security manager is >>>>>> present. >>>>>> </action> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> E-Mail: [email protected] | [email protected] >>>>> Java Persistence with Hibernate, Second Edition >>>>> <http://www.manning.com/bauer3/> >>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >>>>> Spring Batch in Action <http://www.manning.com/templier/> >>>>> Blog: http://garygregory.wordpress.com >>>>> Home: http://garygregory.com/ >>>>> Tweet! http://twitter.com/GaryGregory >>>>> >>>> >>>> >>>> >>>> -- >>>> Matt Sicker <[email protected]> >>>> >>> >>> >>> >>> -- >>> E-Mail: [email protected] | [email protected] >>> Java Persistence with Hibernate, Second Edition >>> <http://www.manning.com/bauer3/> >>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >>> Spring Batch in Action <http://www.manning.com/templier/> >>> Blog: http://garygregory.wordpress.com >>> Home: http://garygregory.com/ >>> Tweet! http://twitter.com/GaryGregory >>> >> >> >> >> -- >> Matt Sicker <[email protected]> >> > > > > -- > E-Mail: [email protected] | [email protected] > Java Persistence with Hibernate, Second Edition > <http://www.manning.com/bauer3/> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> > Spring Batch in Action <http://www.manning.com/templier/> > Blog: http://garygregory.wordpress.com > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory > -- Matt Sicker <[email protected]>
