Will Jaynes wrote:




robert burrell donkin wrote:
...

2, a more sophisticated implementation for the loading of the LogFactory implementation that would try to load the class from the LogFactory class classloader if the assignment fails. since (at the moment) we throw an exception in this circumstance (from would be very difficult to recover from), i don't think that add this extra step would do any harm.

comments anyone?


I agree with Norbert that the central remaining classloading problem is that the Log interface may not have been loaded by the same classloader as the log implementation, so the isAssignableFrom test fails. To be consistent, the Log interface should be loaded by the loadClass() method, just as the logClass is. Here's my suggested code change from LogFactoryImpl.getLogConstructor():

    // Attempt to load the Log implementation class
    Class logClass = null;
    Class logInterface = null;
    try {
        logClass = loadClass(logClassName);
        logInterface = loadClass("org.apache.commons.logging.Log");
        if (logClass == null) {
            throw new LogConfigurationException
                ("No suitable Log implementation for " + logClassName);
        }
        if (logInterface == null) {
            throw new LogConfigurationException
                ("Log interface not found by classloader");
        }
        if (!logInterface.isAssignableFrom(logClass)) {
            throw new LogConfigurationException
                ("Class " + logClassName + " does not implement Log");
        }
    } catch (Throwable t) {
        throw new LogConfigurationException(t);
    }

It is certainly possible to also add Robert's suggestion of trying the LogFactory class classloader if the initial load fails, but I don't think that is necessary.

Will

Well, I submitted this as a bug and included a patch. But Remy immediately marked it as RESOLVED/WONTFIX. He doesn't really address my suggestion other than to say that c-l "if used properly ... is well thought out, and works perfectly well" I would never suggest that c-l is not well thought out. However, I don't think that all the problems we see involving c-l classloading can be attributed to us not using c-l "properly". It may work perfectly well with Tomcat5, but that doesn't mean there aren't improvements that can be made. Or even accomodations that could be made to other containers than Tomcat5.


Could someone please explain why the suggestion to load the Log interface with the loadClass() method is not acceptable.

Will




--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to