geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevan Miller <kevan.mil...@gmail.com>
Subject BasicProxyManager.java commit r344848
Date Wed, 16 Nov 2005 16:44:30 GMT
Aaron,
Nice work getting to the bottom of this issue. However, I'm not sure that
I'm happy with your fix. I'm confident that your fix will find a
ClassLoaders which can load all of the classes/interfaces. However, there
can be multiple of these and there's no guarantee that you're finding the
most appropriate ClassLoader. For example imagine an application classloader
with inverseClassLoading set to true. You're technique might find a parent
ClassLoader, when the desired ClassLoader is the application classloader.

I have an alternate fix which calculates a child ClassLoader from the
potential list of ClassLoaders (my fix assumes that there is one ClassLoader
to which all other ClassLoaders are ancestors). I've tested against Joe's
test case. It too fixes the problem...

As I'm typing this, I'm wondering if we have an even larger problem. Is
there a guarantee that the list of ClassLoaders available to the
BasicProxyManager constructor contains a single ClassLoader which is capable
of loading <all> of the given classes? Seems pretty easy to construct a
scenario in which this is not true. I'd be interested in hearing what you or
others might think...

A problematic scenario would look like this:

System ClassLoader
/ | \
A B C
\ | /
My ClassLoader

Assume that ClassLoader A is the loader for class a, etc. If the classes a,
b, and c are passed to the BasicProxyManager constructor, it will not be
able to determine a ClassLoader capable of loading a, b, <and> c. Is this an
invalid use case? Neither of our fixes will work in this case...

--kevan


On 11/15/05, ammulder@apache.org <ammulder@apache.org> wrote:
>
> Author: ammulder
> Date: Tue Nov 15 18:32:31 2005
> New Revision: 344848
>
> URL: http://svn.apache.org/viewcvs?rev=344848&view=rev
> Log:
> Pick the best ClassLoader for the provided set of interfaces
> (Fixes GERONIMO-1064)
>
> Modified:
>
> geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/BasicProxyManager.java
>
> Modified:
> geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/BasicProxyManager.java
> URL:
> http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/BasicProxyManager.java?rev=344848&r1=344847&r2=344848&view=diff
>
> ==============================================================================
> ---
> geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/BasicProxyManager.java
> (original)
> +++
> geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/BasicProxyManager.java
> Tue Nov 15 18:32:31 2005
> @@ -207,6 +207,24 @@
> } else if(type.length == 1) { // Unlikely (as a result of
> GeronimoManagedBean)
> enhancer.setSuperclass(type[0]);
> } else {
> + ClassLoader best = null;
> + outer:
> + for (int i = 0; i < type.length; i++) {
> + ClassLoader test = type[i].getClassLoader();
> + for (int j = 0; j < type.length; j++) {
> + String className = type[j].getName();
> + try {
> + test.loadClass(className);
> + } catch (ClassNotFoundException e) {
> + continue outer;
> + }
> + }
> + best = test;
> + break;
> + }
> + if(best != null) {
> + enhancer.setClassLoader(best);
> + }
> if(type[0].isInterface()) {
> enhancer.setSuperclass(Object.class);
> enhancer.setInterfaces(type);
>
>
>

Mime
View raw message