Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with 1.3.1-SNAPSHOT and extraClasspath #274

Closed
theigl opened this issue Dec 2, 2018 · 11 comments
Closed

Issues with 1.3.1-SNAPSHOT and extraClasspath #274

theigl opened this issue Dec 2, 2018 · 11 comments
Labels

Comments

@theigl
Copy link
Contributor

theigl commented Dec 2, 2018

Thanks for the great library and your work on JDK11!

I just downloaded the latest JDK11 binaries and tested them with my application and a vanilla Spring Boot starter application (see attachment).

Everything seems to work fine with the default configuration.

If I add anything to extraClasspath the application fails to start with ClassNotFoundExceptions:

Caused by: java.lang.NoClassDefFoundError: org/hotswap/agent/javassist/util/proxy/ProxyObject
at java.base/java.lang.ClassLoader.defineClass1(Native Method) ~[na:na]
at java.base/java.lang.System$2.defineClass(System.java:2123) ~[na:na]
at java.base/java.lang.invoke.MethodHandles$Lookup.defineClass(MethodHandles.java:962) ~[na:na]
at org.hotswap.agent.javassist.util.proxy.DefineClassHelper.toClass(DefineClassHelper.java:298) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.javassist.util.proxy.DefineClassHelper$Java11.defineClass(DefineClassHelper.java:48) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.javassist.util.proxy.DefineClassHelper.toClass(DefineClassHelper.java:263) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.javassist.util.proxy.FactoryHelper.toClass(FactoryHelper.java:136) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.javassist.util.proxy.ProxyFactory.createClass3(ProxyFactory.java:603) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.javassist.util.proxy.ProxyFactory.createClass2(ProxyFactory.java:587) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.javassist.util.proxy.ProxyFactory.createClass1(ProxyFactory.java:523) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.javassist.util.proxy.ProxyFactory.createClass(ProxyFactory.java:449) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.util.classloader.URLClassLoaderHelper.<clinit>(URLClassLoaderHelper.java:47) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.config.PluginConfiguration.initExtraClassPath(PluginConfiguration.java:178) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.config.PluginConfiguration.init(PluginConfiguration.java:159) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.config.PluginConfiguration.<init>(PluginConfiguration.java:62) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.config.PluginManager.initClassLoader(PluginManager.java:170) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.config.PluginManager.initClassLoader(PluginManager.java:143) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.config.PluginRegistry.initializePlugin(PluginRegistry.java:140) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.util.PluginManagerInvoker.callInitializePlugin(PluginManagerInvoker.java:26) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.hotswap.agent.plugin.tomcat.TomcatPlugin.init(TomcatPlugin.java:106) ~[hotswap-agent.jar:1.3.1-SNAPSHOT]
at org.apache.catalina.loader.WebappLoader.startInternal(WebappLoader.java:422) ~[tomcat-embed-core-8.5.31.jar:8.5.31]
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150) ~[tomcat-embed-core-8.5.31.jar:8.5.31]
... 9 common frames omitted    

You can easily verify this behavior by uncommenting extraClasspath in the attached application and running com.example.demo.DemoApplication.

demo.zip

@skybber
Copy link
Contributor

skybber commented Dec 2, 2018

Hi, thanks for report. There is a problem in javassist. It is not possible to make proxy of JDK-11 class. I've tried to fix it, the demoproject is starting now, but the extraClassPath is not working since the UrlClassLoader patching mechanism is still failing. You can check the fix at https://github.com/skybber/HotswapAgent/commits/master

@skybber skybber added the bug label Dec 2, 2018
@theigl
Copy link
Contributor Author

theigl commented Dec 3, 2018

@skybber thanks for the ultra-fast fix! I'll give it a try asap.

Is the javassist issue something we should report to them or do you just have to use a different approach for JDK11?

Is the UrlClassLoader patching something you can fix eventually or is there some deeper issue?

@skybber
Copy link
Contributor

skybber commented Dec 3, 2018

It looks that the problem has been already reported jboss-javassist/javassist#228. We had to patch UrlClassLoader to be able to accept extraClass path in j9. Let's look at the source:

https://github.com/HotswapProjects/HotswapAgent/blob/master/hotswap-agent-core/src/main/java/org/hotswap/agent/util/classloader/URLClassLoaderHelper.java

UrlClassLoader keeps URLs in field of class URLClassPath. URLClassPath was moved to jdk.internal.loader package in java9, therefore we can proxy both old sun.misc.URLClassPath or new jdk.internal.loader.URLClassPath with single proxy class. For it, we're using proxy definer from javassist, since it allows to proxy classes extended from java.lang.Object. (Unfortunately we can't use java.lang.reflect.Proxy in this case). Javassist's Proxy definer creates proxy class using either :

  • classLoader.define() for j8
  • unsafe.define() for j9,j10
  • MethodHandles.lookup().define() for j11

and there is pretty problem in j11, since the javassist mechanism (which is only allowed in java >=j11) is not able to make proxy on jdk class! Any attempt to make proxy throws exception - javassist ProxyObject class is unknown for JDK classes. May be I'm overlooking something or we could use another machanism in URLClassLoaderHelper.java as you're writting about. Have you some idea ?

@theigl
Copy link
Contributor Author

theigl commented Dec 3, 2018

I just re-read this article: https://dzone.com/articles/jdk-11-and-proxies-in-a-world-past-sunmiscunsafe

In order to perform a class definition, an instance of MethodHandles.Lookup is required which can be retrieved by invoking the MethodHandles::lookup method. Invoking the latter method is call site sensitive; the returned instance will therefore represent the privileges of the class and package from within the method is invoked. To define a class in another package then the current one, a class from this package is required to resolve against it using MethodHandles::privateLookupIn. This will only be possible if this target class's package resides in the same module as the original lookup class or if this package is explicitly opened to the lookup class's module. If those requirements are not met, attempting to resolving the private lookup throws an IllegalAccessException, protecting the boundaries that are implied by the JPMS.

I'm not an expert on this subject, but it sounds like proxying internal classes can only be done from other internal classes or if the package is explicitly opened via java --add-opens.

@skybber
Copy link
Contributor

skybber commented Dec 3, 2018

I've read this article carefully yesterday. It looks that you're right. We can open it in dcevm as we're opening another packages, but I think it is not good solution since it can bring some problems to end users.

@skybber
Copy link
Contributor

skybber commented Dec 3, 2018

Problem should be fixed now. Btw I've seen your wicket plugin 👍 , may be we could use it in HotswapAgent.

@skybber skybber closed this as completed Dec 3, 2018
@theigl
Copy link
Contributor Author

theigl commented Dec 3, 2018

@skybber absolutely! Please feel free to include it in HotswapAgent.

I'll test your fix asap and will give feedback.

@skybber
Copy link
Contributor

skybber commented Dec 3, 2018

Could you make, please, initial PR for WicketPlugin? Or where can I get most up-to-date version?

@theigl
Copy link
Contributor Author

theigl commented Dec 4, 2018

I'll create a PR.

@theigl theigl changed the title Issues with 1.3.1-SNAPSHOT and extraClasspath/pluginPackages Issues with 1.3.1-SNAPSHOT and extraClasspath Dec 4, 2018
@theigl
Copy link
Contributor Author

theigl commented Dec 4, 2018

@skybber: Your fix works!

I created another ticket of the less important logging issue: #276

@theigl
Copy link
Contributor Author

theigl commented Dec 6, 2018

@skybber I just created a PR for the Wicket plugin: #278

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants