You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Gary Gregory <ga...@gmail.com> on 2016/09/08 16:09:34 UTC

Re: logging-log4j2 git commit: Fix class loader deadlock when using async logging and extended stack trace pattern

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, <ma...@apache.org> 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/284067cb
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/284067cb
>
> Branch: refs/heads/master
> Commit: 284067cbdb5f13c41225e22c2a60d79cd43f1383
> Parents: 99c0d01
> Author: Matt Sicker <bo...@gmail.com>
> Authored: Thu Sep 8 10:30:12 2016 -0500
> Committer: Matt Sicker <bo...@gmail.com>
> 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/
> 284067cb/log4j-core/src/main/java/org/apache/logging/log4j/
> core/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/
> impl/ThrowableProxy.java
> index a052e4d..e12e14a 100644
> --- 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/
> impl/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/
> 284067cb/log4j-core/src/test/java/org/apache/logging/log4j/core/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/async/AsyncLoggerClassLoadDeadlock.java
> new file mode 100644
> index 0000000..f8341f9
> --- /dev/null
> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/
> 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/
> 284067cb/log4j-core/src/test/java/org/apache/logging/log4j/core/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/async/AsyncLoggerClassLoadDeadlockTest.java
> new file mode 100644
> index 0000000..990312d
> --- /dev/null
> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/
> 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/
> 284067cb/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/
> 284067cb/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: garydgregory@gmail.com | ggregory@apache.org
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

Re: logging-log4j2 git commit: Fix class loader deadlock when using async logging and extended stack trace pattern

Posted by Gary Gregory <ga...@gmail.com>.
Should they have the same name?

Gary

On Thu, Sep 8, 2016 at 11:24 AM, Matt Sicker <bo...@gmail.com> wrote:

