harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexey Varlamov" <alexey.v.varla...@gmail.com>
Subject Re: svn commit: r477583 - in /harmony/enhanced/drlvm/trunk/vm: tests/kernel/java/lang/ClassLoaderTest.java vmcore/src/kernel_classes/javasrc/java/lang/ClassLoader.java
Date Wed, 22 Nov 2006 09:45:26 GMT
2006/11/22, Alexey Varlamov <alexey.v.varlamov@gmail.com>:
> Yep, I did it again - commit without explaining/discussing enough...
>
> OK, on the matter now : the root cause of failed verification is
> classloading failure, see the excerpt from trace below:
> -----------------------------------------
> [0x590] classloader.loading : StartLoading class
> org/eclipse/core/runtime/IStatus with loader 033C6460
> [0x590] class : collision error: class
> org/eclipse/core/runtime/IStatus is defined second time
> [0x590] class : Loading of org/eclipse/core/runtime/IStatus class
> failed due to java/lang/LinkageError
> [0x590] classloader : Failed loading class
> org/eclipse/core/runtime/IStatus with loader 04278B78
> [0x590] root : vf_debug: verifying class
> org/eclipse/core/internal/registry/RegistryProperties (method
> getContextProperty(Ljava/lang/String;)Ljava/lang/String;) couldn't
> load class "org/eclipse/core/runtime/IStatus"
> [0x590] root : vf_debug: VerifyError: org/eclipse/core/runtime/IStatus
> -----------------------------------------
>
> Why this happens? The VM already have this class loaded and some
> loader Ld is registered as defining loader for it. This registration
> is basically done on VM side, in native loader structure, see also
> discussion [1]. We had parallel registration mechanism on Java side,
> in kernel j.l.ClassLoader, which actually remained from experimental
> "Java class registry" implementation (which become obsolete after
> [1]). It worked fine for classical delegation mechanism, but can be
> easily bypassed if Ld overrides loadClass() method from scratch,
> without referering to standard implementation super.loadClass(). You
> can check it is the case here, Ld is
> org/eclipse/osgi/internal/baseadaptor/DefaultClassLoader.
> So the key change is the following:
>
>      protected final Class<?> findLoadedClass(String name) {
>  -        return initiatedClasses.get(name);
>  +        return VMClassRegistry.findLoadedClass(name, this);
>      }

Um, probably I should finish the explanation.
So, as Java class registry is bypassed, the Ld is registered as
defining loader for the IStatus class on VM side only. When Ld is
asked for this class next time, it checks findLoadedClass() for it but
in vain, then Ld proceeds with defining it for the second time and VM
disallows this, throwing LinkageError.

This way, the fix consist of redirecting findLoadedClass() request to
VM, in accordance with JVMS. It is possible to consider the
initiatedClasses table as an optimization to classloading, but then it
should be double-checked for correctness and real perf gain.

How Eclipse worked before and which particular commit broke it, I
dunno. My guess is that 3.2.0 version of Eclipse behave slightly
different in regards to startup classloading than 3.2.1, so it stopped
working due to Eclipse upgrade...

