Commons Logging
  1. Commons Logging
  2. LOGGING-13

[logging] NullPointException when Logger.getClassLoader returns null

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: Nightly Builds
    • Fix Version/s: 1.1.0
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: PC

      Description

      getClassLoader can return null to imply the Boot Class Loader (per JDK 1.3
      documentatio at least) however unfortunately commons logging is not coded to
      support that.

      There are at least two places where this is a problem & commons logging dies w/
      a NullPointerException, one in a Hashtable (used as a key to factory) [HashMap
      allows null key, Hashtable does not] and one other more directly.

      I modified the code to use the ClassLoader.getSystemClassLoader() when a null
      was returned for Logger.getClassLoader() – assuming ['cos I had no better
      guess/choice] that System == Boot [since there was no getBootClassLoader.] This
      appears to be working.

      I can send my modified code if interested.

      1. ASF.LICENSE.NOT.GRANTED--patch.txt
        6 kB
        Volker Leidl
      2. ASF.LICENSE.NOT.GRANTED--patch.txt
        2 kB
        Erik Erskine
      3. ASF.LICENSE.NOT.GRANTED--LogFactory.java
        21 kB
        Adam Jack
      4. ASF.LICENSE.NOT.GRANTED--LogFactory.java
        20 kB
        Adam Jack

        Activity

        Hide
        Craig McClanahan added a comment -

        This sounds like a good fix – could you post your changes as an attachment to
        this bug report, or in a mail message to COMMONS-DEV@JAKARTA.APACHE.ORG?

        Show
        Craig McClanahan added a comment - This sounds like a good fix – could you post your changes as an attachment to this bug report, or in a mail message to COMMONS-DEV@JAKARTA.APACHE.ORG?
        Hide
        Adam Jack added a comment -

        Created an attachment (id=2383)
        This is my hacked copy...

        Show
        Adam Jack added a comment - Created an attachment (id=2383) This is my hacked copy...
        Hide
        Adam Jack added a comment -

        Created an attachment (id=2384)
        This is the original I diff'ed

        Show
        Adam Jack added a comment - Created an attachment (id=2384) This is the original I diff'ed
        Hide
        Adam Jack added a comment -

        Diff of mine (Attached) with yours (Attached)

        445,452d445
        <
        <
        < if ( null == classLoader )
        <

        { < classLoader = ClassLoader.getSystemClassLoader(); < < }

        <
        483d475
        <
        503,516c495
        <

        { < ClassLoader local = LogFactory.class.getClassLoader(); < < if ( null != local ) < factory = (LogFactory) factories.get (LogFactory.class.getClassLoader()); < }

        <
        < if ( factory == null )
        <

        { < < factory = (LogFactory) factories.get(ClassLoader.getSystemClassLoader ()); < < }

        <

        > factory = (LogFactory) factories.get
        (LogFactory.class.getClassLoader());
        548,552d526
        <
        <
        < if (classLoader == null)
        < classLoader = ClassLoader.getSystemClassLoader();
        <

        Show
        Adam Jack added a comment - Diff of mine (Attached) with yours (Attached) 445,452d445 < < < if ( null == classLoader ) < { < classLoader = ClassLoader.getSystemClassLoader(); < < } < 483d475 < 503,516c495 < { < ClassLoader local = LogFactory.class.getClassLoader(); < < if ( null != local ) < factory = (LogFactory) factories.get (LogFactory.class.getClassLoader()); < } < < if ( factory == null ) < { < < factory = (LogFactory) factories.get(ClassLoader.getSystemClassLoader ()); < < } < — > factory = (LogFactory) factories.get (LogFactory.class.getClassLoader()); 548,552d526 < < < if (classLoader == null) < classLoader = ClassLoader.getSystemClassLoader(); <
        Hide
        Richard A. Sitze added a comment -

        Don't forget that commons-logger is targeted to JVM < 1.2,
        and getSystemClassLoader() is new for 1.2. Hmm...

        Show
        Richard A. Sitze added a comment - Don't forget that commons-logger is targeted to JVM < 1.2, and getSystemClassLoader() is new for 1.2. Hmm...
        Hide
        Richard A. Sitze added a comment -

        Let me clarify that last remark: commons-logging should execute on JDK 1.1.8+,
        so it must avoid code that will not execute in pre 1.2/1.3/1.4 environs.

        Show
        Richard A. Sitze added a comment - Let me clarify that last remark: commons-logging should execute on JDK 1.1.8+, so it must avoid code that will not execute in pre 1.2/1.3/1.4 environs.
        Hide
        Adam Jack added a comment -

        I see you have other code in commons that does introspection (or catches
        MethodNotFound) to deal with earlier JDKs. Could that not occur here?

        I assume you aren't saying it must crash for me on JDK 1.3 so it works for
        folks on JDK 1.1. Are you proposing something?

        Show
        Adam Jack added a comment - I see you have other code in commons that does introspection (or catches MethodNotFound) to deal with earlier JDKs. Could that not occur here? I assume you aren't saying it must crash for me on JDK 1.3 so it works for folks on JDK 1.1. Are you proposing something?
        Hide
        Richard A. Sitze added a comment -

        Take a look at commons.discovery. This will eventually replace the factory
        code in logging (at least that's my intent), so I'm going to assume that you
        would like the problem fixed there also

        Anyway, take a look at the class BootstrapLoader. I'd appreciate someone with
        a JDK 1.1.x environ putting together a test-case for this loader and verifying
        that it does what's expected. It SHOULD build in earlier JVM's, and function
        as you specified. For JDK 1.2 we could use a similar mechanism to get directly
        to the bootstrap loader, but that code wouldn't build in JDK 1.1.x... so I
        think your proposed solution + portable code is going to be about as good as it
        gets. Thanks for the proposed solution.

        BTW, IF you have time, it would be great if you used 'discovery' directly in
        your environment to get the LogFactory. This would verify for me that I've
        solved your problem and haven't introduced another...

        Use the following to get the LogFactory (note properties are protected, so I've
        put literals here):

        (LogFactory)ServiceFinder.find(LogFactory.class,
        LogFactory.class,
        "org.apache.commons.logging.impl.LogFactoryImpl",
        "commons-logging.properties");

        Show
        Richard A. Sitze added a comment - Take a look at commons.discovery. This will eventually replace the factory code in logging (at least that's my intent), so I'm going to assume that you would like the problem fixed there also Anyway, take a look at the class BootstrapLoader. I'd appreciate someone with a JDK 1.1.x environ putting together a test-case for this loader and verifying that it does what's expected. It SHOULD build in earlier JVM's, and function as you specified. For JDK 1.2 we could use a similar mechanism to get directly to the bootstrap loader, but that code wouldn't build in JDK 1.1.x... so I think your proposed solution + portable code is going to be about as good as it gets. Thanks for the proposed solution. BTW, IF you have time, it would be great if you used 'discovery' directly in your environment to get the LogFactory. This would verify for me that I've solved your problem and haven't introduced another... Use the following to get the LogFactory (note properties are protected, so I've put literals here): (LogFactory)ServiceFinder.find(LogFactory.class, LogFactory.class, "org.apache.commons.logging.impl.LogFactoryImpl", "commons-logging.properties");
        Hide
        Adam Jack added a comment -

        I clearly do not understand this bug form, I keep getting things out of thread
        sequence. Oh well...

        I would volunteer to help, in any way, it is certainly in my interest.
        Unfortunaly I leave Friday for vacation and I am no where near wrapped up, so
        know I won't have time before then.

        I don't know if it helps, but I've added it to my calendar to do upon my return
        August 1st.

        regards

        Adam

        Show
        Adam Jack added a comment - I clearly do not understand this bug form, I keep getting things out of thread sequence. Oh well... I would volunteer to help, in any way, it is certainly in my interest. Unfortunaly I leave Friday for vacation and I am no where near wrapped up, so know I won't have time before then. I don't know if it helps, but I've added it to my calendar to do upon my return August 1st. regards Adam
        Hide
        Richard A. Sitze added a comment -

        Just to keep the conversation going I'm moving this side comment from Axis
        (because I don't think it is relevant there, it's not constrained as commons-
        logging is):

        >> [If JDK 1.1 has not getSystemClassLoader I have to assume it does not return
        null.]

        Not true.

        JDK 1.1.x: Class.getClassLoader() == null == SystemClassLoader
        JDK 1.2+: Class.getClassLoader() == null == BootstrapClassLoader

        ... now how do you get the system classloader in JDK 1.1.x?
        ... and how do you get the bootstrap classloader in JDK 1.2+?

        I'm using the following class as my "SystemClassLoader" in Discovery:

        /**

        • JDK 1.1.x compatible?
        • There is no direct way to get the system class loader
        • in 1.1.x, so work around...
          */
          class SystemClassLoader extends ClassLoader {
          protected Class loadClass(String className, boolean resolve)
          throws ClassNotFoundException { return findSystemClass(className); }

        public URL getResource(String resName)

        { return getSystemResource(resName); }

        public InputStream getResourceAsStream(String resName)

        { return getSystemResourceAsStream(resName); }

        }

        Show
        Richard A. Sitze added a comment - Just to keep the conversation going I'm moving this side comment from Axis (because I don't think it is relevant there, it's not constrained as commons- logging is): >> [If JDK 1.1 has not getSystemClassLoader I have to assume it does not return null.] Not true. JDK 1.1.x: Class.getClassLoader() == null == SystemClassLoader JDK 1.2+: Class.getClassLoader() == null == BootstrapClassLoader ... now how do you get the system classloader in JDK 1.1.x? ... and how do you get the bootstrap classloader in JDK 1.2+? I'm using the following class as my "SystemClassLoader" in Discovery: /** JDK 1.1.x compatible? There is no direct way to get the system class loader in 1.1.x, so work around... */ class SystemClassLoader extends ClassLoader { protected Class loadClass(String className, boolean resolve) throws ClassNotFoundException { return findSystemClass(className); } public URL getResource(String resName) { return getSystemResource(resName); } public InputStream getResourceAsStream(String resName) { return getSystemResourceAsStream(resName); } }
        Hide
        Richard A. Sitze added a comment -

        I believe that this was corrected along with the issues raised in 12149.
        [quite possible the same issue, in the end].

            • This bug has been marked as a duplicate of 12149 ***
        Show
        Richard A. Sitze added a comment - I believe that this was corrected along with the issues raised in 12149. [quite possible the same issue, in the end] . This bug has been marked as a duplicate of 12149 ***
        Hide
        Volker Leidl added a comment -

        Created an attachment (id=3501)
        patch file including code patches and a new test case

        Show
        Volker Leidl added a comment - Created an attachment (id=3501) patch file including code patches and a new test case
        Hide
        Volker Leidl added a comment -

        This bug is still unsolved in the current CVS version (2002-10-17), so I decided
        to reopen it again.
        This bug occurs when the commons-logging classes are loaded by the bootstrap
        class loader and no context class loader is available.
        A patch as well as an appropriate test case is attached. This test makes use of
        the Java endorsed library mechanism to expose the critical classes to the boot
        strap class loader, so you will need a JDK that supports that mechanism to make
        the test work. I used Sun J2SE SDK 1.4.0 on a Windows NT 4.0 machine.

        The patch for LogFactory uses Class.forName() instead of
        Class.getClassLoader().loadClass(). AFAIK this should not make a difference.
        Furthermore this method is also available in pre 1.2 JDKs so it should not raise
        any compatibility issues.

        Here is the modified newFactory method:

        protected static LogFactory newFactory(String factoryClass,
        ClassLoader classLoader)
        throws LogConfigurationException {
        try {
        if (classLoader != null) {
        try

        { return (LogFactory) classLoader.loadClass(factoryClass) .newInstance(); }

        catch (ClassNotFoundException cnfx)

        { // continue }

        }

        // class loader is null, or Factory class could not be loaded
        // by means of the specified class loader at this point.

        // Let's try the class loader that loaded this class. If this
        // class is loaded by the bootstrap class loader
        // Class.getClassLoader() can return null according to the
        // contract of that method so we have to use Class.forName()
        // instead of Class.getClassLoader().loadClass().
        return (LogFactory) Class.forName(factoryClass).newInstance();

        } catch (Exception e)

        { throw new LogConfigurationException(e); }

        }

        To apply the patch enter the commons-logging directory and run "patch -Np0 <
        patch.txt".

        The patch applies changes to LogFactory.java and build.xml, and creates a new
        TestCase called BootStrapTest.java

        Show
        Volker Leidl added a comment - This bug is still unsolved in the current CVS version (2002-10-17), so I decided to reopen it again. This bug occurs when the commons-logging classes are loaded by the bootstrap class loader and no context class loader is available. A patch as well as an appropriate test case is attached. This test makes use of the Java endorsed library mechanism to expose the critical classes to the boot strap class loader, so you will need a JDK that supports that mechanism to make the test work. I used Sun J2SE SDK 1.4.0 on a Windows NT 4.0 machine. The patch for LogFactory uses Class.forName() instead of Class.getClassLoader().loadClass(). AFAIK this should not make a difference. Furthermore this method is also available in pre 1.2 JDKs so it should not raise any compatibility issues. Here is the modified newFactory method: protected static LogFactory newFactory(String factoryClass, ClassLoader classLoader) throws LogConfigurationException { try { if (classLoader != null) { try { return (LogFactory) classLoader.loadClass(factoryClass) .newInstance(); } catch (ClassNotFoundException cnfx) { // continue } } // class loader is null, or Factory class could not be loaded // by means of the specified class loader at this point. // Let's try the class loader that loaded this class. If this // class is loaded by the bootstrap class loader // Class.getClassLoader() can return null according to the // contract of that method so we have to use Class.forName() // instead of Class.getClassLoader().loadClass(). return (LogFactory) Class.forName(factoryClass).newInstance(); } catch (Exception e) { throw new LogConfigurationException(e); } } To apply the patch enter the commons-logging directory and run "patch -Np0 < patch.txt". The patch applies changes to LogFactory.java and build.xml, and creates a new TestCase called BootStrapTest.java
        Hide
        Volker Leidl added a comment -

        Oops, I mean "patch -Np2 < patch.txt". Furthermore the patch makes some changes
        to TestAll.java, which I just forgot to take out again

        Cheers,
        Volker

        Show
        Volker Leidl added a comment - Oops, I mean "patch -Np2 < patch.txt". Furthermore the patch makes some changes to TestAll.java, which I just forgot to take out again Cheers, Volker
        Hide
        Richard A. Sitze added a comment -

        Thanks for the education and the patch!
        Really appreciate when someone explains WHY the code is wrong

        Show
        Richard A. Sitze added a comment - Thanks for the education and the patch! Really appreciate when someone explains WHY the code is wrong
        Hide
        Richard A. Sitze added a comment -
            • COM-260 has been marked as a duplicate of this bug. ***
        Show
        Richard A. Sitze added a comment - COM-260 has been marked as a duplicate of this bug. ***
        Hide
        Richard A. Sitze added a comment -
            • COM-361 has been marked as a duplicate of this bug. ***
        Show
        Richard A. Sitze added a comment - COM-361 has been marked as a duplicate of this bug. ***
        Hide
        scott added a comment -

        It looks like this bug is back. I'm getting a NullPointerException in
        commons-logging-1.0.4 LogFactoryImpl.java line 374

        Show
        scott added a comment - It looks like this bug is back. I'm getting a NullPointerException in commons-logging-1.0.4 LogFactoryImpl.java line 374
        Hide
        rdonkin@apache.org added a comment -

        I've just committed a patch which should fix this problem as well.

        Robert

        Show
        rdonkin@apache.org added a comment - I've just committed a patch which should fix this problem as well. Robert
        Hide
        Ernest E Vogelsinger added a comment -

        Hithere,

        using commons logging 1.0.4 this bug seems to be (again? still?) in production,
        see code snippet below, lines 374/375. As soon as you have the commons in your
        bootstrap classpath you're doomed, as "this.getClass().getClassLoader()" will
        return null (JVM 1.4.2).

        // from LogFactoryImpl.java
        360: protected Constructor getLogConstructor()
        361: throws LogConfigurationException {
        362:
        363: // Return the previously identified Constructor (if any)
        364: if (logConstructor != null)

        { 365: return logConstructor; 366: }

        367:
        368: String logClassName = getLogClassName();
        369:
        370: // Attempt to load the Log implementation class
        371: Class logClass = null;
        372: Class logInterface = null;
        373: try {
        374: logInterface = this.getClass().getClassLoader().loadClass
        375: (LOG_INTERFACE);

        org.apache.commons.logging.LogConfigurationException:
        org.apache.commons.logging.LogConfigurationException:
        java.lang.NullPointerException (Caused by java.lang.NullPointerException)
        (Caused by org.apache.commons.logging.LogConfigurationException:
        java.lang.NullPointerException (Caused by java.lang.NullPointerException))
        at org.apache.commons.logging.impl.LogFactoryImpl.newInstance
        (LogFactoryImpl.java:543)
        at org.apache.commons.logging.impl.LogFactoryImpl.getInstance
        (LogFactoryImpl.java:235)
        at org.apache.commons.logging.LogFactory.getLog(LogFactory.java:370)
        at org.apache.commons.digester.Digester.<init>(Digester.java:346)
        at Main.main(Main.java:53)
        Caused by: org.apache.commons.logging.LogConfigurationException:
        java.lang.NullPointerException (Caused by java.lang.NullPointerException)
        at org.apache.commons.logging.impl.LogFactoryImpl.getLogConstructor
        (LogFactoryImpl.java:397)
        at org.apache.commons.logging.impl.LogFactoryImpl.newInstance
        (LogFactoryImpl.java:529)
        ... 4 more
        Caused by: java.lang.NullPointerException
        at org.apache.commons.logging.impl.LogFactoryImpl.getLogConstructor
        (LogFactoryImpl.java:374)
        ... 5 more
        Exception in thread "main"

        Show
        Ernest E Vogelsinger added a comment - Hithere, using commons logging 1.0.4 this bug seems to be (again? still?) in production, see code snippet below, lines 374/375. As soon as you have the commons in your bootstrap classpath you're doomed, as "this.getClass().getClassLoader()" will return null (JVM 1.4.2). // from LogFactoryImpl.java 360: protected Constructor getLogConstructor() 361: throws LogConfigurationException { 362: 363: // Return the previously identified Constructor (if any) 364: if (logConstructor != null) { 365: return logConstructor; 366: } 367: 368: String logClassName = getLogClassName(); 369: 370: // Attempt to load the Log implementation class 371: Class logClass = null; 372: Class logInterface = null; 373: try { 374: logInterface = this.getClass().getClassLoader().loadClass 375: (LOG_INTERFACE); org.apache.commons.logging.LogConfigurationException: org.apache.commons.logging.LogConfigurationException: java.lang.NullPointerException (Caused by java.lang.NullPointerException) (Caused by org.apache.commons.logging.LogConfigurationException: java.lang.NullPointerException (Caused by java.lang.NullPointerException)) at org.apache.commons.logging.impl.LogFactoryImpl.newInstance (LogFactoryImpl.java:543) at org.apache.commons.logging.impl.LogFactoryImpl.getInstance (LogFactoryImpl.java:235) at org.apache.commons.logging.LogFactory.getLog(LogFactory.java:370) at org.apache.commons.digester.Digester.<init>(Digester.java:346) at Main.main(Main.java:53) Caused by: org.apache.commons.logging.LogConfigurationException: java.lang.NullPointerException (Caused by java.lang.NullPointerException) at org.apache.commons.logging.impl.LogFactoryImpl.getLogConstructor (LogFactoryImpl.java:397) at org.apache.commons.logging.impl.LogFactoryImpl.newInstance (LogFactoryImpl.java:529) ... 4 more Caused by: java.lang.NullPointerException at org.apache.commons.logging.impl.LogFactoryImpl.getLogConstructor (LogFactoryImpl.java:374) ... 5 more Exception in thread "main"
        Hide
        Simon Kitching added a comment -

        Release 1.0.4 was made from revision 139045 on 2004-06-11.
        This bug was fixed in revision 139052 on 2004-10-17.
        And this bugzilla entry was closed on that same date.

        So the bug is fixed; there just hasn't been an official release since the fix
        went in. If you pull the latest code from SVN and compile that, you should get a
        fixed JCL - though of course you'll get some other changes too.

        Still, I presume that if you're putting JCL into the bootclasspath, you're doing
        embedded systems work? In that case, distributing a non-official version of JCL
        shouldn't bother you. I would recommend pulling the code from the LOGGING_1_0_4
        tagdir and applying the fix from r139052 only, in order to avoid getting any
        "experimental" code.

        Show
        Simon Kitching added a comment - Release 1.0.4 was made from revision 139045 on 2004-06-11. This bug was fixed in revision 139052 on 2004-10-17. And this bugzilla entry was closed on that same date. So the bug is fixed; there just hasn't been an official release since the fix went in. If you pull the latest code from SVN and compile that, you should get a fixed JCL - though of course you'll get some other changes too. Still, I presume that if you're putting JCL into the bootclasspath, you're doing embedded systems work? In that case, distributing a non-official version of JCL shouldn't bother you. I would recommend pulling the code from the LOGGING_1_0_4 tagdir and applying the fix from r139052 only, in order to avoid getting any "experimental" code.
        Hide
        Erik Erskine added a comment -

        Created an attachment (id=14974)
        changes LogFactory.getFactory to return same instance if classloader is null

        I'm experiencing similar issues on Jeode (basically 1.1.8) where
        getClassLoader() is returing null.

        When this is the case LogFactory.getFactory() returns a different instance of
        LogFactory each time it is called. This means I cannot call setAttribute on a
        LogFactory and expect future calls to getLog to use the same instance.

        LogFactory uses a map ("factories") of
        ClassLoader->LogFactory. Where ClassLoader == null it cannot be used as a key
        in this map, therefore previously created instances are not found.

        I have included a patch to fix this problem. It adds an additional static
        variable "nullClassLoaderFactory" that performs the same function as the
        factories map, but for cases where ClassLoader is null. If the factories map
        is accessed and the ClassLoader is null this variable can be used instead.

        This is against svn trunk.

        Show
        Erik Erskine added a comment - Created an attachment (id=14974) changes LogFactory.getFactory to return same instance if classloader is null I'm experiencing similar issues on Jeode (basically 1.1.8) where getClassLoader() is returing null. When this is the case LogFactory.getFactory() returns a different instance of LogFactory each time it is called. This means I cannot call setAttribute on a LogFactory and expect future calls to getLog to use the same instance. LogFactory uses a map ("factories") of ClassLoader->LogFactory. Where ClassLoader == null it cannot be used as a key in this map, therefore previously created instances are not found. I have included a patch to fix this problem. It adds an additional static variable "nullClassLoaderFactory" that performs the same function as the factories map, but for cases where ClassLoader is null. If the factories map is accessed and the ClassLoader is null this variable can be used instead. This is against svn trunk.
        Hide
        Simon Kitching added a comment -

        (In reply to comment #22)
        > changes LogFactory.getFactory to return same instance if classloader is null

        Thanks for the patch Erik. It has been committed to SVN trunk.

        Show
        Simon Kitching added a comment - (In reply to comment #22) > changes LogFactory.getFactory to return same instance if classloader is null Thanks for the patch Erik. It has been committed to SVN trunk.
        Hide
        Simon Kitching added a comment -

        As there has been no feedback in the last 21 days, I will assume the patch
        worked and am closing the issue.

        Show
        Simon Kitching added a comment - As there has been no feedback in the last 21 days, I will assume the patch worked and am closing the issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Adam Jack
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development