Return-Path: Delivered-To: apmail-geronimo-dev-archive@www.apache.org Received: (qmail 17969 invoked from network); 16 Nov 2005 16:44:57 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 16 Nov 2005 16:44:57 -0000 Received: (qmail 66825 invoked by uid 500); 16 Nov 2005 16:44:53 -0000 Delivered-To: apmail-geronimo-dev-archive@geronimo.apache.org Received: (qmail 66787 invoked by uid 500); 16 Nov 2005 16:44:53 -0000 Mailing-List: contact dev-help@geronimo.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: dev@geronimo.apache.org List-Id: Delivered-To: mailing list dev@geronimo.apache.org Received: (qmail 66758 invoked by uid 99); 16 Nov 2005 16:44:53 -0000 Received: from asf.osuosl.org (HELO asf.osuosl.org) (140.211.166.49) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 16 Nov 2005 08:44:53 -0800 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests=HTML_MESSAGE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (asf.osuosl.org: domain of kevan.miller@gmail.com designates 64.233.182.204 as permitted sender) Received: from [64.233.182.204] (HELO nproxy.gmail.com) (64.233.182.204) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 16 Nov 2005 08:46:26 -0800 Received: by nproxy.gmail.com with SMTP id i2so414585nfe for ; Wed, 16 Nov 2005 08:44:30 -0800 (PST) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:mime-version:content-type; b=Ke7LPg1we2KJa9u7cnZYsyWQeGxdmfFCtAtgClAmNNdbb4HsFbds8VljqWqnwJH0mTR+SU+tHBa2y1cJDHmnRJ/RyAO3oL9l/fmov2HZos1hDubgvi/x6H6LRbSfuRDp89ZMg9kBp4Z/gxu+KJrE/1/vxspeY/Hm8d0HHFXI2Z8= Received: by 10.48.236.15 with SMTP id j15mr318465nfh; Wed, 16 Nov 2005 08:44:30 -0800 (PST) Received: by 10.48.224.18 with HTTP; Wed, 16 Nov 2005 08:44:30 -0800 (PST) Message-ID: Date: Wed, 16 Nov 2005 11:44:30 -0500 From: Kevan Miller To: dev@geronimo.apache.org Subject: BasicProxyManager.java commit r344848 MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_8125_31033540.1132159470403" X-Virus-Checked: Checked by ClamAV on apache.org X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N ------=_Part_8125_31033540.1132159470403 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline 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 classloade= r 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 ClassLoade= r 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 capabl= e of loading 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 o= r 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, c. Is this a= n invalid use case? Neither of our fixes will work in this case... --kevan On 11/15/05, ammulder@apache.org wrote: > > Author: ammulder > Date: Tue Nov 15 18:32:31 2005 > New Revision: 344848 > > URL: http://svn.apache.org/viewcvs?rev=3D344848&view=3Drev > 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/B= asicProxyManager.java > > Modified: > geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/B= asicProxyManager.java > URL: > http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/= apache/geronimo/kernel/basic/BasicProxyManager.java?rev=3D344848&r1=3D34484= 7&r2=3D344848&view=3Ddiff > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- > geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/B= asicProxyManager.java > (original) > +++ > geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/B= asicProxyManager.java > Tue Nov 15 18:32:31 2005 > @@ -207,6 +207,24 @@ > } else if(type.length =3D=3D 1) { // Unlikely (as a result of > GeronimoManagedBean) > enhancer.setSuperclass(type[0]); > } else { > + ClassLoader best =3D null; > + outer: > + for (int i =3D 0; i < type.length; i++) { > + ClassLoader test =3D type[i].getClassLoader(); > + for (int j =3D 0; j < type.length; j++) { > + String className =3D type[j].getName(); > + try { > + test.loadClass(className); > + } catch (ClassNotFoundException e) { > + continue outer; > + } > + } > + best =3D test; > + break; > + } > + if(best !=3D null) { > + enhancer.setClassLoader(best); > + } > if(type[0].isInterface()) { > enhancer.setSuperclass(Object.class); > enhancer.setInterfaces(type); > > > ------=_Part_8125_31033540.1132159470403 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline 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
            / &= nbsp;    |     \
           A  &= nbsp;  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> wro= te:
Author: am= mulder
Date: Tue Nov 15 18:32:31 2005
New Revision: 344848

URL= :=20 http:= //svn.apache.org/viewcvs?rev=3D344848&view=3Drev
Log:
Pick th= e 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

Modifie= d: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/= BasicProxyManager.java
URL: http://svn.apach= e.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/ke= rnel/basic/BasicProxyManager.java?rev=3D344848&r1=3D344847&r2=3D344= 848&view=3Ddiff
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D
--- geronimo/trunk/modules/kernel/src/java/org/apache= /geronimo/kernel/basic/BasicProxyManager.java (original)
+++ geronimo/tr= unk/modules/kernel/src/java/org/apache/geronimo/kernel/basic/BasicProxyMana= ger.java Tue Nov 15 18:32:31 2005
@@ -207,6 +207,24 @@
        = ;     } else if(type.length =3D=3D 1) { // Unlikely (as a result of GeronimoManagedBean)
        &nb= sp;        enhancer.setSuperclass(type[0]);
      &nb= sp;      } else {
+    &nbs= p;           ClassLo= ader best =3D null;
+        &nb= sp;       outer:
+   &= nbsp;           &nbs= p;for (int i =3D 0; i < type.length; i++) {
+     =             &nb= sp;  ClassLoader test =3D type[i].getClassLoader();
+      =             &nb= sp; for (int j =3D 0; j < type.length; j++) {
+     =             &nb= sp;      String className =3D type[j].getName();
+      &n= bsp;            = ;     try {
+           &nb= sp;            =     test.loadClass(className);
+   &n= bsp;            = ;        } catch (ClassNotFoundException e) {
+      =             &nb= sp;         continue outer;
+          &nbs= p;            &= nbsp;}
+          &nbs= p;         }
+  &= nbsp;           &nbs= p;     best =3D test;
+          &= nbsp;         break;
+ = ;            &n= bsp;  }
+         = ;       if(best !=3D null) {
+ &= nbsp;           &nbs= p;      enhancer.setClassLoader(best);
+&n= bsp;            = ;   }
        &nb= sp;        if(type[0].isInterface()) {
            = ;         enhancer.setSuperclass(Object.class);
     &nbs= p;            &= nbsp;  enhancer.setInterfaces(type);



------=_Part_8125_31033540.1132159470403--