> 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 <ga...@gmail.com> 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 <bo...@gmail.com> wrote:
>>
>>> Which ones?
>>>
>>> On 8 September 2016 at 12:27, Gary Gregory <ga...@gmail.com>
>>> wrote:
>>>
>>>> Good to know. Can the Javadocs be improved?
>>>>
>>>> Gary
>>>>
>>>> On Thu, Sep 8, 2016 at 9:39 AM, Matt Sicker <bo...@gmail.com> 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 <ga...@gmail.com>
>>>>> 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, <ma...@apache.org> 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 <bo...@gmail.com>
>>>>>>> Authored: Thu Sep 8 10:30:12 2016 -0500
>>>>>>> Committer: Matt Sicker <bo...@gmail.com>
>>>>>>> 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(Configurati
>>>>>>> onFactory.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: garydgregory@gmail.com | ggregory@apache.org
>>>>>> 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 <bo...@gmail.com>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>> 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 <bo...@gmail.com>
>>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> 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 <bo...@gmail.com>
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
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

Re: logging-log4j2 git commit: Fix class loader deadlock when using async logging and extended stack trace pattern

Posted by Matt Sicker <bo...@gmail.com>.
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 <ga...@gmail.com> 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 <bo...@gmail.com> wrote:
>
>> Which ones?
>>
>> On 8 September 2016 at 12:27, Gary Gregory <ga...@gmail.com>
>> wrote:
>>
>>> Good to know. Can the Javadocs be improved?
>>>
>>> Gary
>>>
>>> On Thu, Sep 8, 2016 at 9:39 AM, Matt Sicker <bo...@gmail.com> 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 <ga...@gmail.com>
>>>> 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, <ma...@apache.org> 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 <bo...@gmail.com>
>>>>>> Authored: Thu Sep 8 10:30:12 2016 -0500
>>>>>> Committer: Matt Sicker <bo...@gmail.com>
>>>>>> 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: garydgregory@gmail.com | ggregory@apache.org
>>>>> 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 <bo...@gmail.com>
>>>>
>>>
>>>
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>> 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 <bo...@gmail.com>
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> 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 <bo...@gmail.com>

Re: logging-log4j2 git commit: Fix class loader deadlock when using async logging and extended stack trace pattern

Posted by Gary Gregory <ga...@gmail.com>.
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 <bo...@gmail.com> wrote:

> Which ones?
>
> On 8 September 2016 at 12:27, Gary Gregory <ga...@gmail.com> wrote:
>
>> Good to know. Can the Javadocs be improved?
>>
>> Gary
>>
>> On Thu, Sep 8, 2016 at 9:39 AM, Matt Sicker <bo...@gmail.com> 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 <ga...@gmail.com>
>>> 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, <ma...@apache.org> 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 <bo...@gmail.com>
>>>>> Authored: Thu Sep 8 10:30:12 2016 -0500
>>>>> Committer: Matt Sicker <bo...@gmail.com>
>>>>> 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: garydgregory@gmail.com | ggregory@apache.org
>>>> 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 <bo...@gmail.com>
>>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> 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 <bo...@gmail.com>
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
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

Re: logging-log4j2 git commit: Fix class loader deadlock when using async logging and extended stack trace pattern

Posted by Matt Sicker <bo...@gmail.com>.
Which ones?

On 8 September 2016 at 12:27, Gary Gregory <ga...@gmail.com> wrote:

> Good to know. Can the Javadocs be improved?
>
> Gary
>
> On Thu, Sep 8, 2016 at 9:39 AM, Matt Sicker <bo...@gmail.com> 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 <ga...@gmail.com>
>> 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, <ma...@apache.org> 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 <bo...@gmail.com>
>>>> Authored: Thu Sep 8 10:30:12 2016 -0500
>>>> Committer: Matt Sicker <bo...@gmail.com>
>>>> 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/asy
>>>> nc/AsyncLoggerClassLoadDeadlock.java b/log4j-core/src/test/java/org
>>>> /apache/logging/log4j/core/async/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/asy
>>>> nc/AsyncLoggerClassLoadDeadlockTest.java b/log4j-core/src/test/java/org
>>>> /apache/logging/log4j/core/async/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: garydgregory@gmail.com | ggregory@apache.org
>>> 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 <bo...@gmail.com>
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> 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 <bo...@gmail.com>

Re: logging-log4j2 git commit: Fix class loader deadlock when using async logging and extended stack trace pattern

Posted by Gary Gregory <ga...@gmail.com>.
Good to know. Can the Javadocs be improved?

Gary

On Thu, Sep 8, 2016 at 9:39 AM, Matt Sicker <bo...@gmail.com> 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 <ga...@gmail.com> 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, <ma...@apache.org> 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 <bo...@gmail.com>
>>> Authored: Thu Sep 8 10:30:12 2016 -0500
>>> Committer: Matt Sicker <bo...@gmail.com>
>>> 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/asy
>>> nc/AsyncLoggerClassLoadDeadlock.java b/log4j-core/src/test/java/org
>>> /apache/logging/log4j/core/async/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/asy
>>> nc/AsyncLoggerClassLoadDeadlockTest.java b/log4j-core/src/test/java/org
>>> /apache/logging/log4j/core/async/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: garydgregory@gmail.com | ggregory@apache.org
>> 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 <bo...@gmail.com>
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
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

Re: logging-log4j2 git commit: Fix class loader deadlock when using async logging and extended stack trace pattern

Posted by Matt Sicker <bo...@gmail.com>.
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 <ga...@gmail.com> 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, <ma...@apache.org> 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/284067cb
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/284067cb
>>
>> Branch: refs/heads/master
>> Commit: 284067cbdb5f13c41225e22c2a60d79cd43f1383
>> Parents: 99c0d01
>> Author: Matt Sicker <bo...@gmail.com>
>> Authored: Thu Sep 8 10:30:12 2016 -0500
>> Committer: Matt Sicker <bo...@gmail.com>
>> 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/asy
>> nc/AsyncLoggerClassLoadDeadlock.java b/log4j-core/src/test/java/org
>> /apache/logging/log4j/core/async/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/asy
>> nc/AsyncLoggerClassLoadDeadlockTest.java b/log4j-core/src/test/java/org
>> /apache/logging/log4j/core/async/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: garydgregory@gmail.com | ggregory@apache.org
> 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 <bo...@gmail.com>