spring-projects / spring-loaded

Java agent that enables class reloading in a running JVM
Apache License 2.0
2.72k stars 515 forks source link

Conflict with Rhino Javascript Engine #69

Open ylazzari opened 10 years ago

ylazzari commented 10 years ago

I haven't debugged further, don't know where to start, but there seems to be an issue when using the Rhino Javascript engine (JDK 7) to instantiate an interface (haven't tested with Nashorn on JDK 8).

Assuming the following simple interface:

public interface SomeInterface {
    String sayHello();
}

The following should be possible to obtain an instance of this interface backed by a ScriptEngine:

String scriptContent = "function sayHello() { return \"Hello world\"; }"
ScriptEngineManager scriptEngineManager = new ScriptEngineManager();
ScriptEngine engine = scriptEngineManager.getEngineByName("JavaScript");
engine.eval(scriptContent);
SomeInterface someInterface = ((Invocable)engine).getInterface(SomeInterface.class);
System.out.println(someInterface.sayHello());

This should print "Hello world". When I run this test case with the Spring-Loaded agent, the "getInterface" method on the engine returns "null", which usually happens when the script doesn't implement all the methods of the interface. I guess the script engine doesn't play nice when the class has been loaded with the agent running.

aclement commented 10 years ago

Possibly caused by some reflection calls not being intercepted correctly. If you gave me a sample project I might be able to dig into it but I have no experience with rhino.

ylazzari commented 10 years ago

All you have to do is create a class with the following content:

import javax.script.Invocable;
import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;

public class ScriptWithSpringLoadedAgentTestCase {

    public interface SomeInterface {
        String sayHello();
    }

    public static void main(String[] args) throws Exception {
        String scriptContent = "function sayHello() { return \"Hello world\"; }";
        ScriptEngineManager scriptEngineManager = new ScriptEngineManager();
        ScriptEngine engine = scriptEngineManager.getEngineByName("JavaScript");
        engine.eval(scriptContent);
        SomeInterface someInterface = ((Invocable) engine).getInterface(SomeInterface.class);
        System.out.println(someInterface.sayHello());
    }
}

When you run it, it prints "Hellow world". Add the VM arguments to enable the agent, and you get a NullPointerException because the engine couldn't figure out that the script block implements the interface properly:

Exception in thread "main" java.lang.NullPointerException
    at com.mediagrif.mets.testcase.script.ScriptWithSpringLoadedAgentTestCase.main(ScriptWithSpringLoadedAgentTestCase.java:19)

Might be a bit of a pain to debug though. The Rhino implementation code in com.sun.script.javascript... doesn't seem to be distributed with the JDK. Anyways, thanks a lot for looking into it if you have time!

aclement commented 10 years ago

Thanks for the test code, very helpful. It looks to be what I expected. I had some 'debug' logic in that determines if classes we aren't currently modifying actually need modifying - I've now put that logic behind some real config options. I just committed some changes which enable your test case to work (and without the options it still fails).

The options are:

investigateSystemClassReflection - this option will cause us to (expensively) dig through classes to see if they are using reflection but we aren't currently modifying them. This will print out classes it finds, it will not change them

rewriteAllSystemClasses - related to the option above, this will change them.

Running your test program with

-Dspringloaded=rewriteAllSystemClasses

now works and prints Hello world

This is using the latest code so a 1.2.1 build snapshot. The full fix here will be to fold the list of classes that needed changing into the managed list inside spring loaded, but at least this will get you far enough to play :)

Note, there will be messages like this:

!!! SystemClassReflectionRewriter: nyi for java/lang/reflect/Field.getBoolean

which indicates we aren't currently rewriting reflective calls to Field.getBoolean() - which is another thing we should do in properly fixing this issue.

ylazzari commented 10 years ago

Thanks for the quick response! My test case was more complicate than what I submitted as an example, but it now works. I will play around with it longer and report if I find anything else! Feel free to close this issue if you want.