geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron Mulder <ammul...@alumni.princeton.edu>
Subject Re: BasicProxyManager.java commit r344848
Date Wed, 16 Nov 2005 16:54:43 GMT
On 11/16/05, Kevan Miller <kevan.miller@gmail.com> wrote:
> 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.

For my part, I don't really care which class loader is "ideal" so long
as we try to get one that can load all the classes.  I'm not too
concerned that parent and child may be using different definitions of
the same class such that a different "master" CL might result in using
a different definition.  We're only talking about GBean interfaces
here, and I think unlikely to have different versions in use by
different CLs (at present).  Can you think of a specific use case in
Geronimo where we might run into a problem?  I know David J is working
on fully versioned configurations, which might make a difference.

>  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...

Right, there's no guarantee this will solve all possible problems. 
Right now it just falls through if it can't identify a "master" CL.  I
suppose we could create a new multi-parent CL on the spot if we needed
to.

Aaron

>  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