ReadyTalk / avian

[INACTIVE] Avian is a lightweight virtual machine and class library designed to provide a useful subset of Java's features, suitable for building self-contained applications.
https://readytalk.github.io/avian/
Other
1.22k stars 172 forks source link

Fix crash when resolving system classes. #541

Closed chrisr3 closed 7 years ago

chrisr3 commented 7 years ago

I uncovered this crash when trying to write a classloader that extends avian.SystemClassLoader. The problem is that loader->map() can return NULL, but the resolveSystemClass() function doesn't check for this. However, this check is built into the findLoadedClass() function, so use that instead.

Having said this, I am wondering if avian.SystemClassLoader is supposed to be extended in the first place. Consider this test case, which currently fails because the class returned by findClass() doesn't have myLoader as its classloader, despite the class having been loaded by it:

import java.io.IOException;
import java.io.File;
import java.io.FileInputStream;

public class CustomClassLoaders {
  private static void expect(boolean v) {
    if (! v) throw new RuntimeException();
  }

  private static void testClassLoader() throws Exception {
    MyClassLoader myLoader = new MyClassLoader();
    Class<?> myClass = myLoader.findClass("CustomClassLoaders$Hello");
    expect(myClass.getClassLoader() == myLoader);
  }

  public static void main(String[] args) throws Exception {
    testClassLoader();
  }

  public static class Hello {
    public static void main(String[] args) {
      System.out.println("hello, world!");
    }
  }

  private static class MyClassLoader extends avian.SystemClassLoader {
    @Override
    public Class<?> findClass(String name) throws ClassNotFoundException {
      return super.findClass(name);
    }
  }
}

The OpenJDK equivalent of MyClassLoader would look like this:

  private static class MyClassLoader extends java.net.URLClassLoader {
    MyClassLoader(ClassLoader parent) {
        super(((URLClassLoader)parent).getURLs(), parent);
    }

    @Override
    public Class<?> findClass(String name) throws ClassNotFoundException {
      return super.findClass(name);
    }
  }

And this does associate class Hello with my custom classloader, so Avian's classloading semantics look different to me.

chrisr3 commented 7 years ago

I have hacked together a "Nuclear Evil" native method to alter a class's classloader immediately after it has been resolved:

extern "C" AVIAN_EXPORT int64_t JNICALL
    Avian_avian_Classes_nuclearEvil(Thread* t, object, uintptr_t* arguments)
{
    GcJclass* classType = cast<GcJclass>(t, reinterpret_cast<object>(arguments[0]));
    GcClassLoader* classLoader = cast<GcClassLoader>(t, reinterpret_cast<object>(arguments[1]));
    classType->vmClass()->setLoader(t, classLoader);
    classType->setClassLoader(t, classLoader);
    return reinterpret_cast<intptr_t>(classType);
}

I can then write a custom classloader as follows:

public class MyClassLoader extends avian.SystemClassLoader {
    @Override
    public Class<?> findClass(String name) throws ClassNotFoundException {
        return avian.Classes.nuclearEvil(super.findClass(name), this);
    }
}

Avian will then (correctly) use my new classloader to link classes that my classloader has loaded. It would obviously be better if Avian would assign the correct classloader automatically in the first place. However, my naive attempts at implementing this are just resulting in "unsafe" exceptions.

dicej commented 7 years ago

Hi Chris,

Sorry about the delayed response. As you found, SystemClassLoader was never meant to be used directly or extended by application code, and the same goes for almost all the classes under the avian package. Those classes exist to bridge the abstraction gap between the Avian VM and the standard Java classes.

Did you try extending java.net.URLClassLoader as you would have with OpenJDK? If you did try, and it didn't work, maybe there's something we can fix there so you don't have to reference a non-standard class like SystemClassLoader in your application code.

chrisr3 commented 7 years ago

Hi, thanks for replying. My use-case is trying to filter which classes can be loaded from the boot classloader itself, which I have been unable to do by writing Java class-loaders. I have ultimately resorted to implementing my filter inside here instead:

GcClass* resolveSystemClass(Thread* t,
                            GcClassLoader* loader,
                            GcByteArray* spec,
                            bool throw_,
                            Gc::Type throwType);

This seems to have the semantics that I want, even if I did also have to modify avian.SystemClassLoader directly.

dicej commented 7 years ago

I see. So how would you have done that on OpenJDK? I would guess that there's no standard way to do it, in which case it makes sense that you'd have to use a non-standard, VM-specific approach.

chrisr3 commented 7 years ago

Yes, I found a similar place to patch such a filter into HotSpot once I'd given up trying to inject a new system classloader by setting OpenJDK's java.system.class.loader property. IIRC, I had tried implementing it as:

class MyClassLoader extends URLClassLoader {
    MyClassLoader(ClassLoader parent) {
        super(((URLClassLoader)parent).getURLs(), parent);
    }

    ...

which is obviously incompatible with Avian anyway because avian.SystemClassLoader doesn't extend URLClassLoader.