On Wed, 16 Dec 2020 19:44:04 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Mandy is correct, the bigger picture here is accessibility. Code in the >> java.instrument module should be able to invoke the premain or agentmain >> without needing to suppress access checks. It's a perquisite to supporting >> the deployment of java agents as modules. When an agent is deployed as a >> module it will need the agent class to be public in a package that is >> exported to at least the java.instrument module. The premain method will >> need to be public too. >> So PR1694 is really just about aligning the spec so that the premain method >> is public. However, it does not fix the overall accessibility issue. Fixing >> the accessibility issue will fixing the java.lang.instrument package >> description to specify that the agent class is public. > >> So PR1694 is really just about aligning the spec so that the premain method >> is public. However, it does not fix the overall accessibility issue. > > OK I see that JDK-5070281 changes to allow the agent class to be non-public > as the main class that declares `public static void main`. So > `setAccessible` should only be called if the agent class is in an unnamed > module. > > If the agent class is in a named module and not accessible to java.instrument > module (if the java agent is packaged in a modular JAR file), it should fail > with IAE - can you confirm? I wanted to understand the current behavior. Mandy, I've pushed updates with changes: - NegativeAgentRunner checks if the exit value is non-zero - AgentJarBuilder is replaced with JavaAgentBuilder - removed unneeded qualified exports > So setAccessible should only be called if the agent class is in an unnamed > module. Now the agent class is always loaded as in the unnamed module. I tried to test this with a modular jar file which has module-info.class in it. In fact, I don't know if there are any options that would help to load the premain agent class in named module. I guess, it has to be implemented as part of the enhancement https://bugs.openjdk.java.net/browse/JDK-6932391 . The following patch can be committed if you like as it does not harm: @@ -476,11 +476,13 @@ public class InstrumentationImpl implements Instrumentation { String msg = "method " + classname + "." + methodname + " must be declared public"; throw new IllegalAccessException(msg); } - - if ((javaAgentClass.getModifiers() & java.lang.reflect.Modifier.PUBLIC) == 0) { - // If the java agent class is not public, make the premain/agentmain - // accessible, so we can call it. + if ((javaAgentClass.getModifiers() & java.lang.reflect.Modifier.PUBLIC) == 0 && + !javaAgentClass.getModule().isNamed()) { + // If the java agent class is not public and is in unnamed module + // then make the premain/agentmain accessible, so we can call it. setAccessible(m, true); } ------------- PR: https://git.openjdk.java.net/jdk/pull/1694