> And remained part of commit is trivial clean up.
>
> Well, this sutiation raises several issues:
> 1) Seems nobody tried running Eclipse since mid-October (after
> resolving HARMONY-1688). There were some discussions around automated
> Eclipse scenarios, AFAIR there are even some JIRA hanging for a while;
> shouldn't we
> revive this direction and integrate them in CC?
> 2) Code for handling initiating loaders needs closer inspection, I
> found some suspecting places. E.g. why CLassLoader::LookupClass() does
> not look in a table of initiated classes, do we behave rigth in
> regards to array classes, etc.
> I recall Eugene Ostrovsky mentioned about some tests he did
> inestigating RI behaviour, it would be nice if we have them in SVN.
>
> [1] http://article.gmane.org/gmane.comp.java.harmony.devel/15269
>
> --
> Alexey
>
>
> 2006/11/22, Geir Magnusson Jr. <geir@pobox.com>:
> > More than that, things just don't break by themselves.
> >
> > Alexey, please explain why you think this happened...
> >
> > geir
> >
> >
> >
> > Nathan Beyer wrote:
> > > Yeah, I'd like to hear about what's going on here. The classloader
> > > pieces of DRLVM are not quite obvious, at least not to me, from my
> > > cursory look.
> > >
> > > -Nathan
> > >
> > > On 11/21/06, Geir Magnusson Jr. <geir@pobox.com> wrote:
> > >> Hang on - where did this come from and why?  I'd like to understand this
> > >> before we accept it.
> > >>
> > >> geir
> > >>
> > >>
> > >> varlax@apache.org wrote:
> > >> > Author: varlax
> > >> > Date: Tue Nov 21 00:57:13 2006
> > >> > New Revision: 477583
> > >> >
> > >> > URL: http://svn.apache.org/viewvc?view=rev&rev=477583
> > >> > Log:
> > >> > Fixed Eclipse launching. The reason was in dangling remainders of
> > >> Java class registry.
> > >> > Tested on SUSE9, Win2003
> > >> >
> > >> > Modified:
> > >> >
> > >> harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ClassLoaderTest.java
> > >>
> > >> >
> > >> harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ClassLoader.java
> > >>
> > >> >
> > >> > Modified:
> > >> harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ClassLoaderTest.java
> > >>
> > >> > URL:
> > >> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ClassLoaderTest.java?view=diff&rev=477583&r1=477582&r2=477583
> > >>
> > >> >
> > >> ==============================================================================
> > >>
> > >> > ---
> > >> harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ClassLoaderTest.java
> > >> (original)
> > >> > +++
> > >> harmony/enhanced/drlvm/trunk/vm/tests/kernel/java/lang/ClassLoaderTest.java
> > >> Tue Nov 21 00:57:13 2006
> > >> > @@ -706,9 +706,9 @@
> > >> >      }
> > >> >
> > >> >      /**
> > >> > -     *
> > >> > +     * FIXME invalid test: only VM can initiate loading class
> > >> >       */
> > >> > -    public void test_findLoadedClass_Str_2() {
> > >> > +    public void te_st_findLoadedClass_Str_2() {
> > >> >          // TEST CASE #4:
> > >> >          try {
> > >> >              Class c =
> > >> Class.forName("java.lang.ClassLoaderTest$7LCL", true,
> > >> > @@ -915,7 +915,8 @@
> > >> >                  return 104;
> > >> >              }
> > >> >          }
> > >> > -        new a3().main(new String[] { "" });
> > >> > +        // FIXME invalid test: only VM can initiate loading class
> > >> > +        //new a3().main(new String[] { "" });
> > >> >      }
> > >> >
> > >> >
> > >> >
> > >> > Modified:
> > >> harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ClassLoader.java
> > >>
> > >> > URL:
> > >> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ClassLoader.java?view=diff&rev=477583&r1=477582&r2=477583
> > >>
> > >> >
> > >> ==============================================================================
> > >>
> > >> > ---
> > >> harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ClassLoader.java
> > >> (original)
> > >> > +++
> > >> harmony/enhanced/drlvm/trunk/vm/vmcore/src/kernel_classes/javasrc/java/lang/ClassLoader.java
> > >> Tue Nov 21 00:57:13 2006
> > >> > @@ -107,12 +107,6 @@
> > >> >      private final HashMap<String, Package> definedPackages;
> > >> >
> > >> >      /**
> > >> > -     * The following mapping is used <String binaryClassName,
Class
> > >> clazz>, where binaryClassName - class name,
> > >> > -     * clazz - corresponding class.
> > >> > -     */
> > >> > -    private Hashtable<String, Class<?>> initiatedClasses
= new
> > >> Hashtable<String, Class<?>>();
> > >> > -
> > >> > -    /**
> > >> >       * package private to access from the java.lang.Class class.
> > >> The following
> > >> >       * mapping is used <String name, Certificate[] certificates>,
> > >> where name -
> > >> >       * the name of a package, certificates - array of certificates.
> > >> > @@ -461,7 +455,7 @@
> > >> >       * @com.intel.drl.spec_ref
> > >> >       */
> > >> >      protected final Class<?> findLoadedClass(String name) {
> > >> > -        return initiatedClasses.get(name);
> > >> > +        return VMClassRegistry.findLoadedClass(name, this);
> > >> >      }
> > >> >
> > >> >      /**
> > >> > @@ -547,7 +541,6 @@
> > >> >          if (resolve) {
> > >> >              resolveClass(clazz);
> > >> >          }
> > >> > -        initiatedClasses.put(clazz.getName(), clazz);
> > >> >          return clazz;
> > >> >      }
> > >> >
> > >> >
> > >> >
> > >> >
> > >>
> >
>

Mime
View raw message