Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-beta9
    • Fix Version/s: None
    • Component/s: Core
    • Labels:
      None

      Description

      The problem is that some classes don't have class loaders: this can happen when working with dynamic JVM languages (Rhino, Jython, etc.)

      Here's the stack trace I got:

      java.lang.NullPointerException
      	at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.locateContext(ClassLoaderContextSelector.java:182)
      	at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:145)
      	at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:81)
      	at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:83)
      	at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:34)
      

        Activity

        Tal Liron created issue -
        Remko Popma made changes -
        Field Original Value New Value
        Assignee Remko Popma [ remkop@yahoo.com ]
        Hide
        Remko Popma added a comment -

        Fixed in revision 1552996.
        Please verify and close.

        Show
        Remko Popma added a comment - Fixed in revision 1552996. Please verify and close.
        Remko Popma made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Tal Liron added a comment - - edited

        I'm sorry, but this is still not fixed. Here is the new stack trace (built from TRUNK):

        java.lang.NullPointerException
        	at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.locateContext(ClassLoaderContextSelector.java:187)
        	at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:145)
        	at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:81)
        	at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:83)
        	at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:34)
        	at org.apache.logging.log4j.LogManager.getContext(LogManager.java:200)
        

        I changed line 187 to this:

        ClassLoader parent = loader != null ? loader.getParent() : null;
        

        You're also accessing "loader" in line 222! I changed it to this:

                    if (loader != null){
                      CONTEXT_MAP.putIfAbsent(loader.toString(), r);
                      ctx = CONTEXT_MAP.get(name).get().get();
                    }
        

        With my changes, the NPE disappeared – but I honestly don't understand this code, so i don't want to suggest that my fixes are correct.

        Show
        Tal Liron added a comment - - edited I'm sorry, but this is still not fixed. Here is the new stack trace (built from TRUNK): java.lang.NullPointerException at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.locateContext(ClassLoaderContextSelector.java:187) at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:145) at org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:81) at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:83) at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:34) at org.apache.logging.log4j.LogManager.getContext(LogManager.java:200) I changed line 187 to this: ClassLoader parent = loader != null ? loader.getParent() : null ; You're also accessing "loader" in line 222! I changed it to this: if (loader != null ){ CONTEXT_MAP.putIfAbsent(loader.toString(), r); ctx = CONTEXT_MAP.get(name).get().get(); } With my changes, the NPE disappeared – but I honestly don't understand this code, so i don't want to suggest that my fixes are correct.
        Tal Liron made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Tal Liron added a comment -

        Actually, I'm quite certain that my fix is not good: the NPE disappears, but it looks like the log4j configuration is loaded multiple times. I hope you can suggest a better fix.

        Show
        Tal Liron added a comment - Actually, I'm quite certain that my fix is not good: the NPE disappears, but it looks like the log4j configuration is loaded multiple times. I hope you can suggest a better fix.
        Hide
        Remko Popma added a comment - - edited

        Tal, thanks for checking!
        I've made the following change:

        private LoggerContext locateContext(final ClassLoader loaderOrNull, final URI configLocation) {
            final ClassLoader loader = loaderOrNull != null ? loaderOrNull : ClassLoader.getSystemClassLoader();
            final String name = loader.toString();
        ...
                CONTEXT_MAP.putIfAbsent(name, r);
                ctx = CONTEXT_MAP.get(name).get().get();
        ...
        

        This preserves the semantics of the original code: it ensure that the CONTEXT_MAP always has a value for the classloader name.

        The only assumption I'm making when fixing this bug is that if null is passed in as the classLoader, then the system class loader is an appropriate replacement. To be honest I am not 100% sure this is correct, but I figured it is better than a NullPointerException...

        Fixed in revision 1553262.
        Please verify and close.

        Show
        Remko Popma added a comment - - edited Tal, thanks for checking! I've made the following change: private LoggerContext locateContext( final ClassLoader loaderOrNull, final URI configLocation) { final ClassLoader loader = loaderOrNull != null ? loaderOrNull : ClassLoader .getSystemClassLoader(); final String name = loader.toString(); ... CONTEXT_MAP.putIfAbsent(name, r); ctx = CONTEXT_MAP.get(name).get().get(); ... This preserves the semantics of the original code: it ensure that the CONTEXT_MAP always has a value for the classloader name. The only assumption I'm making when fixing this bug is that if null is passed in as the classLoader, then the system class loader is an appropriate replacement. To be honest I am not 100% sure this is correct, but I figured it is better than a NullPointerException... Fixed in revision 1553262. Please verify and close.
        Remko Popma made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Tal Liron added a comment -

        NPE gone!

        Show
        Tal Liron added a comment - NPE gone!
        Tal Liron made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Cédric Champeau added a comment -

        The Groovy build is suffering this bug. Do you have any plan to release a new beta with this included? Thanks!

        Show
        Cédric Champeau added a comment - The Groovy build is suffering this bug. Do you have any plan to release a new beta with this included? Thanks!
        Hide
        Stefan Seifert added a comment -

        the same happens when using the the java.util.logging-to-slf4j bridge together with the slf4j-to-log4j bridge when trying to resolve the classloader of the latest stacktrace-element that does not belong to the slf4j-to-log4j bridge - its a class from java.util.logging which has not valid classloader attached.
        the current trunk version fixes this problem as well.

        Show
        Stefan Seifert added a comment - the same happens when using the the java.util.logging-to-slf4j bridge together with the slf4j-to-log4j bridge when trying to resolve the classloader of the latest stacktrace-element that does not belong to the slf4j-to-log4j bridge - its a class from java.util.logging which has not valid classloader attached. the current trunk version fixes this problem as well.

          People

          • Assignee:
            Remko Popma
            Reporter:
            Tal Liron
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development