You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Jan Luehe <Ja...@Sun.COM> on 2005/09/29 22:57:15 UTC

Classloading regression between commons-digester-1.0 and later versions

This may be an old topic, but it seems a classloading regression was
introduced between commons-digester-1.0 and later versions (including
the most recent).

In commons-digester-1.0, a class was loaded as follows:

    Class clazz = null;

    // Check to see if the context class loader is set,
    // and if so, use it, as it may be set in server-side
    // environments and Class.forName() may cause issues
    ClassLoader ctxLoader =
        Thread.currentThread().getContextClassLoader();
    if (ctxLoader == null) {
        clazz = Class.forName(realClassName);
    } else {
        clazz = ctxLoader.loadClass(realClassName);
    }

that is, if a context classloader had been set, it was used
(unconditionally).

In contrast, commons-digester-1.1 (and up) loads a class as follows:

    Class clazz = digester.getClassLoader().loadClass(realClassName);

where Digester.getClassLoader() is implemented as follows:

    /**
     * Return the class loader to be used for instantiating application
objects
     * when required.  This is determined based upon the following rules:
     * <ul>
     * <li>The class loader set by <code>setClassLoader()</code>, if
any</li>
     * <li>The thread context class loader, if it exists and the
     *     <code>useContextClassLoader</code> property is set to true</li>
     * <li>The class loader used to load the Digester class itself.
     * </ul>
     */
    public ClassLoader getClassLoader() {

        if (this.classLoader != null)
            return (this.classLoader);
        if (this.useContextClassLoader) {
            ClassLoader classLoader =
                Thread.currentThread().getContextClassLoader();
            if (classLoader != null)
                return (classLoader);
        }
        return (this.getClass().getClassLoader());

    }

that is, the context classloader is used only if it exists *AND*
the "useContextClassLoader" property has been set to TRUE. Unfortunately,
"useContextClassLoader" is FALSE by default, preventing the digester to
load any classes further down in the classloading hierarchy (e.g., in
WEB-INF/lib).

Can someone explain to me the motivation for setting the
"useContextClassLoader" property to FALSE by default?

We can easily restore the commons-digester-1.0 behaviour in our own
code by setting "useContextClassLoader" to TRUE on the Digester
instances we acquire. However, this is not possible on "foreign" code
that we bundle.

It would be useful if Digester.getClassLoader() would also consider a
system property (in addition to its own "useContextClassLoader"
property) when determining whether to use the context classloader.

Any comments appreciated.


Jan




---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] Re: Classloading regression between commons-digester-1.0 and later versions

Posted by Jan Luehe <Ja...@Sun.COM>.
Hi Robert,

robert burrell donkin wrote On 10/01/05 11:11,:
> hi jan
> 
> please remember to add the subject prefix for the component.

will remember from now on! :)

> On Thu, 2005-09-29 at 13:57 -0700, Jan Luehe wrote:
> 
>>This may be an old topic, but it seems a classloading regression was
>>introduced between commons-digester-1.0 and later versions (including
>>the most recent).
> 
> 
> this is indeed a very old topic :)
> 
> <snip>
> 
>>Can someone explain to me the motivation for setting the
>>"useContextClassLoader" property to FALSE by default?
> 
> 
> i can offer you nothing more than the logs: it is set to FALSE by
> default for no very good reason but most likely since it was felt
> (rightly or wrongly) to preserve backwards compatibility.
> 
> 
>>We can easily restore the commons-digester-1.0 behaviour in our own
>>code by setting "useContextClassLoader" to TRUE on the Digester
>>instances we acquire. However, this is not possible on "foreign" code
>>that we bundle.
>>
>>It would be useful if Digester.getClassLoader() would also consider a
>>system property (in addition to its own "useContextClassLoader"
>>property) when determining whether to use the context classloader.
> 
> 
> +1
> 
> can anyone think of any reasons not to add this?

The only problem with adding a system property for this purpose
is that it would require granting the commons-digester jar the
permission to read it. It a web application bundles commons-digester
locally, it will not have this permission in most cases.

Perhaps it is safer to stick to the current "useContextClassLoader"
property and require that web applications always bundle
commons-digester, in which case the classloader that loaded
Digester.class will correspond to the web application's classloader,
which has access to all classes under WEB-INF/lib[classes].

Thanks,


Jan


> 
>>Any comments appreciated.
> 
> 
> submit a patch :)
> 
> - robert
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


[digester] Re: Classloading regression between commons-digester-1.0 and later versions

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
hi jan

please remember to add the subject prefix for the component.

On Thu, 2005-09-29 at 13:57 -0700, Jan Luehe wrote:
> This may be an old topic, but it seems a classloading regression was
> introduced between commons-digester-1.0 and later versions (including
> the most recent).

this is indeed a very old topic :)

<snip>

> Can someone explain to me the motivation for setting the
> "useContextClassLoader" property to FALSE by default?

i can offer you nothing more than the logs: it is set to FALSE by
default for no very good reason but most likely since it was felt
(rightly or wrongly) to preserve backwards compatibility.

> We can easily restore the commons-digester-1.0 behaviour in our own
> code by setting "useContextClassLoader" to TRUE on the Digester
> instances we acquire. However, this is not possible on "foreign" code
> that we bundle.
> 
> It would be useful if Digester.getClassLoader() would also consider a
> system property (in addition to its own "useContextClassLoader"
> property) when determining whether to use the context classloader.

+1

can anyone think of any reasons not to add this?

> Any comments appreciated.

submit a patch :)

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org